[SPARK-49565][SQL] Improve auto-generated expression aliases with pipe SQL operators #49245

dtenedor · 2024-12-19T18:39:49Z

What changes were proposed in this pull request?

This RP improves auto-generated expression aliases with pipe SQL operators.

For example, consider the pipe SQL syntax query:

table t
|> extend 1

Previously, the analyzed plan was:

Project [x#x, y#x, pipeexpression(1, false, EXTEND) AS pipeexpression(1)#x]
+- SubqueryAlias spark_catalog.default.t
   +- Relation spark_catalog.default.t[x#x,y#x] csv

After this PR, it is:

Project [x#x, y#x, pipeexpression(1, false, EXTEND) AS 1#x]
+- SubqueryAlias spark_catalog.default.t
   +- Relation spark_catalog.default.t[x#x,y#x] csv

Note that the output aliases visible in the resulting DataFrame for the query derive from the AS <alias> part of the analyzed plans shown.

Why are the changes needed?

This improves the user experience with pipe SQL syntax.

Does this PR introduce any user-facing change?

Yes, see above.

How was this patch tested?

Existing golden file tests update to show the improved aliases.

Was this patch authored or co-authored using generative AI tooling?

No.

dtenedor · 2024-12-19T18:42:38Z

cc @cloud-fan @gengliangwang

cloud-fan · 2024-12-20T07:37:29Z

Shall we make AliasResolution to properly support PipeExpression?

dtenedor · 2024-12-20T23:00:50Z

Shall we make AliasResolution to properly support PipeExpression?

@cloud-fan I tried this, but the PipeExpression needs to check whether its child expression tree contains any aggregate functions, and this needs its child tree to be fully resolved first (which may not have happened yet by the time we run the ResolveAliases rule).

cloud-fan · 2024-12-23T05:22:39Z

@dtenedor after a few iterations, ResolveAliases should see PipeExpression with its child resolved, right? The thing is that we should have consistent auto alias generation behavior between normal and pipe SQL syntaxes, e.g.

      case ne: NamedExpression => ne
      case go @ GeneratorOuter(g: Generator) if g.resolved => MultiAlias(go, Nil)
      case e if !e.resolved => u
      case g: Generator => MultiAlias(g, Nil)
      case c @ Cast(ne: NamedExpression, _, _, _) => Alias(c, ne.name)()
      case e: ExtractValue if extractOnly(e) => Alias(e, toPrettySQL(e))()

dtenedor · 2025-01-06T22:38:35Z

@cloud-fan I looked again and I was able to make this work in ResolveAliases by moving the check for the PipeExpression until after the existing check that the child of the UnresolvedAlias is itself resolved.

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/AliasResolution.scala

cloud-fan · 2025-01-07T04:59:20Z

@dtenedor there are more test failures in the golden files.

respond to code review comments

sync

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala

cloud-fan · 2025-01-09T04:58:24Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala

@@ -503,7 +504,8 @@ class Analyzer(override val catalogManager: CatalogManager) extends RuleExecutor
   */
  object ResolveAliases extends Rule[LogicalPlan] {
    def apply(plan: LogicalPlan): LogicalPlan =
-      plan.resolveOperatorsUpWithPruning(_.containsPattern(UNRESOLVED_ALIAS), ruleId) {
+      plan.resolveOperatorsUpWithPruning(
+        _.containsAnyPattern(UNRESOLVED_ALIAS, PIPE_EXPRESSION), ruleId) {


unnecessary change

good catch, reverted this change.

cloud-fan · 2025-01-09T04:58:33Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala

@@ -538,6 +540,13 @@ class Analyzer(override val catalogManager: CatalogManager) extends RuleExecutor
            if c.child.resolved && AliasResolution.hasUnresolvedAlias(c.metrics) =>
          c.copy(metrics = AliasResolution.assignAliases(c.metrics))
      }
+
+    private def removePipeExpressions(exprs: Seq[NamedExpression]): Seq[NamedExpression] =


unnecessary change

good catch, reverted this change.

cloud-fan · 2025-01-09T04:59:38Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/pipeOperators.scala

+    case node: LogicalPlan =>
+      node.resolveExpressions {
+        case p: PipeExpression if p.child.resolved =>
+          p.checkInvariantsAndRemove


shall we remove this lazy val and inline the validation code here?

good idea, done.

cloud-fan · 2025-01-10T03:21:27Z

thanks, merging to master!

commit

94b5fa7

github-actions bot added the SQL label Dec 19, 2024

dtenedor marked this pull request as ready for review December 19, 2024 18:42

respond to code review comments

c82385e

cloud-fan reviewed Jan 7, 2025

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/AliasResolution.scala Outdated Show resolved Hide resolved

dtenedor added 3 commits January 7, 2025 12:18

respond to code review comments

88592af

respond to code review comments

Merge branch 'master' into fix-pipe-output-aliases

efc60c8

sync

sync

b2fcf3a

dtenedor requested a review from cloud-fan January 7, 2025 20:28

cloud-fan reviewed Jan 8, 2025

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala Outdated Show resolved Hide resolved

respond to code review comments

46267a7

dtenedor requested a review from cloud-fan January 8, 2025 18:53

cloud-fan reviewed Jan 9, 2025

View reviewed changes

cloud-fan approved these changes Jan 9, 2025

View reviewed changes

dtenedor added 2 commits January 9, 2025 14:55

sync

168402f

sync

ad74a06

cloud-fan closed this in a8bec11 Jan 10, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-49565][SQL] Improve auto-generated expression aliases with pipe SQL operators #49245

[SPARK-49565][SQL] Improve auto-generated expression aliases with pipe SQL operators #49245

dtenedor commented Dec 19, 2024 •

edited

Loading

dtenedor commented Dec 19, 2024

cloud-fan commented Dec 20, 2024

dtenedor commented Dec 20, 2024 •

edited

Loading

cloud-fan commented Dec 23, 2024

dtenedor commented Jan 6, 2025

cloud-fan commented Jan 7, 2025

cloud-fan Jan 9, 2025

dtenedor Jan 9, 2025

cloud-fan Jan 9, 2025

dtenedor Jan 9, 2025

cloud-fan Jan 9, 2025

dtenedor Jan 9, 2025

cloud-fan commented Jan 10, 2025

[SPARK-49565][SQL] Improve auto-generated expression aliases with pipe SQL operators #49245

[SPARK-49565][SQL] Improve auto-generated expression aliases with pipe SQL operators #49245

Conversation

dtenedor commented Dec 19, 2024 • edited Loading

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

dtenedor commented Dec 19, 2024

cloud-fan commented Dec 20, 2024

dtenedor commented Dec 20, 2024 • edited Loading

cloud-fan commented Dec 23, 2024

dtenedor commented Jan 6, 2025

cloud-fan commented Jan 7, 2025

cloud-fan Jan 9, 2025

Choose a reason for hiding this comment

dtenedor Jan 9, 2025

Choose a reason for hiding this comment

cloud-fan Jan 9, 2025

Choose a reason for hiding this comment

dtenedor Jan 9, 2025

Choose a reason for hiding this comment

cloud-fan Jan 9, 2025

Choose a reason for hiding this comment

dtenedor Jan 9, 2025

Choose a reason for hiding this comment

cloud-fan commented Jan 10, 2025

dtenedor commented Dec 19, 2024 •

edited

Loading

dtenedor commented Dec 20, 2024 •

edited

Loading