Copilot commented on code in PR #13033:
URL: https://github.com/apache/gluten/pull/13033#discussion_r4032829251
##########
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 now rewrites every eligible aggregate, including cases where the
child's partitioning already satisfies the aggregate's grouping keys. A
flushable aggregate is explicitly allowed to emit duplicate grouping keys
(HashAggregateExecTransformer.scala:659-660), but Spark can rely on a
key-partitioned input to perform a partial aggregation whose output is already
locally grouped; converting that node can invalidate the assumed shape and
produce incorrect downstream results. Please retain the existing
distributed-by-keys guard used by the analogous Bolt rule
(backends-bolt/src/main/scala/org/apache/gluten/extension/FlushableHashAggregateRule.scala:92-95,
127-140) before converting these nodes.
--
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]
