[GitHub] [arrow] augustoasilva commented on a change in pull request #10350: ARROW-12814: [C++][Gandiva] Implements ABS, FLOOR, PI, SQRT, SIGN, LSHIFT, RSHIFT, CEIL, TRUNC, LN and LOG2 functions

[GitBox <gi...@apache.org>]

augustoasilva commented on a change in pull request #10350:
URL: https://github.com/apache/arrow/pull/10350#discussion_r670892726



##########
File path: cpp/src/gandiva/precompiled/extended_math_ops.cc
##########
@@ -212,6 +239,24 @@ ENUMERIC_TYPES_UNARY(RADIANS, float64)
   }
 ENUMERIC_TYPES_UNARY(DEGREES, float64)
 
+// Ceil
+#define CEIL(IN_TYPE)                                                      \
+  FORCE_INLINE                                                             \
+  gdv_##IN_TYPE ceil_##IN_TYPE(gdv_##IN_TYPE in) {                         \
+    return static_cast<gdv_##IN_TYPE>(ceil(static_cast<long double>(in))); \
+  }
+CEIL(float32)
+CEIL(float64)
+
+// Ceil

Review comment:
       I think this comment was wrong, please fix it.
   
   ```suggestion
   // Floor```

##########
File path: cpp/src/gandiva/gdv_function_stubs_test.cc
##########
@@ -20,10 +20,19 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include <cmath>
+
 #include "gandiva/execution_context.h"
 
 namespace gandiva {
 
+static const double DEFAULT_THRESHOLD = 0.00005;

Review comment:
       Why did you choose this threshold? On fabs documentation it uses 6 places for decimal part, shouldn't it be **0.000005** a better value for AlmostEquals threshold?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org