dtenedor commented on code in PR #49248:
URL: https://github.com/apache/spark/pull/49248#discussion_r1894491799
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala:
##########
@@ -6183,12 +6183,30 @@ class AstBuilder extends DataTypeAstBuilder
a.aggregateExpressions.foreach(visit)
// Prepend grouping keys to the list of aggregate functions, since
operator pipe AGGREGATE
// clause returns the GROUP BY expressions followed by the list of
aggregate functions.
- val namedGroupingExpressions: Seq[NamedExpression] =
- a.groupingExpressions.map {
- case n: NamedExpression => n
- case e: Expression => UnresolvedAlias(e, None)
- }
- a.copy(aggregateExpressions = namedGroupingExpressions ++
a.aggregateExpressions)
+ val newGroupingExpressions = ArrayBuffer.empty[Expression]
+ val newAggregateExpressions = ArrayBuffer.empty[NamedExpression]
+ var numGroupByOrdinalReferences = 0
+ a.groupingExpressions.foreach {
+ case n: NamedExpression =>
+ newGroupingExpressions += n
+ newAggregateExpressions += n
+ // If the grouping expression is an integer literal, create
[[UnresolvedOrdinal]] and
+ // [[UnresolvedPipeAggregateOrdinal]] expressions to represent it
in the final grouping
+ // and aggregate expressions, respectively. This will let the
+ // [[ResolveOrdinalInOrderByAndGroupBy]] rule detect the ordinal
in the aggregate list
+ // and replace it with the corresponding attribute from the child
operator.
+ case Literal(v: Int, IntegerType) if conf.groupByOrdinal =>
+ numGroupByOrdinalReferences += 1
+ newGroupingExpressions +=
UnresolvedOrdinal(numGroupByOrdinalReferences)
Review Comment:
Good catch!! This should be
`UnresolvedOrdinal(newAggregateExpressions.length + 1)` instead of
`UnresolvedOrdinal(numGroupByOrdinalReferences)`. I made this update and added
a test case to cover this.
--
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]