Skip to content

Conversation

@comphead
Copy link
Contributor

Which issue does this PR close?

Rationale for this change

  • Error occurred in BooleanArray::new() when comparing nested arrays (arrays of arrays)
  • Root cause: Null buffer length mismatch in scalar-to-array comparisons

What changes are included in this PR?

  • Fixed Length Mismatch: Null buffers now correctly match result array length
  • Proper Null Propagation: Null scalars correctly propagate to entire result
  • Combined Cases: (false, false) | (true, true) use same logic
  • Clear Intent: filter(|nulls| !nulls.is_valid(0)) clearly shows null scalar check

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt) labels Aug 21, 2025
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @comphead -- this fix makes sense to me. I have some small test suggestion but otherwise this looks good to me

CREATE EXTERNAL TABLE array_has STORED AS PARQUET location 'test_files/scratch/array/array_has/single_file.parquet';

query B
select array_contains(a, b) from array_has;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This panics for me locally without the fix:

> select array_contains(a, b) from '/tmp/t.parquet';

thread 'main' panicked at /Users/andrewlamb/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/arrow-array-56.0.0/src/array/boolean_array.rs:91:13:
assertion `left == right` failed
  left: 2
 right: 1
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend we also test with array_contains (b, a) as well to covert the other argument order

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @alamb I'm not sure I'm following, the reverse order would violate array_has function signature.

Error during planning: Failed to coerce arguments to satisfy a call to 'array_has' function: coercion from [List(Field { name: "item", data_type: Boolean, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: {} }), List(Field { name: "item", data_type: List(Field { name: "item", data_type: Boolean, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: {} }), nullable: true, dict_id: 0, dict_is_ordered: false, metadata: {} })] to the signature ArraySignature(Array { arguments: [Array, Element], array_coercion: Some(FixedSizedListToList) }) failed No function matches the given name and argument types 'array_has(List(Field { name: "item", data_type: Boolean, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: {} }), List(Field { name: "item", data_type: List(Field { name: "item", data_type: Boolean, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: {} }), nullable: true, dict_id: 0, dict_is_ordered: false, metadata: {} }))'. You might need to add explicit type casts.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry -- I was just noticing in the code that it has two code paths when left and right are constants, but this query only exercises one of the paths, I think

@comphead comphead merged commit 02a7472 into apache:main Aug 21, 2025
27 checks passed
comphead added a commit to comphead/arrow-datafusion that referenced this pull request Aug 21, 2025
* fix: align `array_has` null buffer for scalar
alamb pushed a commit that referenced this pull request Aug 21, 2025
* fix: align `array_has` null buffer for scalar (#17272)

* fix: align `array_has` null buffer for scalar

* merge
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: array_has fails on null buffer union

2 participants