[GitHub] [arrow-datafusion] Dandandan commented on a diff in pull request #2183: MINOR: handle `NULL` in advance to avoid value copy in `string_concat`

[GitBox <gi...@apache.org>]

Dandandan commented on code in PR #2183:
URL: https://github.com/apache/arrow-datafusion/pull/2183#discussion_r846838079


##########
datafusion/physical-expr/src/expressions/binary.rs:
##########
@@ -430,17 +431,17 @@ fn string_concat(left: ArrayRef, right: ArrayRef) -> Result<ArrayRef> {
         scalar_value => scalar_value.into_array(left.clone().len()),
     };
     let ignore_null_array = ignore_null.as_any().downcast_ref::<StringArray>().unwrap();
-    let result = (0..ignore_null_array.len())
+    let index_array = (0..ignore_null_array.len())
         .into_iter()
         .map(|index| {
             if left.is_null(index) || right.is_null(index) {
                 None
             } else {
-                Some(ignore_null_array.value(index))
+                Some(index as u32)
             }
         })
-        .collect::<StringArray>();
-
+        .collect::<UInt32Array>();
+    let result = take(ignore_null_array, &index_array, None)?;

Review Comment:
   AIUC, the suggestion of @alamb might be roughly:
   
   
   * Create the validity array based on `left_bitmap && right_bitmap` (this is an fast operation, as it can be performed on 64 bits at once). A fast path could be implemented for the null count being equal to 0 or being equal to length of the array.
   
   * Compute the concatenation based on left + right side (without needing taking into account null values).
   
   We should add some benches to see the improvements this brings.
   
   In the arrow-rs kernels you could find some examples of this strategy implemented, but usually this brings 2-10x improvements, depending on data type.
   
   In arrow2 this strategy is applied quite a bit more consistently, see for example the `substring` kernel: https://github.com/jorgecarleitao/arrow2/blob/main/src/compute/substring.rs#L66



-- 
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: github-unsubscribe@arrow.apache.org

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