projjal commented on a change in pull request #9890:
URL: https://github.com/apache/arrow/pull/9890#discussion_r609323671



##########
File path: cpp/src/gandiva/function_registry_datetime.cc
##########
@@ -83,7 +83,9 @@ std::vector<NativeFunction> GetDateTimeFunctionRegistry() {
       NativeFunction("extractDay", {}, DataTypeVector{day_time_interval()}, 
int64(),
                      kResultNullIfNull, "extractDay_daytimeinterval"),
 
-      DATE_TYPES(LAST_DAY_SAFE_NULL_IF_NULL, last_day, {})};
+      DATE_TYPES(LAST_DAY_SAFE_NULL_IF_NULL, last_day, {}),
+
+      DATE_TYPES(TO_TIMESTAMP_SAFE_NULL_IF_NULL, to_timestamp, {})};

Review comment:
       If the function converts number of seconds to timestamp, than the 
argument should be int32/int64/float/double. Date/timestamp as arguments would 
be incorrect as they are already in millis.

##########
File path: cpp/src/gandiva/function_registry_common.h
##########
@@ -174,6 +174,15 @@ typedef std::unordered_map<const FunctionSignature*, const 
NativeFunction*, KeyH
   NativeFunction(#NAME, std::vector<std::string> ALIASES, 
DataTypeVector{TYPE()}, \
                  date64(), kResultNullIfNull, 
ARROW_STRINGIFY(NAME##_from_##TYPE))
 
+// To timestamp functions (used with data/time types) that :
+// - NULL handling is of type NULL_IF_NULL
+//
+// The pre-compiled fn name includes the base name & input type name. eg:
+// - to_timestamp_date64
+#define TO_TIMESTAMP_SAFE_NULL_IF_NULL(NAME, ALIASES, TYPE)                    
   \

Review comment:
       better to keep these macros in function_registry_datetime.cc. This file 
is for generic macros.




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

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


Reply via email to