Skip to content

Conversation

@maropu
Copy link
Member

@maropu maropu commented Jul 29, 2017

What changes were proposed in this pull request?

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

How was this patch tested?

Added tests in AnalysisSuite, PlanParserSuite, and SQLQueryTestSuite.

@maropu
Copy link
Member Author

maropu commented Jul 29, 2017

In visitTableName in AstBuilder, we already have a different analyzer path to resolve column aliases (https://github.com/maropu/spark/blob/6c93f859b7ef2cc7f7cb49ab6fbd0397b43fda84/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala#L685). We could also use UnresolvedSubqueryColumnAliases along with visitAliasedQuery and visitAliasedRelation though, I'm not sure it is worth fixing so. So, could you give me insight on this, too? Thanks.

@SparkQA
Copy link

SparkQA commented Jul 29, 2017

Test build #80047 has finished for PR 18772 at commit 6c93f85.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@maropu
Copy link
Member Author

maropu commented Jul 30, 2017

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented Jul 30, 2017

Test build #80049 has finished for PR 18772 at commit 6c93f85.

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

@maropu
Copy link
Member Author

maropu commented Jul 31, 2017

ping

@gatorsmile
Copy link
Member

ping me? : )

Will review it in the next few days.

@maropu
Copy link
Member Author

maropu commented Jul 31, 2017

oh, yea. Sorry to forget to add @... ;) Thanks!

@gatorsmile
Copy link
Member

#18772 (comment)

This sounds a good idea to me. 👍

@maropu
Copy link
Member Author

maropu commented Jul 31, 2017

ok, I'll fix that way. Is it okay to include the fix in this pr? or, follow-up?

@gatorsmile
Copy link
Member

Let us do it in a separate PR. Thanks!

@maropu
Copy link
Member Author

maropu commented Jul 31, 2017

ok. When I finished this pr, I'll do. Thanks!

@gatorsmile
Copy link
Member

retest this please

@gatorsmile
Copy link
Member

LGTM pending Jenkins.

@SparkQA
Copy link

SparkQA commented Aug 6, 2017

Test build #80290 has finished for PR 18772 at commit 6c93f85.

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

@gatorsmile
Copy link
Member

Thanks! Merging to master.

@maropu
Copy link
Member Author

maropu commented Aug 6, 2017

Thanks! I'll make a pr later to fix this as follow-up.

@asfgit asfgit closed this in 990efad Aug 6, 2017
asfgit pushed a commit that referenced this pull request Aug 6, 2017
… 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 <[email protected]>

Closes #18857 from maropu/SPARK-20963-FOLLOWUP.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants