vvellanki commented on code in PR #12803:
URL: https://github.com/apache/arrow/pull/12803#discussion_r848571206


##########
cpp/src/gandiva/precompiled/string_ops.cc:
##########
@@ -2395,19 +2395,22 @@ const char* byte_substr_binary_int32_int32(gdv_int64 
context, const char* text,
 
 FORCE_INLINE
 void concat_word(char* out_buf, int* out_idx, const char* in_buf, int in_len,
-                 bool in_validity, const char* separator, int separator_len) {
+                 bool in_validity, const char* separator, int separator_len,
+                 bool* seenAnyValidInput) {
   if (!in_validity) {
+    *seenAnyValidInput = false;

Review Comment:
   This is wrong. ws_concat("", null, "world", "-") should return "-world"
   
   This would return "world" because seenAnyValidInput is getting reset to 
false due to the 2nd arg that is null.
   
   The logic should be simple:
   if (!in_validity) {
     return;
   }
   
   if (seenAnyValidInput) {
     // copy the separator and update out_idx
   }
   
   // copy the input
   
   The caller will do the following:
   seenAnyValidInput = false;
   concat_word(..., in_validity, ...);
   seenAnyValidInput = seenAnyValidInput || in_validity;



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