sunchao opened a new pull request, #5421: URL: https://github.com/apache/datafusion-comet/pull/5421
## Why are the changes needed? Closes #5419. The existing unsafe-partial prepass checks aggregate expressions before child conversion is known. A final aggregate can pass that check and then fall back because its shuffle child is not native, leaving a native partial whose buffers are not declared safe for Spark consumption. This is observable with decimal AVG: filtering four scan partitions down to one matching row returns `NULL` instead of `200.000000`, with AQE either off or on. The unsafe partial must be reverted before AQE materializes its output. ## What changes were proposed in this PR? - Retain the early prepass, and also inspect actual conversion results for Spark final aggregates that do not support mixed execution. - Restore only the feeding partial/intermediate-aggregate/exchange chain, tag the partial to keep fallback sticky during stage-only reapplication, and rebuild consumers from their original Spark operators so native plans do not retain stale buffer schemas. - Respect materialized-stage/reused-exchange ownership boundaries. Preserve native work below the partial, supported mixed aggregates such as MIN/MAX, and fully native aggregate chains. - Add AQE-on/off result regressions and ordinary/distinct-chain rule tests; align existing plan expectations with the existing mixed-execution eligibility policy. The eligibility policy and strict fallback-reason checks are unchanged. The independent empty-AVG-state issue is tracked in #5418. This PR is tested with the unmodified native library and does not depend on that fix. ## How was this PR tested? - On public `main`, all **four** new targeted regressions failed; after this change, all **four passed**, with no aborted suites. - Full `CometAggregateSuite`, `CometExecRuleSuite`, and `CometShuffleFallbackStickinessSuite`: **121 passed**, zero failures/aborted suites, and two existing Spark-version-gated cancellations (123 registered tests). - The JVM tests use Spark 4.0.4 / Java 17 and the unchanged public-base native library. Checks cover AQE materialization, whole-plan and stage-only reapplication, distinct PartialMerge chains, safe mixed execution, and fully native execution. - Root-reactor Maven package, Spotless, Scala style checks, and `git diff --check` passed. - Independent stock Spark 4.0.2 SQL replay confirmed correct wide-decimal AVG results and preserved native MIN/MAX with AQE off/on. Integer/narrow-decimal empty AVG still exhibits the separate #5418 bug, as expected; applying both fixes locally matches Spark in **20/20** synthetic cases. -- 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]
