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