Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/10677#discussion_r51644362
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -233,14 +233,39 @@ class Analyzer(
}
}.toMap
- val aggregations: Seq[NamedExpression] = x.aggregations.map {
- // If an expression is an aggregate (contains a
AggregateExpression) then we dont change
- // it so that the aggregation is computed on the unmodified
value of its argument
- // expressions.
- case expr if
expr.find(_.isInstanceOf[AggregateExpression]).nonEmpty => expr
- // If not then its a grouping expression and we need to use the
modified (with nulls from
- // Expand) value of the expression.
- case expr => expr.transformDown {
+ val aggregations: Seq[NamedExpression] = x.aggregations.map { case
expr =>
+ // collect all the found AggregateExpression, so we can check an
expression is part of
+ // any AggregateExpression or not.
+ val aggsBuffer = ArrayBuffer[Expression]()
+ def isPartOfAggregation(e: Expression): Boolean = {
+ aggsBuffer.exists(a => a.find(_ eq e).isDefined)
+ }
+ expr.transformDown {
+ // AggregateExpression should be computed on the unmodified
value of its argument
+ // expressions, so we should not replace any references to
grouping expression
+ // inside it.
+ case e: AggregateExpression =>
+ aggsBuffer += e
+ e
+ case e if isPartOfAggregation(e) => e
+ case e: GroupingID =>
--- End diff --
Right now, it will fail to resolve, agreed that should be have a better
error message.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
