You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/09/07 05:02:44 UTC
[GitHub] [spark] cloud-fan commented on a change in pull request #29626: [SPARK-32777][SQL] Aggregation support aggregate function with multiple foldable expressions.
cloud-fan commented on a change in pull request #29626:
URL: https://github.com/apache/spark/pull/29626#discussion_r484184121
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/RewriteDistinctAggregates.scala
##########
@@ -216,20 +216,24 @@ object RewriteDistinctAggregates extends Rule[LogicalPlan] {
val distinctAggs = aggExpressions.filter(_.isDistinct)
// Extract distinct aggregate expressions.
- val distinctAggGroups = aggExpressions.filter(_.isDistinct).groupBy { e =>
- val unfoldableChildren = e.aggregateFunction.children.filter(!_.foldable).toSet
- if (unfoldableChildren.nonEmpty) {
- // Only expand the unfoldable children
- unfoldableChildren
- } else {
- // If aggregateFunction's children are all foldable
- // we must expand at least one of the children (here we take the first child),
- // or If we don't, we will get the wrong result, for example:
- // count(distinct 1) will be explained to count(1) after the rewrite function.
- // Generally, the distinct aggregateFunction should not run
- // foldable TypeCheck for the first child.
- e.aggregateFunction.children.take(1).toSet
- }
+ val distinctAggGroupMap = aggExpressions.filter(_.isDistinct).map { e =>
+ val unfoldableChildren = e.aggregateFunction.children.filter(!_.foldable).toSet
+ if (unfoldableChildren.nonEmpty) {
+ // Only expand the unfoldable children
+ e -> unfoldableChildren
+ } else {
+ // If aggregateFunction's children are all foldable
+ // we must expand at least one of the children (here we take the first child),
+ // or If we don't, we will get the wrong result, for example:
+ // count(distinct 1) will be explained to count(1) after the rewrite function.
+ // Generally, the distinct aggregateFunction should not run
+ // foldable TypeCheck for the first child.
+ e -> e.aggregateFunction.children.take(1).toSet
+ }
+ }
+ val distinctAggGroupLookup = distinctAggGroupMap.toMap
+ val distinctAggGroups = distinctAggGroupMap.groupBy(_._2).map{ kv =>
Review comment:
nit: `mapValues`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org