anthonylouisbsb commented on a change in pull request #10742:
URL: https://github.com/apache/arrow/pull/10742#discussion_r674369764



##########
File path: cpp/src/gandiva/precompiled/arithmetic_ops.cc
##########
@@ -208,6 +208,22 @@ NUMERIC_FUNCTION(DIVIDE)
 
 #undef DIVIDE
 
+#define POSITIVE(TYPE) \
+  FORCE_INLINE         \
+  gdv_##TYPE positive_##TYPE(gdv_##TYPE in) { return 
static_cast<gdv_##TYPE>(in); }
+
+NUMERIC_FUNCTION(POSITIVE)
+
+#undef POSITIVE
+
+#define NEGATIVE(TYPE) \
+  FORCE_INLINE         \
+  gdv_##TYPE negative_##TYPE(gdv_##TYPE in) { return 
static_cast<gdv_##TYPE>(-1 * in); }

Review comment:
       I think is a good idea to add a test using the `MIN` and `MIN - 1` 
values to see if it is working correctly.

##########
File path: cpp/src/gandiva/precompiled/arithmetic_ops.cc
##########
@@ -208,6 +208,22 @@ NUMERIC_FUNCTION(DIVIDE)
 
 #undef DIVIDE
 
+#define POSITIVE(TYPE) \
+  FORCE_INLINE         \
+  gdv_##TYPE positive_##TYPE(gdv_##TYPE in) { return 
static_cast<gdv_##TYPE>(in); }
+
+NUMERIC_FUNCTION(POSITIVE)
+
+#undef POSITIVE
+
+#define NEGATIVE(TYPE) \
+  FORCE_INLINE         \
+  gdv_##TYPE negative_##TYPE(gdv_##TYPE in) { return 
static_cast<gdv_##TYPE>(-1 * in); }

Review comment:
       Check [this 
line](https://github.com/apache/arrow/blob/848fe4e6032f0291e7174cef5019e48ae2a55f21/cpp/src/gandiva/gdv_function_stubs.cc#L776)
 with the implementation of the ABS function.
   
   You need to take care if the input value is equals to the MIN value for an 
integer, because when converting to positive you cause loss of digits

##########
File path: cpp/src/gandiva/precompiled/arithmetic_ops.cc
##########
@@ -208,6 +208,22 @@ NUMERIC_FUNCTION(DIVIDE)
 
 #undef DIVIDE
 
+#define POSITIVE(TYPE) \
+  FORCE_INLINE         \
+  gdv_##TYPE positive_##TYPE(gdv_##TYPE in) { return 
static_cast<gdv_##TYPE>(in); }

Review comment:
       Since you are casting to the same type, I think you can just return the 
input number, right?




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to