Copilot commented on code in PR #13033:
URL: https://github.com/apache/gluten/pull/13033#discussion_r4025009293
##########
backends-velox/src/main/scala/org/apache/gluten/extension/FlushableHashAggregateRule.scala:
##########
@@ -38,15 +34,12 @@ 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)
+ plan.transformUp {
+ case agg: RegularHashAggregateExecTransformer if isEligible(agg,
protectedAggIds) =>
+ toFlushableAgg(agg)
+ case agg: SortHashAggregateExecTransformer if isEligible(agg,
protectedAggIds) =>
+ toFlushableAgg(agg)
Review Comment:
This global traversal now rewrites `Partial` aggregates created by
`PushAggregateThroughJoin`, including broadcast/no-shuffle join sides. Those
aggregates are followed by the packing `ProjectExec` and the join rather than
an aggregate merge (`ImplementJoinAggregate.scala:107-195`), so enabling Velox
flushing can emit duplicate grouping keys/buffers across the join; the final
phase then merges each duplicate and inflates sums/counts. Keep these
join-pushed partials out of the rewrite (or require an aggregate consumer) and
add a regression with push-through-join plus a low flush threshold.
This issue also appears on line 82 of the same file.
--
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]
