Skip to content

fix: revert unsafe partial aggregates after final fallback - #5421

Open
sunchao wants to merge 1 commit into
apache:mainfrom
sunchao:dev/chao/codex/oss-unsafe-partial-aggregate-fallback
Open

fix: revert unsafe partial aggregates after final fallback#5421
sunchao wants to merge 1 commit into
apache:mainfrom
sunchao:dev/chao/codex/oss-unsafe-partial-aggregate-fallback

Conversation

@sunchao

@sunchao sunchao commented Aug 22, 2026

Copy link
Copy Markdown
Member

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 Incorrect AVG result from empty native partial buffers #5418 bug, as expected; applying both fixes locally matches Spark in 20/20 synthetic cases.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unsafe native partial aggregates survive child-triggered final fallback

1 participant