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/05/12 13:06:41 UTC

[GitHub] [arrow] projjal commented on a change in pull request #10112: ARROW-12479: [C++][Gandiva] Implement castBigInt, castInt, castIntervalDay and castIntervalYear extra functions

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



##########
File path: cpp/src/gandiva/precompiled/time.cc
##########
@@ -828,4 +828,44 @@ gdv_int64 castBIGINT_daytimeinterval(gdv_day_time_interval in) {
          extractDay_daytimeinterval(in) * MILLIS_IN_DAY;
 }
 
+#define CAST_INT_YEAR_INTERVAL(NAME, OUT_TYPE)                \
+  FORCE_INLINE                                                \
+  gdv_##OUT_TYPE NAME##_year_interval(gdv_year_interval in) { \
+    return static_cast<gdv_##OUT_TYPE>(in / 12.0);            \
+  }
+
+CAST_INT_YEAR_INTERVAL(castBIGINT, int64)
+CAST_INT_YEAR_INTERVAL(castINT, int32)
+
+#define CAST_NULLABLE_YEAR_INTERVAL(NAME, OUT_TYPE)                        \

Review comment:
       nit: use TYPE instead of NAME

##########
File path: cpp/src/gandiva/precompiled/time.cc
##########
@@ -828,4 +828,44 @@ gdv_int64 castBIGINT_daytimeinterval(gdv_day_time_interval in) {
          extractDay_daytimeinterval(in) * MILLIS_IN_DAY;
 }
 
+#define CAST_INT_YEAR_INTERVAL(NAME, OUT_TYPE)                \
+  FORCE_INLINE                                                \
+  gdv_##OUT_TYPE NAME##_year_interval(gdv_year_interval in) { \
+    return static_cast<gdv_##OUT_TYPE>(in / 12.0);            \

Review comment:
       why divide by 12

##########
File path: cpp/src/gandiva/function_registry_datetime.cc
##########
@@ -83,6 +83,38 @@ std::vector<NativeFunction> GetDateTimeFunctionRegistry() {
       NativeFunction("castBIGINT", {}, DataTypeVector{day_time_interval()}, int64(),
                      kResultNullIfNull, "castBIGINT_daytimeinterval"),
 
+      NativeFunction("castINT", {}, DataTypeVector{month_interval()}, int32(),

Review comment:
       why not use castINT and castNULLABLEINT as alias

##########
File path: cpp/src/gandiva/function_registry_arithmetic.cc
##########
@@ -33,6 +33,10 @@ namespace gandiva {
 
 #define UNARY_CAST_TO_FLOAT32(name) UNARY_SAFE_NULL_IF_NULL(castFLOAT4, {}, name, float32)
 
+#define UNARY_CAST_TO_INT32(name) UNARY_SAFE_NULL_IF_NULL(castINT, {}, name, int32)

Review comment:
       nit: replace 'name' with 'type'. Also in the above two 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:
users@infra.apache.org