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/03/31 11:27:44 UTC

[GitHub] [arrow] anthonylouisbsb opened a new pull request #9861: ARROW-12166: [C++][Gandiva] Implements CONVERT_TO(value, type) function

anthonylouisbsb opened a new pull request #9861:
URL: https://github.com/apache/arrow/pull/9861


   Implements the CONVERT_TO function inside the Gandiva, which receives a value for a defined type and returns its bytes representation.
   
   The behavior is based on Apache Drill implementation: https://drill.apache.org/docs/data-type-conversion/#convert_to-and-convert_from


-- 
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] anthonylouisbsb commented on a change in pull request #9861: ARROW-12166: [C++][Gandiva] Implements CONVERT_TO(value, type) function

Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on a change in pull request #9861:
URL: https://github.com/apache/arrow/pull/9861#discussion_r623285117



##########
File path: cpp/src/gandiva/function_registry_string.cc
##########
@@ -209,6 +209,71 @@ std::vector<NativeFunction> GetStringFunctionRegistry() {
                      "convert_replace_invalid_fromUTF8_binary",
                      NativeFunction::kNeedsContext),
 
+      NativeFunction("convert_toDOUBLE", {"convert_todouble"}, DataTypeVector{float64()},

Review comment:
       I add a comment in the place that functions are created




-- 
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 #9861: ARROW-12166: [C++][Gandiva] Implements CONVERT_TO(value, type) function

Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #9861:
URL: https://github.com/apache/arrow/pull/9861#discussion_r609404186



##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -1246,6 +1246,69 @@ const char* convert_fromUTF8_binary(gdv_int64 context, const char* bin_in, gdv_i
   return ret;
 }
 
+#define CONVERT_TO_FN(TYPE)                                                              \
+  FORCE_INLINE                                                                           \
+  const char* convert_to##TYPE##_binary(gdv_int64 context, gdv_##TYPE value,             \
+                                        gdv_int32* out_len) {                            \
+    *out_len = sizeof(value);                                                            \
+    char* ret = reinterpret_cast<char*>(gdv_fn_context_arena_malloc(context, *out_len)); \
+    if (ret == nullptr) {                                                                \
+      gdv_fn_context_set_error_msg(context,                                              \
+                                   "Could not allocate memory for output string");       \
+      *out_len = 0;                                                                      \
+      return "";                                                                         \
+    }                                                                                    \
+    memcpy(ret, &value, *out_len);                                                       \
+    return ret;                                                                          \
+  }
+
+// Expand inner macro for all numeric types.
+#define NUMERIC_BOOL_DATE_TYPES(INNER) \
+  INNER(int8)                          \
+  INNER(int16)                         \
+  INNER(int32)                         \
+  INNER(int64)                         \
+  INNER(uint8)                         \
+  INNER(uint16)                        \
+  INNER(uint32)                        \
+  INNER(uint64)                        \
+  INNER(float32)                       \
+  INNER(float64)                       \
+  INNER(boolean)                       \
+  INNER(date64)                        \
+  INNER(date32)                        \
+  INNER(time32)                        \
+  INNER(timestamp)
+
+NUMERIC_BOOL_DATE_TYPES(CONVERT_TO_FN)
+
+#undef NUMERIC_BOOL_DATE_TYPES
+#undef CONVERT_TO_FN
+
+#define CONVERT_TO_FN_BUF_OP(TYPE)                                                       \
+  FORCE_INLINE                                                                           \
+  const char* convert_to##TYPE##_binary(gdv_int64 context, const char* value,            \
+                                        gdv_int32 value_len, gdv_int32* out_len) {       \
+    *out_len = value_len;                                                                \
+    char* ret = reinterpret_cast<char*>(gdv_fn_context_arena_malloc(context, *out_len)); \

Review comment:
       You can just return the original pointer and length instead of doing a malloc and memcpy

##########
File path: cpp/src/gandiva/precompiled/decimal_wrapper.cc
##########
@@ -430,4 +430,18 @@ char* castVARCHAR_decimal128_int64(int64_t context, int64_t x_high, uint64_t x_l
   return dec_str;
 }
 
+FORCE_INLINE
+const char* convert_todecimal128_binary(int64_t context, int64_t x_high, uint64_t x_low,
+                                        int32_t x_precision, int32_t x_scale,
+                                        int32_t* out_length) {
+  char* dec_str = gdv_fn_dec_to_string(context, x_high, x_low, x_scale, out_length);

Review comment:
       convert to binary and convert to string should be different. cast to string returns the string representation while convert to binary would return the 128 bytes

##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -1246,6 +1246,69 @@ const char* convert_fromUTF8_binary(gdv_int64 context, const char* bin_in, gdv_i
   return ret;
 }
 
+#define CONVERT_TO_FN(TYPE)                                                              \

Review comment:
       may can also add functions for bigendian outputs




-- 
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 #9861: ARROW-12166: [C++][Gandiva] Implements CONVERT_TO(value, type) function

Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #9861:
URL: https://github.com/apache/arrow/pull/9861#discussion_r623609556



##########
File path: cpp/src/gandiva/function_registry_string.cc
##########
@@ -225,6 +225,67 @@ std::vector<NativeFunction> GetStringFunctionRegistry() {
                      "convert_replace_invalid_fromUTF8_binary",
                      NativeFunction::kNeedsContext),
 
+      NativeFunction("convert_todouble", {}, DataTypeVector{float64()}, binary(),

Review comment:
       Should fix it if some part of code assumes the names are case-sensitive.




-- 
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 #9861: ARROW-12166: [C++][Gandiva] Implements CONVERT_TO(value, type) function

Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #9861:
URL: https://github.com/apache/arrow/pull/9861#discussion_r622906780



##########
File path: cpp/src/gandiva/function_registry_string.cc
##########
@@ -209,6 +209,71 @@ std::vector<NativeFunction> GetStringFunctionRegistry() {
                      "convert_replace_invalid_fromUTF8_binary",
                      NativeFunction::kNeedsContext),
 
+      NativeFunction("convert_toDOUBLE", {"convert_todouble"}, DataTypeVector{float64()},

Review comment:
       Can you add a comment explaining convert_toX functions converts X type to binary

##########
File path: cpp/src/gandiva/function_registry_string.cc
##########
@@ -209,6 +209,71 @@ std::vector<NativeFunction> GetStringFunctionRegistry() {
                      "convert_replace_invalid_fromUTF8_binary",
                      NativeFunction::kNeedsContext),
 
+      NativeFunction("convert_toDOUBLE", {"convert_todouble"}, DataTypeVector{float64()},
+                     binary(), kResultNullIfNull, "convert_toDOUBLE_binary",
+                     NativeFunction::kNeedsContext),
+
+      NativeFunction("convert_toDOUBLE_BE", {"convert_todouble_be"},
+                     DataTypeVector{float64()}, binary(), kResultNullIfNull,
+                     "convert_toDOUBLE_binary_be", NativeFunction::kNeedsContext),
+
+      NativeFunction("convert_toFLOAT", {"convert_tofloat"}, DataTypeVector{float32()},
+                     binary(), kResultNullIfNull, "convert_toFLOAT_binary",

Review comment:
       convert_toFLOAT_binary name seems ambigious..lets keep the function name as convert_toFLOAT

##########
File path: cpp/src/gandiva/function_registry_string.cc
##########
@@ -209,6 +209,71 @@ std::vector<NativeFunction> GetStringFunctionRegistry() {
                      "convert_replace_invalid_fromUTF8_binary",
                      NativeFunction::kNeedsContext),
 
+      NativeFunction("convert_toDOUBLE", {"convert_todouble"}, DataTypeVector{float64()},
+                     binary(), kResultNullIfNull, "convert_toDOUBLE_binary",
+                     NativeFunction::kNeedsContext),
+
+      NativeFunction("convert_toDOUBLE_BE", {"convert_todouble_be"},

Review comment:
       function name should not be case sensitive..so no need to having same name in lower case as alias

##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -1310,6 +1311,196 @@ const char* convert_replace_invalid_fromUTF8_binary(int64_t context, const char*
   return ret;
 }
 
+FORCE_INLINE
+const char* convert_toDOUBLE_binary(int64_t context, double value, int32_t* out_len) {
+  *out_len = sizeof(value);
+  char* ret = reinterpret_cast<char*>(gdv_fn_context_arena_malloc(context, *out_len));
+
+  if (ret == nullptr) {
+    gdv_fn_context_set_error_msg(context,
+                                 "Could not allocate memory for the output string");
+
+    *out_len = 0;
+    return "";
+  }
+
+  memcpy(ret, &value, *out_len);
+
+  return ret;
+}
+
+FORCE_INLINE
+const char* convert_toDOUBLE_binary_be(int64_t context, double value, int32_t* out_len) {
+  // The function behaves like convert_toDOUBLE_binary, but always return the result
+  // in big endian format
+  char* ret = const_cast<char*>(convert_toDOUBLE_binary(context, value, out_len));
+
+#if ARROW_LITTLE_ENDIAN
+  std::reverse(ret, ret + *out_len);

Review comment:
       lets not use c++ stdlib functions in precompiled.




-- 
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] anthonylouisbsb commented on a change in pull request #9861: ARROW-12166: [C++][Gandiva] Implements CONVERT_TO(value, type) function

Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on a change in pull request #9861:
URL: https://github.com/apache/arrow/pull/9861#discussion_r623300302



##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -1310,6 +1311,196 @@ const char* convert_replace_invalid_fromUTF8_binary(int64_t context, const char*
   return ret;
 }
 
+FORCE_INLINE
+const char* convert_toDOUBLE_binary(int64_t context, double value, int32_t* out_len) {
+  *out_len = sizeof(value);
+  char* ret = reinterpret_cast<char*>(gdv_fn_context_arena_malloc(context, *out_len));
+
+  if (ret == nullptr) {
+    gdv_fn_context_set_error_msg(context,
+                                 "Could not allocate memory for the output string");
+
+    *out_len = 0;
+    return "";
+  }
+
+  memcpy(ret, &value, *out_len);
+
+  return ret;
+}
+
+FORCE_INLINE
+const char* convert_toDOUBLE_binary_be(int64_t context, double value, int32_t* out_len) {
+  // The function behaves like convert_toDOUBLE_binary, but always return the result
+  // in big endian format
+  char* ret = const_cast<char*>(convert_toDOUBLE_binary(context, value, out_len));
+
+#if ARROW_LITTLE_ENDIAN
+  std::reverse(ret, ret + *out_len);

Review comment:
       Added a method that do this processing of reverse the buffer




-- 
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] anthonylouisbsb commented on a change in pull request #9861: ARROW-12166: [C++][Gandiva] Implements CONVERT_TO(value, type) function

Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on a change in pull request #9861:
URL: https://github.com/apache/arrow/pull/9861#discussion_r609879378



##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -1246,6 +1246,69 @@ const char* convert_fromUTF8_binary(gdv_int64 context, const char* bin_in, gdv_i
   return ret;
 }
 
+#define CONVERT_TO_FN(TYPE)                                                              \
+  FORCE_INLINE                                                                           \
+  const char* convert_to##TYPE##_binary(gdv_int64 context, gdv_##TYPE value,             \
+                                        gdv_int32* out_len) {                            \
+    *out_len = sizeof(value);                                                            \
+    char* ret = reinterpret_cast<char*>(gdv_fn_context_arena_malloc(context, *out_len)); \
+    if (ret == nullptr) {                                                                \
+      gdv_fn_context_set_error_msg(context,                                              \
+                                   "Could not allocate memory for output string");       \
+      *out_len = 0;                                                                      \
+      return "";                                                                         \
+    }                                                                                    \
+    memcpy(ret, &value, *out_len);                                                       \
+    return ret;                                                                          \
+  }
+
+// Expand inner macro for all numeric types.
+#define NUMERIC_BOOL_DATE_TYPES(INNER) \
+  INNER(int8)                          \
+  INNER(int16)                         \
+  INNER(int32)                         \
+  INNER(int64)                         \
+  INNER(uint8)                         \
+  INNER(uint16)                        \
+  INNER(uint32)                        \
+  INNER(uint64)                        \
+  INNER(float32)                       \
+  INNER(float64)                       \
+  INNER(boolean)                       \
+  INNER(date64)                        \
+  INNER(date32)                        \
+  INNER(time32)                        \
+  INNER(timestamp)
+
+NUMERIC_BOOL_DATE_TYPES(CONVERT_TO_FN)
+
+#undef NUMERIC_BOOL_DATE_TYPES
+#undef CONVERT_TO_FN
+
+#define CONVERT_TO_FN_BUF_OP(TYPE)                                                       \
+  FORCE_INLINE                                                                           \
+  const char* convert_to##TYPE##_binary(gdv_int64 context, const char* value,            \
+                                        gdv_int32 value_len, gdv_int32* out_len) {       \
+    *out_len = value_len;                                                                \
+    char* ret = reinterpret_cast<char*>(gdv_fn_context_arena_malloc(context, *out_len)); \

Review comment:
       Changes applied




-- 
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 #9861: ARROW-12166: [C++][Gandiva] Implements CONVERT_TO(value, type) function

Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #9861:
URL: https://github.com/apache/arrow/pull/9861#discussion_r623600134



##########
File path: cpp/src/gandiva/function_registry_string.cc
##########
@@ -225,6 +225,67 @@ std::vector<NativeFunction> GetStringFunctionRegistry() {
                      "convert_replace_invalid_fromUTF8_binary",
                      NativeFunction::kNeedsContext),
 
+      NativeFunction("convert_todouble", {}, DataTypeVector{float64()}, binary(),

Review comment:
       nit: lets keep the names as "convert_toDOUBLE", reads better




-- 
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] anthonylouisbsb commented on a change in pull request #9861: ARROW-12166: [C++][Gandiva] Implements CONVERT_TO(value, type) function

Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on a change in pull request #9861:
URL: https://github.com/apache/arrow/pull/9861#discussion_r623612726



##########
File path: cpp/src/gandiva/function_registry_string.cc
##########
@@ -225,6 +225,67 @@ std::vector<NativeFunction> GetStringFunctionRegistry() {
                      "convert_replace_invalid_fromUTF8_binary",
                      NativeFunction::kNeedsContext),
 
+      NativeFunction("convert_todouble", {}, DataTypeVector{float64()}, binary(),

Review comment:
       Ok, I understand now. I applied your suggestion.




-- 
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] anthonylouisbsb commented on a change in pull request #9861: ARROW-12166: [C++][Gandiva] Implements CONVERT_TO(value, type) function

Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on a change in pull request #9861:
URL: https://github.com/apache/arrow/pull/9861#discussion_r609942643



##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -1246,6 +1246,69 @@ const char* convert_fromUTF8_binary(gdv_int64 context, const char* bin_in, gdv_i
   return ret;
 }
 
+#define CONVERT_TO_FN(TYPE)                                                              \

Review comment:
       I add a function that convert the value in the reversed way from the native architecture. If the arrow is using little endian, it will convert using big-endian and if is using big-endian, it will convert using little-endian




-- 
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] anthonylouisbsb commented on a change in pull request #9861: ARROW-12166: [C++][Gandiva] Implements CONVERT_TO(value, type) function

Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on a change in pull request #9861:
URL: https://github.com/apache/arrow/pull/9861#discussion_r609880890



##########
File path: cpp/src/gandiva/precompiled/decimal_wrapper.cc
##########
@@ -430,4 +430,18 @@ char* castVARCHAR_decimal128_int64(int64_t context, int64_t x_high, uint64_t x_l
   return dec_str;
 }
 
+FORCE_INLINE
+const char* convert_todecimal128_binary(int64_t context, int64_t x_high, uint64_t x_low,
+                                        int32_t x_precision, int32_t x_scale,
+                                        int32_t* out_length) {
+  char* dec_str = gdv_fn_dec_to_string(context, x_high, x_low, x_scale, out_length);

Review comment:
       changes applied




-- 
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] anthonylouisbsb commented on a change in pull request #9861: ARROW-12166: [C++][Gandiva] Implements CONVERT_TO(value, type) function

Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on a change in pull request #9861:
URL: https://github.com/apache/arrow/pull/9861#discussion_r623284558



##########
File path: cpp/src/gandiva/function_registry_string.cc
##########
@@ -209,6 +209,71 @@ std::vector<NativeFunction> GetStringFunctionRegistry() {
                      "convert_replace_invalid_fromUTF8_binary",
                      NativeFunction::kNeedsContext),
 
+      NativeFunction("convert_toDOUBLE", {"convert_todouble"}, DataTypeVector{float64()},
+                     binary(), kResultNullIfNull, "convert_toDOUBLE_binary",
+                     NativeFunction::kNeedsContext),
+
+      NativeFunction("convert_toDOUBLE_BE", {"convert_todouble_be"},

Review comment:
       Changes applied and aliases removed




-- 
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] anthonylouisbsb commented on a change in pull request #9861: ARROW-12166: [C++][Gandiva] Implements CONVERT_TO(value, type) function

Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on a change in pull request #9861:
URL: https://github.com/apache/arrow/pull/9861#discussion_r623616510



##########
File path: cpp/src/gandiva/function_registry_string.cc
##########
@@ -225,6 +225,67 @@ std::vector<NativeFunction> GetStringFunctionRegistry() {
                      "convert_replace_invalid_fromUTF8_binary",
                      NativeFunction::kNeedsContext),
 
+      NativeFunction("convert_todouble", {}, DataTypeVector{float64()}, binary(),

Review comment:
       Suggestions applied and I checked and there's no part that assumes that names are case-sensitive




-- 
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] anthonylouisbsb commented on a change in pull request #9861: ARROW-12166: [C++][Gandiva] Implements CONVERT_TO(value, type) function

Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on a change in pull request #9861:
URL: https://github.com/apache/arrow/pull/9861#discussion_r609882288



##########
File path: cpp/src/gandiva/precompiled/decimal_wrapper.cc
##########
@@ -430,4 +430,18 @@ char* castVARCHAR_decimal128_int64(int64_t context, int64_t x_high, uint64_t x_l
   return dec_str;
 }
 
+FORCE_INLINE
+const char* convert_todecimal128_binary(int64_t context, int64_t x_high, uint64_t x_low,
+                                        int32_t x_precision, int32_t x_scale,
+                                        int32_t* out_length) {
+  char* dec_str = gdv_fn_dec_to_string(context, x_high, x_low, x_scale, out_length);

Review comment:
       And I think that you mean 128 bits(16 bytes), not 128 bytes, 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] praveenbingo closed pull request #9861: ARROW-12166: [C++][Gandiva] Implements CONVERT_TO(value, type) function

Posted by GitBox <gi...@apache.org>.
praveenbingo closed pull request #9861:
URL: https://github.com/apache/arrow/pull/9861


   


-- 
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 #9861: ARROW-12166: [C++][Gandiva] Implements CONVERT_TO(value, type) function

Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #9861:
URL: https://github.com/apache/arrow/pull/9861#discussion_r623607724



##########
File path: cpp/src/gandiva/function_registry_string.cc
##########
@@ -225,6 +225,67 @@ std::vector<NativeFunction> GetStringFunctionRegistry() {
                      "convert_replace_invalid_fromUTF8_binary",
                      NativeFunction::kNeedsContext),
 
+      NativeFunction("convert_todouble", {}, DataTypeVector{float64()}, binary(),

Review comment:
       I said function_names are not case-sensitive. So you don't need to have both upper case and lower case variants, only one would do.




-- 
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] anthonylouisbsb commented on a change in pull request #9861: ARROW-12166: [C++][Gandiva] Implements CONVERT_TO(value, type) function

Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on a change in pull request #9861:
URL: https://github.com/apache/arrow/pull/9861#discussion_r623606472



##########
File path: cpp/src/gandiva/function_registry_string.cc
##########
@@ -225,6 +225,67 @@ std::vector<NativeFunction> GetStringFunctionRegistry() {
                      "convert_replace_invalid_fromUTF8_binary",
                      NativeFunction::kNeedsContext),
 
+      NativeFunction("convert_todouble", {}, DataTypeVector{float64()}, binary(),

Review comment:
       Should I need to apply that sugestion to other functions too? I am asking it because in this comment https://github.com/apache/arrow/pull/9861#discussion_r622908353 you said to not use case-sensitive names




-- 
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] anthonylouisbsb commented on a change in pull request #9861: ARROW-12166: [C++][Gandiva] Implements CONVERT_TO(value, type) function

Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on a change in pull request #9861:
URL: https://github.com/apache/arrow/pull/9861#discussion_r623284804



##########
File path: cpp/src/gandiva/function_registry_string.cc
##########
@@ -209,6 +209,71 @@ std::vector<NativeFunction> GetStringFunctionRegistry() {
                      "convert_replace_invalid_fromUTF8_binary",
                      NativeFunction::kNeedsContext),
 
+      NativeFunction("convert_toDOUBLE", {"convert_todouble"}, DataTypeVector{float64()},
+                     binary(), kResultNullIfNull, "convert_toDOUBLE_binary",
+                     NativeFunction::kNeedsContext),
+
+      NativeFunction("convert_toDOUBLE_BE", {"convert_todouble_be"},
+                     DataTypeVector{float64()}, binary(), kResultNullIfNull,
+                     "convert_toDOUBLE_binary_be", NativeFunction::kNeedsContext),
+
+      NativeFunction("convert_toFLOAT", {"convert_tofloat"}, DataTypeVector{float32()},
+                     binary(), kResultNullIfNull, "convert_toFLOAT_binary",

Review comment:
       I removed the binary suffix from the functions names




-- 
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 #9861: ARROW-12166: [C++][Gandiva] Implements CONVERT_TO(value, type) function

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9861:
URL: https://github.com/apache/arrow/pull/9861#issuecomment-811126868


   https://issues.apache.org/jira/browse/ARROW-12166


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