cloud-fan commented on PR #57859:
URL: https://github.com/apache/spark/pull/57859#issuecomment-5236433239

   Thanks for the detailed review. Addressed in 359d3dc5d3f:
   
   1. The combined hash aggregate now retains 
`finalAgg.requiredChildDistributionExpressions`. This preserves 
`ClusteredDistribution`/`AllTuples` and lets AQE validation reject skewed 
rebalance reads that split a grouping key. I added an AQE rebalance/skew 
regression that checks the answer, confirms the aggregates combine, and 
verifies no `PartialReducerPartitionSpec` survives below the combined aggregate.
   
   2. Removed the `partialAgg.outputSet != finalAgg.usedInputs` check, since it 
is true by construction and did not validate the post-rewrite input layout.
   
   3. The `PartialMerge` -> `Final` result now takes `initialInputBufferOffset` 
from the lower aggregate, whose child becomes the combined aggregate child.
   
   I also updated the class documentation. The sort/object-hash paths continue 
retaining the final node distribution through `copy`, consistently with the 
hash path.
   
   For execution coverage, the PartialMerge test now builds a valid executable 
Partial -> PartialMerge -> Final pipeline and compares results before and after 
combining, in addition to its metadata and filter checks. OSS does not 
currently generate this pure adjacency from a built-in query; the documented 
use case is a physical-plan extension that introduces an extra aggregation 
stage.
   
   I attempted `build/sbt 'sql/testOnly 
org.apache.spark.sql.execution.CombineAdjacentAggregationSuite'` twice, but SBT 
project loading was blocked before compilation by DNS failures resolving three 
BOMs from Maven Central (jackson-bom, jjwt-bom, and kubernetes-client-bom). 
Static diff/style checks pass.


-- 
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]

Reply via email to