alamb commented on code in PR #12097:
URL: https://github.com/apache/datafusion/pull/12097#discussion_r1729194704


##########
datafusion/functions-nested/src/array_has.rs:
##########
@@ -251,75 +237,176 @@ impl ScalarUDFImpl for ArrayHasAny {
 }
 
 /// Represents the type of comparison for array_has.
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, Clone, Copy)]
 enum ComparisonType {
     // array_has_all
     All,
     // array_has_any
     Any,
-    // array_has
-    Single,
 }
 
-fn general_array_has_dispatch<O: OffsetSizeTrait>(
+fn array_has_dispatch<O: OffsetSizeTrait>(
+    haystack: &ArrayRef,
+    needle: &ArrayRef,
+) -> Result<ArrayRef> {
+    let haystack = as_generic_list_array::<O>(haystack)?;
+    match needle.data_type() {
+        DataType::Utf8 | DataType::LargeUtf8 | DataType::Utf8View => {
+            array_has_string_internal::<O>(haystack, needle)
+        }
+        _ => general_array_has::<O>(haystack, needle),
+    }
+}
+
+fn array_has_all_and_any_dispatch<O: OffsetSizeTrait>(
     haystack: &ArrayRef,
     needle: &ArrayRef,
     comparison_type: ComparisonType,
 ) -> Result<ArrayRef> {
-    let array = if comparison_type == ComparisonType::Single {
-        let arr = as_generic_list_array::<O>(haystack)?;
-        check_datatypes("array_has", &[arr.values(), needle])?;
-        arr
-    } else {
-        check_datatypes("array_has", &[haystack, needle])?;
-        as_generic_list_array::<O>(haystack)?
-    };
+    let haystack = as_generic_list_array::<O>(haystack)?;
+    let needle = as_generic_list_array::<O>(needle)?;
+    match needle.data_type() {
+        DataType::Utf8 | DataType::LargeUtf8 | DataType::Utf8View => {
+            array_has_all_and_any_string_internal::<O>(haystack, needle, 
comparison_type)
+        }
+        _ => general_array_has_for_all_and_any::<O>(haystack, needle, 
comparison_type),
+    }
+}
 
-    let mut boolean_builder = BooleanArray::builder(array.len());
+fn array_has_string_internal<O: OffsetSizeTrait>(
+    haystack: &GenericListArray<O>,
+    needle: &ArrayRef,
+) -> Result<ArrayRef> {
+    let mut boolean_builder = BooleanArray::builder(haystack.len());
+    for (arr, element) in 
haystack.iter().zip(string_array_to_vec(needle).into_iter()) {
+        match (arr, element) {
+            (Some(arr), Some(element)) => {
+                boolean_builder.append_value(
+                    string_array_to_vec(&arr)

Review Comment:
   Thanks @jayzhan211  -- when reviewing  
https://github.com/apache/datafusion/pull/12125  and re-reading 
https://github.com/apache/datafusion/issues/12062, I wonder if what would 
really be useful is making specialized implementations when the second argument 
is a constant?
   
   Spefically, what I think what will happen now is `array_has(col, 'foo')` 
will make a `StringArray` that has the string `'foo'` repeated many times
   
   I wonder how much better implementing a special implementation for 
`array_has(array: ArrayRef, val: ScalarValue)` would be? I bet *much* faster



-- 
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-unsubscr...@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to