sunchao opened a new pull request, #5470:
URL: https://github.com/apache/datafusion-comet/pull/5470
## Which issue does this PR close?
No issue is automatically closed. This is a standalone aggregate identity
fix with a regression added below.
## Rationale for this change
Different aggregate result projections can currently have the same Comet
plan identity. For example, an exchange above `COUNT(*) + 1` can be considered
reusable for `COUNT(*) - 1`, even though the outputs differ. Distinct
aggregates also need Spark's original result attributes to canonicalize
equivalent plans correctly.
## What changes are included in this PR?
- Preserve Spark's original `aggregateAttributes` in Comet hash and
object-hash aggregate conversion.
- Include those attributes in canonicalization and produced attributes, and
include aggregate attributes and result expressions in equality and hashing.
- Add four cases to `CometAggregateSuite`: count, distinct count, distinct
count plus sum, and collect-set size. They distinguish different result
projections while requiring equivalent projections with fresh aliases/IDs to
remain reusable.
Only the aggregate implementation and its existing test suite change. No
join feature, native operator, or dependency changes are included.
## How are these changes tested?
**Draft: JVM validation is blocked, so no passing JVM test result is
claimed.**
- Built the native library from public main `5baa6b03`; a subsequent locked
build also passed.
- Spotless checks on the final public patch and `git diff --check` passed.
- Attempted the focused public Spark 4.0 test run below. Maven dependency
resolution stalled on a configured transitive repository before Spark
compilation or these tests ran. Repository configuration was left unchanged.
```sh
./mvnw test -Pspark-4.0 -Dtest=none \
-Dsuites=org.apache.comet.exec.CometAggregateSuite \
'-Dtests=aggregate canonicalization preserves result expressions and
equivalent reuse'
```
The new regressions and affected aggregate tests still need runtime
validation before this draft is ready to merge.
--
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]
