[2010YOUY01]

Which issue does this PR close?

• Closes Implement optimizer hook for combining partial/final aggregate in AggregateExec #25928

Rationale for this change

See issue for backgrounds.

It implements a AggregateExec::try_combine_partial_final() helper, to replace the full logic in CombineParitalFinalAggregate optimizer rule (validation + transformation)

The benefit of this pattern is

• Unify the inner logic in one place for maintainability.
  (The current implementation already did a great job unifying all logic inside the optimizer implementation, however it's common for other rules to implement partial logic in optimizer, and partial logic in ExecutionPlan, which introduces inconsistency. Moving all of them to a atomic optimizer hook in ExecutionPlan seems like a good pattern to avoid such issues)
• Avoid exposing public fields in AggregateExec for optimizer usage, they're in different crate.

What changes are included in this PR?

What is the testing strategy for this PR?

Existing tests

Are there any user-facing changes?

No

[codecov-commenter]

Codecov Report

❌ Patch coverage is 90.00000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.59%. Comparing base (e6dfef5) to head (833a6c0).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
datafusion/physical-plan/src/aggregates/mod.rs	89.70%	6 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #25962      +/-   ##
==========================================
- Coverage   82.59%   82.59%   -0.01%     
==========================================
  Files        1145     1145              
  Lines      443934   444086     +152     
  Branches   443934   444086     +152     
==========================================
+ Hits       366669   366790     +121     
- Misses      55009    55025      +16     
- Partials    22256    22271      +15     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[kosiew]

@2010YOUY01,

Thanks for this refactor. Moving the combination logic into AggregateExec::try_combine_partial_final is appropriate, and the existing transformation behavior appears preserved.

The new boundary is cleaner: the optimizer handles traversal, while AggregateExec owns compatibility checks, reconstruction, and internal state. This removes cross-crate coupling to AggregateKind and allows its implementation details to remain private.

I'm approving because the mode mappings, expression checks, error handling, and distinct-limit preservation remain equivalent. The existing regression test also covers the distinct-limit behavior. I have one non-blocking documentation suggestion to clarify the helper's optimizer-phase prerequisite.

[kosiew]

Could we mention here that this helper should run after distribution enforcement, normally EnsureRequirements? It checks expression compatibility but not distribution requirements. A documentation note would clarify the contract without adding a runtime check.

[alamb]

Merged up and fixed a merge conflict. I am trying to clear the merge queue