zhztheplayer commented on code in PR #12960:
URL: https://github.com/apache/gluten/pull/12960#discussion_r3920464804
##########
backends-velox/src/main/scala/org/apache/gluten/extension/FlushableHashAggregateRule.scala:
##########
@@ -70,12 +70,32 @@ case class FlushableHashAggregateRule(session:
SparkSession) extends Rule[SparkP
aggExprs.exists(isUnsupportedAggregation)
}
+ /**
+ * Returns true if the aggregate applies no aggregate functions and is the
final (or complete)
+ * stage, e.g. the last step of `SELECT DISTINCT`, or of a `GROUP BY`
without aggregate functions.
+ *
+ * Such an aggregate must fully aggregate. Flushing lets Velox abandon
aggregation and emit
+ * duplicate grouping keys, and since no further aggregate follows, the
duplicates reach the
+ * consumer. When the consumer is a join, they turn into extra join output
rows.
+ *
+ * The mode check used by the other guards cannot detect this case:
`aggregateExpressions` is
+ * empty here, so `forall(_.mode == Partial | PartialMerge)` is vacuously
true and says nothing
+ * about which stage this is. Spark distinguishes the stages with
+ * `requiredChildDistributionExpressions`, which is `None` for a partial
aggregate and
+ * `Some(groupingAttributes)` for the final one -- see
`AggUtils.planAggregateWithoutDistinct`.
+ * Spark makes the same check in its own guard for the equivalent runtime
bypass, see
+ * `HashAggregateExec.adaptivePartialAggEnabled`.
+ */
Review Comment:
nit: Can we only keep the 1st paragraph of the comment?
--
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]
