[GitHub] [arrow] anthonylouisbsb commented on a change in pull request #12526: [Gandiva][C++] Fix function istrue and is not true

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

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



##########
File path: cpp/src/gandiva/function_registry_common.h
##########
@@ -285,4 +285,13 @@ typedef std::unordered_map<const FunctionSignature*, const NativeFunction*, KeyH
 #define NUMERIC_BOOL_DATE_VAR_LEN_TYPES(INNER, NAME, ALIASES) \
   NUMERIC_BOOL_DATE_TYPES(INNER, NAME, ALIASES), VAR_LEN_TYPES(INNER, NAME, ALIASES)
 
+// Is True and Is Not True functions with validity that :
+// - NULL handling is of type NULL_NEVER
+//
+// The pre-compiled fn name includes the base name & input type name. istrue_boolean /
+// isfalse_boolean
+#define ISTRUE_ISFALSE_NULL_NEVER(NAME, ALIASES, IN_TYPE, OUT_TYPE)                  \

Review comment:
       You can follow the example of `UNARY_SAFE_NULL_IF_NULL` and create a more generic macro, like `UNARY_SAFE_NULL_NEVER`, if it does not exist yet

##########
File path: cpp/src/gandiva/precompiled/arithmetic_ops.cc
##########
@@ -234,9 +234,13 @@ NUMERIC_TYPES(VALIDITY_OP, isnumeric, +)
 
 #undef VALIDITY_OP
 
-#define IS_TRUE_OR_FALSE_BOOL(NAME, TYPE, OP) \
-  FORCE_INLINE                                \
-  gdv_##TYPE NAME##_boolean(gdv_##TYPE in) { return OP in; }
+#define IS_TRUE_OR_FALSE_BOOL(NAME, TYPE, OP)                      \
+  gdv_##TYPE NAME##_boolean(gdv_##TYPE in, gdv_boolean is_valid) { \
+    if (!is_valid) {                                               \

Review comment:
       I think you can just return:
   ```
   return !is_valid && OP in;
   ```
   
   And remove the if clause




-- 
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