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/04/01 15:29:53 UTC
[GitHub] [arrow] jpedroantunes opened a new pull request #9873: ARROW-12180: [C++][Gandiva] Implement new cache for Gandiva focused on a build time policy
jpedroantunes opened a new pull request #9873:
URL: https://github.com/apache/arrow/pull/9873
Converts ms, the number of milliseconds since midnight, to a time.
--
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:
users@infra.apache.org
[GitHub] [arrow] projjal commented on a change in pull request #9873: ARROW-12180: [C++][Gandiva] Implement TO_TIME([number] ms) function
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #9873:
URL: https://github.com/apache/arrow/pull/9873#discussion_r609459002
##########
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 time (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_time_date64
+#define TO_TIME_SAFE_NULL_IF_NULL(NAME, ALIASES, TYPE) \
Review comment:
better to move this macro to registry_datetime.cc
##########
File path: cpp/src/gandiva/precompiled/time.cc
##########
@@ -817,4 +817,19 @@ gdv_int64 castBIGINT_daytimeinterval(gdv_day_time_interval in) {
extractDay_daytimeinterval(in) * MILLIS_IN_DAY;
}
+// Convert milliseconds to date, considering as input the quantity of milliseconds
+// after midnight
Review comment:
quantity of millis since midnight is defined to be time. I think you are supposed to convert millis since epoch to time.
You can also add another function to_date which would return the corresponding date
##########
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 time (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_time_date64
+#define TO_TIME_SAFE_NULL_IF_NULL(NAME, ALIASES, TYPE) \
+ NativeFunction(#NAME, std::vector<std::string> ALIASES, DataTypeVector{TYPE()}, \
+ date64(), kResultNullIfNull, ARROW_STRINGIFY(NAME##_##TYPE))
Review comment:
output should be time instead of date
##########
File path: cpp/src/gandiva/precompiled/time.cc
##########
@@ -817,4 +817,19 @@ gdv_int64 castBIGINT_daytimeinterval(gdv_day_time_interval in) {
extractDay_daytimeinterval(in) * MILLIS_IN_DAY;
}
+// Convert milliseconds to date, considering as input the quantity of milliseconds
+// after midnight
+#define TO_TIME(TYPE) \
+ FORCE_INLINE \
+ gdv_date64 to_time##_##TYPE(gdv_##TYPE millis) { \
+ EpochTimePoint actual_tp(EpochTimePoint::NowMillisEpoch()); \
Review comment:
why is current time need to be used? The argument is already provided
##########
File path: cpp/src/gandiva/precompiled/time.cc
##########
@@ -817,4 +817,19 @@ gdv_int64 castBIGINT_daytimeinterval(gdv_day_time_interval in) {
extractDay_daytimeinterval(in) * MILLIS_IN_DAY;
}
+// Convert milliseconds to date, considering as input the quantity of milliseconds
+// after midnight
+#define TO_TIME(TYPE) \
+ FORCE_INLINE \
+ gdv_date64 to_time##_##TYPE(gdv_##TYPE millis) { \
Review comment:
output is not date but time
--
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:
users@infra.apache.org
[GitHub] [arrow] jpedroantunes closed pull request #9873: ARROW-12180: [C++][Gandiva] Implement TO_TIME([number] ms) function
Posted by GitBox <gi...@apache.org>.
jpedroantunes closed pull request #9873:
URL: https://github.com/apache/arrow/pull/9873
--
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:
users@infra.apache.org
[GitHub] [arrow] jpedroantunes commented on pull request #9873: ARROW-12180: [C++][Gandiva] Implement TO_TIME([number] ms) function
Posted by GitBox <gi...@apache.org>.
jpedroantunes commented on pull request #9873:
URL: https://github.com/apache/arrow/pull/9873#issuecomment-815788349
Actually this pull request was merged with the TO_TIMESTAMP, since the implementation is similar and use similar macros:
https://github.com/apache/arrow/pull/9890
--
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9873: ARROW-12180: [C++][Gandiva] Implement TO_TIME([number] ms) function
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9873:
URL: https://github.com/apache/arrow/pull/9873#issuecomment-812214273
https://issues.apache.org/jira/browse/ARROW-12180
--
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:
users@infra.apache.org