andygrove opened a new pull request, #6449:
URL: https://github.com/apache/datafusion-comet/pull/6449
## Which issue does this PR close?
Closes #6424.
Found by the 1.1.0 regression audit (#6399), tracked in #6402.
## Rationale for this change
#5692 routes the `Invoke`/`StaticInvoke` predicate of a typed
`Dataset.filter(lambda)` through the
JVM codegen dispatcher instead of falling back to Spark. That predicate runs
in `CometFilter`, and
the dispatcher reads sliced boolean input wrong (#6288): once a native
aggregate's output batches
are sliced, the filter keeps or drops the wrong rows. This is a regression
from Comet 1.0.0, where
the filter ran in Spark.
#6339 fixes #6288 on `main` by zeroing a sliced boolean's offset at every
level, not only the top,
before an array crosses into the JVM. It is not on `branch-1.1`. I
cherry-picked it onto
1.1.0-rc1 to confirm: it applies cleanly, and the #6424 reproducer passes
with the predicate still
dispatched.
`branch-1.0` doesn't need this for #6424: #5692 isn't on `branch-1.0`, so
the dispatcher never sees
this predicate there. The underlying #6288 bug predates 1.0.0, but whether
`branch-1.0` should take
#6339 anyway is a separate decision.
## What changes are included in this PR?
Cherry-pick of aee5e06133ca8e2fc1b1337ca2ac8cf3cb0514f3 (#6339) from `main`,
with `-x`. The pick was
clean; the only difference from the source commit is the `(cherry picked
from ...)` trailer and the
line numbers in two hunks shifting by a few lines. No adaptations.
## How are these changes tested?
- Built every target: `cargo build`, `cargo fmt --check`, `cargo clippy
--all-targets --workspace
-- -D warnings`, `cargo check --all-targets` (all clean), and `./mvnw
test-compile`.
- Ran the tests #6339 added and confirmed they pass on this branch:
- Rust, `datafusion-comet-common` (`ffi_offsets` module, 7 tests) and
`datafusion-comet`
(`execution::utils::tests::test_move_to_spark_zeroes_nested_boolean_offsets`).
- Scala, `org.apache.comet.CometCodegenSuite` (3 tests) and
`org.apache.comet.exec.CometExecSuite` (3 tests), via `./mvnw test -Dtest=none
-Dsuites="<suite> sliced"`.
- As a scratch check (not part of this commit), I added a temporary suite
extending
`CometTestBase` with the exact reproducer from #6424 and ran it with the
backport applied; it
passed. I deleted the scratch file afterward.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
