[SPARK-20962][SQL] Support subquery column aliases in FROM clause by maropu · Pull Request #18185 · apache/spark

maropu · 2017-06-02T06:42:14Z

What changes were proposed in this pull request?

This pr added parsing rules to support subquery column aliases in FROM clause.
This pr is a sub-task of #18079.

How was this patch tested?

Added tests in PlanParserSuite and SQLQueryTestSuite.

SparkQA · 2017-06-02T06:47:35Z

Test build #77671 has started for PR 18185 at commit 7d9d4ca.

maropu · 2017-06-02T07:09:20Z

Jenkins, retest this please.

SparkQA · 2017-06-02T09:40:11Z

Test build #77672 has finished for PR 18185 at commit 7d9d4ca.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class UnresolvedRelation(tableIdentifier: TableIdentifier) extends LeafNode

SparkQA · 2017-07-11T12:46:26Z

Test build #79519 has finished for PR 18185 at commit cd5d0ef.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2017-07-11T13:01:50Z

@gatorsmile If you get time, could you check this? Thanks!

maropu · 2017-07-20T02:31:00Z

ping

gatorsmile · 2017-07-20T04:39:21Z

Will review it tonight. Thanks!

gatorsmile · 2017-07-20T07:03:22Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

We need a DDL syntax example here. Could you add it?

gatorsmile · 2017-07-20T07:36:35Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

The impact of changing SubqueryAlias is too big. How about adding a new operator node UnresolvedColumnAlias? In the analyzer, UnresolvedColumnAlias will be replaced by a Project? The resolution is by position.

ok, I'll check if we could do so.

SparkQA · 2017-07-24T10:06:58Z

Test build #79901 has finished for PR 18185 at commit 5b272e4.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class UnresolvedSubqueryColumnAlias(

maropu · 2017-07-24T10:12:15Z

@gatorsmile ok, done. Could you check again?

maropu · 2017-07-27T08:59:21Z

ping

gatorsmile · 2017-07-27T19:55:12Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

+    }
+    val subquery = SubqueryAlias(alias, plan(ctx.queryNoWith).optionalMap(ctx.sample)(withSample))
+    if (ctx.tableAlias.identifierList != null) {
+      val columnNames = visitIdentifierList(ctx.tableAlias.identifierList)


Nit: columnNames -> columnAliases

gatorsmile · 2017-07-27T19:56:03Z

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

+ *
+ * @param outputColumnNames the column names for this subquery.
+ * @param child the logical plan of this subquery.
+ */


Nit: UnresolvedSubqueryColumnAlias -> UnresolvedSubqueryColumnAliases

gatorsmile · 2017-07-27T19:58:24Z

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

+ *   SELECT col1, col2 FROM testData AS t(col1, col2);
+ * }}}
+ *
+ * @param outputColumnNames the column names for this subquery.


Nit: the logical plan of this subquery -> the [[LogicalPlan]] on which this subquery column aliases apply

gatorsmile · 2017-07-27T19:59:35Z

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

  override lazy val resolved = false
 }

+/**


resolved by positions.

gatorsmile · 2017-07-27T20:02:56Z

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

+        // Checks if the number of the aliases equals to the number of output columns
+        // in the subquery.
+        if (columnNames.size != outputAttrs.size) {
+          u.failAnalysis(s"Number of column aliases does not match number of columns. " +


Nit: remove the string Interpolator s.

gatorsmile · 2017-07-27T20:06:08Z

LGTM except a few minor comments. Thanks for working on it!

maropu · 2017-07-27T23:59:57Z

Thanks! Fixed.

SparkQA · 2017-07-28T00:09:03Z

Test build #80010 has finished for PR 18185 at commit 2b50e50.

This patch fails to build.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class UnresolvedSubqueryColumnAliases(

SparkQA · 2017-07-28T03:16:09Z

Test build #80011 has finished for PR 18185 at commit 23ca897.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class UnresolvedSubqueryColumnAliases(

maropu · 2017-07-28T03:23:05Z

@gatorsmile ok

gatorsmile · 2017-07-28T23:28:57Z

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

      // rule: ResolveDeserializer.
      case plan if containsDeserializer(plan.expressions) => plan

+      case u @ UnresolvedSubqueryColumnAliases(columnNames, child) if child.resolved =>


The last question. What is the reason why we do not have a separate analyzer rule?

Yea, I forgot to move this rule outside. I'll update soon.

SparkQA · 2017-07-29T03:53:59Z

Test build #80031 has finished for PR 18185 at commit 277d6ee.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

gatorsmile · 2017-07-29T17:14:23Z

LGTM.

Thanks! Merging to master.

… visitTableName ## What changes were proposed in this pull request? This pr (follow-up of #18772) used `UnresolvedSubqueryColumnAliases` for `visitTableName` in `AstBuilder`, which is a new unresolved `LogicalPlan` implemented in #18185. ## How was this patch tested? Existing tests Author: Takeshi Yamamuro <yamamuro@apache.org> Closes #18857 from maropu/SPARK-20963-FOLLOWUP.

maropu force-pushed the SPARK-20962 branch from 7d9d4ca to cd5d0ef Compare July 11, 2017 10:05

gatorsmile reviewed Jul 20, 2017

View reviewed changes

maropu added 3 commits July 24, 2017 11:07

Support subquery column aliases in FROM clause

2014ac0

Add a DDL syntax example

d971d35

Add UnresolvedSubqueryColumnAlias

5b272e4

maropu force-pushed the SPARK-20962 branch from cd5d0ef to 5b272e4 Compare July 24, 2017 07:28

gatorsmile reviewed Jul 27, 2017

View reviewed changes

Apply xiao's comments

23ca897

maropu force-pushed the SPARK-20962 branch from 2b50e50 to 23ca897 Compare July 28, 2017 00:38

gatorsmile reviewed Jul 28, 2017

View reviewed changes

Add ResolveSubqueryColumnAliases rule

277d6ee

asfgit closed this in 6550086 Jul 29, 2017

maropu mentioned this pull request Aug 6, 2017

[SPARK-20963][SQL][FOLLOW-UP] Use UnresolvedSubqueryColumnAliases for visitTableName #18857

Closed

Conversation

maropu commented Jun 2, 2017

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

SparkQA commented Jun 2, 2017

Uh oh!

maropu commented Jun 2, 2017

Uh oh!

SparkQA commented Jun 2, 2017

Uh oh!

SparkQA commented Jul 11, 2017

Uh oh!

maropu commented Jul 11, 2017

Uh oh!

maropu commented Jul 20, 2017

Uh oh!

gatorsmile commented Jul 20, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gatorsmile Jul 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 24, 2017

Uh oh!

maropu commented Jul 24, 2017

Uh oh!

maropu commented Jul 27, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gatorsmile commented Jul 27, 2017

Uh oh!

maropu commented Jul 27, 2017

Uh oh!

SparkQA commented Jul 28, 2017

Uh oh!

SparkQA commented Jul 28, 2017

Uh oh!

maropu commented Jul 28, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 29, 2017

Uh oh!

gatorsmile commented Jul 29, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gatorsmile Jul 20, 2017 •

edited

Loading