[GitHub] [spark] cloud-fan commented on a change in pull request #36024: [SPARK-38712][SQL] Fix perf regression in PhysicalOperation

[GitBox <gi...@apache.org>]

cloud-fan commented on a change in pull request #36024:
URL: https://github.com/apache/spark/pull/36024#discussion_r839485573



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/AliasHelper.scala
##########
@@ -70,11 +70,17 @@ trait AliasHelper {
   protected def replaceAliasButKeepName(
      expr: NamedExpression,
      aliasMap: AttributeMap[Alias]): NamedExpression = {
-    // Use transformUp to prevent infinite recursion when the replacement expression
-    // redefines the same ExprId,
-    trimNonTopLevelAliases(expr.transformUp {
+    expr match {
+      // We need to keep the `Alias` if we replace a top-level Attribute, so that it's still a
+      // `NamedExpression`. We also need to keep the name of the original Attribute.
       case a: Attribute => aliasMap.get(a).map(_.withName(a.name)).getOrElse(a)
-    }).asInstanceOf[NamedExpression]
+      case _ =>
+        // Use transformUp to prevent infinite recursion when the replacement expression
+        // redefines the same ExprId,
+        expr.transformUp {
+          case a: Attribute => aliasMap.get(a).map(_.child).getOrElse(a)

Review comment:
       This is the key change. We replace the attribute with `Alias.child`, so that we don't need to call `trimNonTopLevelAliases` to remove it later.




-- 
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-unsubscribe@spark.apache.org

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