You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/07/21 21:51:51 UTC
[GitHub] [arrow] anthonylouisbsb commented on a change in pull request #10742: ARROW-13375: [C++][Gandiva] Implement POSITIVE and NEGATIVE Hive functions on Gandiva
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org