dtenedor commented on code in PR #50606:
URL: https://github.com/apache/spark/pull/50606#discussion_r2047820851
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala:
##########
@@ -6532,12 +6547,12 @@ class AstBuilder extends DataTypeAstBuilder
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
+ // If the grouping expression is an [[UnresolvedOrdinal]], replace
the ordinal value and
+ // create [[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 =>
+ case UnresolvedOrdinal(v: Int) =>
newGroupingExpressions +=
UnresolvedOrdinal(newAggregateExpressions.length + 1)
Review Comment:
Note that for pipe SQL syntax, GROUP BY ordinals work differently. In this
case, the ordinals refer to the one-based indexes of the attributes returned
from the child operator, not to the grouping expressions.
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
