Support `LargeList` in `array_has` simplification to `InList` #17732

Jefffrey · 2025-09-22T23:50:25Z

Which issue does this PR close?

Closes Perf: simplify LargeList constant haystack in array_has to InList expr #15389

Rationale for this change

Already works for regular List (and FixedSizeList technically, since it gets coerced to List). Extending this support to LargeLists.

What changes are included in this PR?

Uplift ScalarValue::convert_array_to_scalar_vec to be generic across both List & LargeList and enable this simplification for LargeLists inside array_has.

Also, fix ScalarValue::convert_array_to_scalar_vec to properly account for nulls in nested arrays.

Are these changes tested?

Fixed existing tests to have this new behaviour.

Are there any user-facing changes?

No.

Jefffrey · 2025-09-22T23:53:24Z

datafusion/common/src/scalar/mod.rs

-        let mut scalars = Vec::with_capacity(array.len());
-
-        for index in 0..array.len() {
-            let nested_array = array.as_list::<i32>().value(index);
-            let scalar_values = (0..nested_array.len())
-                .map(|i| ScalarValue::try_from_array(&nested_array, i))
-                .collect::<Result<Vec<_>>>()?;
-            scalars.push(scalar_values);
+        fn generic_collect<OffsetSize: OffsetSizeTrait>(
+            array: &dyn Array,
+        ) -> Result<Vec<Vec<ScalarValue>>> {
+            array
+                .as_list::<OffsetSize>()
+                .iter()
+                .map(|nested_array| match nested_array {
+                    Some(nested_array) => (0..nested_array.len())
+                        .map(|i| ScalarValue::try_from_array(&nested_array, i))
+                        .collect::<Result<Vec<_>>>(),
+                    // TODO: what can we put for null?
+                    None => Ok(vec![]),
+                })
+                .collect()


Because it used to do array.as_list::<i32>().value(index), this never checked for nulls before.

So in the test case I added below:

// Funky (null slot has non-zero list offsets) // Offsets + Values looks like this: [[1, 2], [3, 4], [5]] // But with NullBuffer it's like this: [[1, 2], NULL, [5]] let funky = ListArray::new( Field::new_list_field(DataType::Int64, true).into(), OffsetBuffer::new(vec![0, 2, 4, 5].into()), Arc::new(Int64Array::from(vec![1, 2, 3, 4, 5, 6])), Some(NullBuffer::from(vec![true, false, true])), ); let converted = ScalarValue::convert_array_to_scalar_vec(&funky).unwrap(); assert_eq!( converted, vec![ vec![ScalarValue::Int64(Some(1)), ScalarValue::Int64(Some(2))], vec![], vec![ScalarValue::Int64(Some(5))], ] );

For the output, it incorrect would have vec![3, 4] instead of the empty Vec (it's accessing that element as if it were a valid list when in fact it was null in the parent list).

For now I made nulls return empty list to not change the signature of the method, though this seems undesirable 🤔

I think we would have to research what the null semantics of array_has are in other implementations (postgres/duckdb/spark) and try and make this one consistent

Maybe we can file a ticket / leave a reference to that ticket in the comments 🤔

Raised #17749 👍

@alamb By null semantics what do you mean exactly? Do you mean what this returns? select array_contains([NULL], 1)

If so, DuckDB returns false:

D select list_contains([NULL], 1); ┌─────────────────────────────────────────┐ │ list_contains(main.list_value(NULL), 1) │ │ boolean │ ├─────────────────────────────────────────┤ │ false │ └─────────────────────────────────────────┘

Spark returns null:

Postgres fails (probably I'm doing something wrong):

SELECT 1 = ANY(ARRAY[null]); ERROR: 42883: operator does not exist: integer = text LINE 1: SELECT 1 = ANY(ARRAY[null]); ^ HINT: No operator matches the given name and argument types. You might need to add explicit type casts. LOCATION: op_error, parse_oper.c:656

Or the other way around: select array_contains(array[1], null)

DuckDB returns null

D select list_contains([1], null); ┌─────────────────────────────────────────┐ │ list_contains(main.list_value(1), NULL) │ │ boolean │ ├─────────────────────────────────────────┤ │ NULL │ └─────────────────────────────────────────┘

Spark errors due to datatype mismatch.

Postgres returns null:

SELECT null = ANY(ARRAY[1,2,3]); -- true ?column? ---------- ¤

I've made an attempt at fixing this here #17891, but I'm not sure about the null semantics. I should add a new test case for array_has, but haven't come up with one yet.

alamb

Thanks @Jefffrey

alamb · 2025-09-23T17:50:04Z

datafusion/sqllogictest/test_files/array.slt

 05)--------ProjectionExec: expr=[]
 06)----------CoalesceBatchesExec: target_batch_size=8192
-07)------------FilterExec: array_has([7f4b18de3cfeb9b4ac78c381ee2ad278, a, b, c], substr(md5(CAST(value@0 AS Utf8View)), 1, 32))
+07)------------FilterExec: substr(md5(CAST(value@0 AS Utf8View)), 1, 32) IN ([Literal { value: Utf8View("7f4b18de3cfeb9b4ac78c381ee2ad278"), field: Field { name: "lit", data_type: Utf8View, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} } }, Literal { value: Utf8View("a"), field: Field { name: "lit", data_type: Utf8View, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} } }, Literal { value: Utf8View("b"), field: Field { name: "lit", data_type: Utf8View, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} } }, Literal { value: Utf8View("c"), field: Field { name: "lit", data_type: Utf8View, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} } }])


this is a pretty terrible plan display (not made worse by this PR, of course)

alamb · 2025-09-23T17:51:14Z

datafusion/common/src/scalar/mod.rs

-        let mut scalars = Vec::with_capacity(array.len());
-
-        for index in 0..array.len() {
-            let nested_array = array.as_list::<i32>().value(index);
-            let scalar_values = (0..nested_array.len())
-                .map(|i| ScalarValue::try_from_array(&nested_array, i))
-                .collect::<Result<Vec<_>>>()?;
-            scalars.push(scalar_values);
+        fn generic_collect<OffsetSize: OffsetSizeTrait>(
+            array: &dyn Array,
+        ) -> Result<Vec<Vec<ScalarValue>>> {
+            array
+                .as_list::<OffsetSize>()
+                .iter()
+                .map(|nested_array| match nested_array {
+                    Some(nested_array) => (0..nested_array.len())
+                        .map(|i| ScalarValue::try_from_array(&nested_array, i))
+                        .collect::<Result<Vec<_>>>(),
+                    // TODO: what can we put for null?
+                    None => Ok(vec![]),
+                })
+                .collect()


I think we would have to research what the null semantics of array_has are in other implementations (postgres/duckdb/spark) and try and make this one consistent

alamb · 2025-09-23T17:51:54Z

datafusion/common/src/scalar/mod.rs

-        let mut scalars = Vec::with_capacity(array.len());
-
-        for index in 0..array.len() {
-            let nested_array = array.as_list::<i32>().value(index);
-            let scalar_values = (0..nested_array.len())
-                .map(|i| ScalarValue::try_from_array(&nested_array, i))
-                .collect::<Result<Vec<_>>>()?;
-            scalars.push(scalar_values);
+        fn generic_collect<OffsetSize: OffsetSizeTrait>(
+            array: &dyn Array,
+        ) -> Result<Vec<Vec<ScalarValue>>> {
+            array
+                .as_list::<OffsetSize>()
+                .iter()
+                .map(|nested_array| match nested_array {
+                    Some(nested_array) => (0..nested_array.len())
+                        .map(|i| ScalarValue::try_from_array(&nested_array, i))
+                        .collect::<Result<Vec<_>>>(),
+                    // TODO: what can we put for null?
+                    None => Ok(vec![]),
+                })
+                .collect()


Maybe we can file a ticket / leave a reference to that ticket in the comments 🤔

alamb · 2025-09-23T17:52:43Z

datafusion/functions-nested/src/array_has.rs

        // if the haystack is a constant list, we can use an inlist expression which is more
        // efficient because the haystack is not varying per-row
-        if let Expr::Literal(ScalarValue::List(array), _) = haystack {
-            // TODO: support LargeList


alamb · 2025-09-23T17:57:02Z

datafusion/functions-nested/src/array_has.rs

+                        .map(|v| Expr::Literal(v, None))
+                        .collect();
+
+                    return Ok(ExprSimplifyResult::Simplified(Expr::InList(InList {


You could potentially make this more concise using

return Ok(ExprSimplifyResult::Simplified(in_list(std::mem::take(needle), list, false)));

Thanks, applied the simplification 👍

alamb · 2025-09-24T10:44:43Z

Thanks again @Jefffrey

Support LargeList in array_has simplification to InList

5ade566

github-actions bot added sqllogictest SQL Logic Tests (.slt) common Related to common crate labels Sep 22, 2025

Jefffrey commented Sep 22, 2025

View reviewed changes

Jefffrey marked this pull request as ready for review September 23, 2025 00:21

alamb approved these changes Sep 23, 2025

View reviewed changes

Merge branch 'main' into array_has_inlist_largelist

000d779

Jefffrey mentioned this pull request Sep 24, 2025

ScalarValue::convert_array_to_scalar_vec doesn't respect null list elements #17749

Open

refactoring

0d1c6e1

alamb added this pull request to the merge queue Sep 24, 2025

Merged via the queue into apache:main with commit 6ec14e9 Sep 24, 2025
28 checks passed

Jefffrey deleted the array_has_inlist_largelist branch September 24, 2025 10:53

Jefffrey mentioned this pull request Oct 2, 2025

Improve InListExpr plan display #17884

Merged

Support LargeList in array_has simplification to InList #17732

Support LargeList in array_has simplification to InList #17732

Uh oh!

Conversation

Jefffrey commented Sep 22, 2025

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

Jefffrey Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vegarsti Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alamb commented Sep 24, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Support `LargeList` in `array_has` simplification to `InList` #17732

Support `LargeList` in `array_has` simplification to `InList` #17732

Jefffrey Sep 22, 2025 •

edited

Loading

vegarsti Oct 2, 2025 •

edited

Loading