Skip to content

fix: [branch-1.2] compare every row when arrays_overlap gets a scalar beside a column (#6743) - #7512

Open
andygrove wants to merge 1 commit into
apache:branch-1.2from
andygrove:backport-6743-branch-1.2
Open

andygrove wants to merge 1 commit into
apache:branch-1.2from
andygrove:backport-6743-branch-1.2

Conversation

@andygrove

Copy link
Copy Markdown
Member

Which issue does this PR close?

Backport of #6743 to branch-1.2. Closes #6742 on this branch.

Rationale for this change

When one argument of arrays_overlap is a scalar and the other is an array column, the native kernel returns row 0's result for the whole batch, or panics when the constant is the second argument (#6742, a correctness bug). branch-1.2 has the same kernel. See #6743 for the full analysis.

What changes are included in this PR?

Cherry-pick of c39eb71 (git cherry-pick -x) with no conflicts and no adaptations. The diff against the source commit is identical apart from line numbers and blob hashes. SparkArraysOverlap keeps a scalar argument as a one-row list that every output row reads, and returns an array result when the other argument is an array.

Other release branches: branch-1.2 is the newest release branch, so there is no newer one to port to. Whether branch-1.1 needs a backport is not part of this PR.

How are these changes tested?

On this branch, with JDK 17: the 30 arrays_overlap Rust unit tests in datafusion-comet-spark-expr pass, including the new ones, and CometSqlFileTestSuite expressions/array/arrays_overlap.sql passes against a native library built from this branch. The same SQL test fails against the branch-1.2 native library without the fix, so the branch has the bug and the test catches it. I ran the default Spark profile only.

…umn (apache#6743)

* fix: compare every row when arrays_overlap gets a scalar beside a column

When one argument was a scalar, SparkArraysOverlap turned it into a
one-row list while the column kept its full length, and returned row 0's
result as a scalar. A constant array beside an array column therefore
answered every row with the first row's result, or panicked with an
index out of bounds when the constant was the second argument.

Keep the scalar as a one-row list that every output row reads, and return
an array result when the other argument is an array. Repeating the scalar
to the batch length instead could overflow the list's 32-bit offsets for
a long constant. Two scalar arguments still produce a scalar result.

Assisted-by: Claude Code (claude-opus-5-5)

* test: reuse the arrays_overlap fixture tables for the scalar case

Write test_arrays_overlap and test_overlap_struct as one file each, so
their rows share a batch. The existing constant-second queries then
cover the scalar-beside-column path, and a constant-first query with
answers that change by row, plus a struct query in both orders, complete
it. This replaces the separate test_overlap_batch table and the flat and
struct Rust tests it duplicated. The long-constant and two-scalar Rust
tests stay, since a fixture cannot reach them.

Assisted-by: Claude Code (claude-opus-5-5)
(cherry picked from commit c39eb71)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:expressions Expression evaluation bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants