Copilot commented on code in PR #13033:
URL: https://github.com/apache/gluten/pull/13033#discussion_r4033914953
##########
backends-velox/src/main/scala/org/apache/gluten/extension/FlushableHashAggregateRule.scala:
##########
@@ -38,15 +34,13 @@ case class FlushableHashAggregateRule(session:
SparkSession) extends Rule[SparkP
if (!VeloxConfig.get.enableVeloxFlushablePartialAggregation) {
return plan
}
- val protectedAggs = collectProtectedOneDistinctPartialMergeAggs(plan)
- plan.transformUpWithPruning(_.containsPattern(EXCHANGE)) {
- case s: ShuffleExchangeLike =>
- // If an exchange follows a hash aggregate in which all functions are
in partial mode,
- // then it's safe to convert the hash aggregate to flushable hash
aggregate.
- val out = s.withNewChildren(
- List(replaceEligibleAggregates(s.child, protectedAggs))
- )
- out
+ val protectedAggIds = collectProtectedOneDistinctPartialMergeAggIds(plan)
+ // Use top-down traversal: child rewrites can copy an aggregate with a new
plan ID.
+ plan.transformDown {
+ case agg: RegularHashAggregateExecTransformer if isEligible(agg,
protectedAggIds) =>
+ toFlushableAgg(agg)
+ case agg: SortHashAggregateExecTransformer if isEligible(agg,
protectedAggIds) =>
+ toFlushableAgg(agg)
Review Comment:
This broadens the rewrite from aggregates below a shuffle to every eligible
aggregate in the plan, but join-aggregate's partial phase is intentionally a
`HashAggregateExec` followed by a packing `ProjectExec` and then a `Join` (see
`gluten-substrait/src/main/scala/org/apache/gluten/extension/joinagg/ImplementJoinAggregate.scala:126-195`).
Converting that partial aggregate to `FlushableHashAggregateExecTransformer`
allows duplicate rows for the same grouping key, so the join can multiply
matches because there is no downstream aggregate to merge them. Please exclude
join-aggregate partials (or otherwise preserve the exchange/downstream-merge
boundary) before applying this traversal.
--
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]
