AngersZhuuuu commented on a change in pull request #28490:
URL: https://github.com/apache/spark/pull/28490#discussion_r470555710
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -1479,6 +1479,33 @@ class Analyzer(
// Skip the having clause here, this will be handled in
ResolveAggregateFunctions.
case h: UnresolvedHaving => h
+ case agg @ (_: Aggregate | _: GroupingSets) =>
+ val resolved = agg.mapExpressions(resolveExpressionTopDown(_, agg))
+ val hasStructField = resolved.expressions.exists {
+ _.collectFirst { case gsf: GetStructField => gsf }.isDefined
+ }
+ if (hasStructField) {
+ // For struct field, it will be resolve as Alias(GetStructField,
name),
+ // In Aggregate/GroupingSets this behavior will cause same struct
field
+ // in aggExprs/groupExprs/selectedGroupByExprs will be resolved
divided
+ // with different ExprId of Alias and replace failed when construct
+ // Aggregate in ResolveGroupingAnalytics, so we resolve duplicated
struct
+ // field here with same ExprId
+ val structFieldMap = mutable.Map[String, Alias]()
+ resolved.transformExpressionsDown {
+ case a @ Alias(struct: GetStructField, _) =>
+ if (structFieldMap.contains(struct.sql)) {
Review comment:
> is it safe to use sql string as key?
Emmmm how about
```
case agg @ (_: Aggregate | _: GroupingSets) =>
def resolveDuplicatedStructField(
expr: Expression,
filedMap: mutable.Map[String, Alias]): Expression = {
expr.transform {
case a @ Alias(struct: GetStructField, _) =>
val exprId = filedMap.getOrElse(struct.sql, a).exprId
Alias(a.child, a.name)(exprId, a.qualifier, a.explicitMetadata)
}
}
val resolved = agg.mapExpressions(resolveExpressionTopDown(_, agg))
val resolvedExpressions = resolved match {
case aggregate: Aggregate =>
aggregate.aggregateExpressions ++ aggregate.groupingExpressions
case groupingSets: GroupingSets =>
groupingSets.selectedGroupByExprs.flatMap(_.toSeq) ++
groupingSets.groupByExprs ++ groupingSets.aggregations
case _ => Nil
}
val hasStructField = resolvedExpressions.exists {
_.collectFirst { case gsf: GetStructField => gsf }.isDefined
}
if (hasStructField) {
// For struct field, it will be resolve as Alias(GetStructField,
name),
// In Aggregate/GroupingSets this behavior will cause the same
struct fields
// in aggExprs/groupExprs/selectedGroupByExprs be treated as
different ones
// due to different ExprIds in Alias, and stops us finding the
grouping expressions
// in aggExprs. Here we resolve duplicated struct field here with
same ExprId
val structFieldMap = mutable.Map[String, Alias]()
resolvedExpressions.flatMap(_.collect {
case a @ Alias(struct: GetStructField, _) => struct.sql -> a
}).foreach { case (name, alias) =>
if (!structFieldMap.contains(name)) {
structFieldMap += (name -> alias)
}
}
resolved match {
case aggregate: Aggregate =>
aggregate.copy(
groupingExpressions = aggregate.groupingExpressions
.map(resolveDuplicatedStructField(_, structFieldMap)),
aggregateExpressions = aggregate.aggregateExpressions
.map(resolveDuplicatedStructField(_, structFieldMap)
.asInstanceOf[NamedExpression]))
case groupingSets: GroupingSets =>
groupingSets.copy(
selectedGroupByExprs = groupingSets.selectedGroupByExprs
.map(_.map(resolveDuplicatedStructField(_,
structFieldMap))),
groupByExprs = groupingSets.groupByExprs
.map(resolveDuplicatedStructField(_, structFieldMap)),
aggregations = groupingSets.aggregations
.map(resolveDuplicatedStructField(_, structFieldMap)
.asInstanceOf[NamedExpression])
)
}
} else {
resolved
}
```
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
