[arrow-datafusion] branch master updated: MINOR: handle `NULL` in advance to avoid value copy in `string_concat` (#2183)

[al...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

alamb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git


The following commit(s) were added to refs/heads/master by this push:
     new 28a6da3d2 MINOR: handle `NULL` in advance to avoid value copy in `string_concat` (#2183)
28a6da3d2 is described below

commit 28a6da3d2d175eb9d2f4ff8a6ea58e7c22dae97c
Author: DuRipeng <45...@qq.com>
AuthorDate: Tue Apr 12 05:54:33 2022 +0800

    MINOR: handle `NULL` in advance to avoid value copy in `string_concat` (#2183)
    
    * string_concat use 'take' to avoid copying
    
    * simplify string concat process
    
    Co-authored-by: duripeng <du...@baidu.com>
---
 datafusion/physical-expr/src/expressions/binary.rs | 26 +++++++++-------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/datafusion/physical-expr/src/expressions/binary.rs b/datafusion/physical-expr/src/expressions/binary.rs
index 7cf460b6b..7b2e82952 100644
--- a/datafusion/physical-expr/src/expressions/binary.rs
+++ b/datafusion/physical-expr/src/expressions/binary.rs
@@ -56,7 +56,6 @@ use arrow::record_batch::RecordBatch;
 
 use crate::coercion_rule::binary_rule::coerce_types;
 use crate::expressions::try_cast;
-use crate::string_expressions;
 use crate::PhysicalExpr;
 use datafusion_common::ScalarValue;
 use datafusion_common::{DataFusionError, Result};
@@ -417,30 +416,25 @@ fn bitwise_or(left: ArrayRef, right: ArrayRef) -> Result<ArrayRef> {
     }
 }
 
-/// Use datafusion build-in expression `concat` to evaluate `StringConcat` operator.
-/// Besides, any `NULL` exists on lhs or rhs will come out result `NULL`
+/// Concat lhs and rhs String Array, any `NULL` exists on lhs or rhs will come out result `NULL`
 /// 1. 'a' || 'b' || 32 = 'ab32'
 /// 2. 'a' || NULL = NULL
 fn string_concat(left: ArrayRef, right: ArrayRef) -> Result<ArrayRef> {
-    let ignore_null = match string_expressions::concat(&[
-        ColumnarValue::Array(left.clone()),
-        ColumnarValue::Array(right.clone()),
-    ])? {
-        ColumnarValue::Array(array_ref) => array_ref,
-        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 left_array = left.as_any().downcast_ref::<StringArray>().unwrap();
+    let right_array = right.as_any().downcast_ref::<StringArray>().unwrap();
+    let result = (0..left.len())
         .into_iter()
-        .map(|index| {
-            if left.is_null(index) || right.is_null(index) {
+        .map(|i| {
+            if left.is_null(i) || right.is_null(i) {
                 None
             } else {
-                Some(ignore_null_array.value(index))
+                let mut owned_string: String = "".to_owned();
+                owned_string.push_str(left_array.value(i));
+                owned_string.push_str(right_array.value(i));
+                Some(owned_string)
             }
         })
         .collect::<StringArray>();
-
     Ok(Arc::new(result) as ArrayRef)
 }