vvellanki commented on a change in pull request #11166:
URL: https://github.com/apache/arrow/pull/11166#discussion_r741663325



##########
File path: cpp/src/gandiva/function_registry_string.cc
##########
@@ -88,6 +88,10 @@ std::vector<NativeFunction> GetStringFunctionRegistry() {
                      "gdv_fn_initcap_utf8",
                      NativeFunction::kNeedsContext | 
NativeFunction::kCanReturnErrors),
 
+      NativeFunction("elt", {}, DataTypeVector{int32(), utf8()}, utf8(),
+                     kResultNullIfNull, "gdv_fn_elt_utf8",
+                     NativeFunction::kNeedsContext | 
NativeFunction::kCanReturnErrors),

Review comment:
       Why does this need the flags kNeedsContext and kCanReturnErrors?

##########
File path: cpp/src/gandiva/gdv_function_stubs.cc
##########
@@ -794,6 +795,54 @@ const char* gdv_fn_initcap_utf8(int64_t context, const 
char* data, int32_t data_
   *out_len = out_idx;
   return out;
 }
+
+GANDIVA_EXPORT
+const char* gdv_fn_elt_utf8(int64_t context, int32_t pos, const char* data,

Review comment:
       elt is a function with variable number of arguments - there is no 
expectation for the arguments to be concatenated using a ',' separator. This 
code assumes that
   
   Please implement elt(int, string1, string2) - elt that works on 2 string 
arguments and
   elt(int, string1, string2, string3) and elt(int, string1, string2, string3, 
string4) and elt(int, string1, string2, string3, string4, string5)
   
   If the query has more arguments, the query needs to be modified




-- 
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: [email protected]

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


Reply via email to