Skip to content

[SPARK-33141][SQL][FOLLOW-UP] Store the max nested view depth in AnalysisContext #30575

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[SPARK-33141][SQL][FOLLOW-UP] Store the max nested view depth in AnalysisContext #30575

wants to merge 1 commit into from

Conversation

@cloud-fan
Copy link
Contributor

@cloud-fan cloud-fan commented Dec 2, 2020

What changes were proposed in this pull request?

This is a followup of #30289. It removes the hack in View.effectiveSQLConf, by putting the max nested view depth in AnalysisContext. Then we don't get the max nested view depth from the active SQLConf, which keeps changing during nested view resolution.

Why are the changes needed?

remove hacks.

Does this PR introduce any user-facing change?

No

How was this patch tested?

If I just remove the hack, SimpleSQLViewSuite.restrict the nested level of a view fails. With this fix, it passes again.

@cloud-fan
Copy link
Contributor Author

cloud-fan commented Dec 2, 2020 •
edited
Loading

cc @luluorta @maropu @LinhongLiu

@github-actions github-actions bot added the SQL label Dec 2, 2020
@SparkQA
Copy link

SparkQA commented Dec 2, 2020

Test build #132065 has finished for PR 30575 at commit 0bfb0e4.

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

@HyukjinKwon HyukjinKwon changed the title [SPARK-33141][SQL][followup] store the max nested view depth in AnalysisContext [SPARK-33141][SQL][FOLLOW-UP] Store the max nested view depth in AnalysisContext Dec 3, 2020
@luluorta
Copy link
Contributor

luluorta commented Dec 3, 2020

cc @leanken

@SparkQA
Copy link

SparkQA commented Dec 3, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/36730/

@SparkQA
Copy link

SparkQA commented Dec 3, 2020

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/36730/

@SparkQA
Copy link

SparkQA commented Dec 3, 2020

Test build #132129 has finished for PR 30575 at commit 0157d7d.

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

luluorta
luluorta approved these changes Dec 4, 2020
View reviewed changes
Copy link
Contributor

@luluorta luluorta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@SparkQA
Copy link

SparkQA commented Dec 4, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/36818/

@SparkQA
Copy link

SparkQA commented Dec 4, 2020

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/36818/

@SparkQA
Copy link

SparkQA commented Dec 4, 2020

Test build #132217 has finished for PR 30575 at commit 82e5029.

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

@cloud-fan
Copy link
Contributor Author

cloud-fan commented Dec 4, 2020 •
edited
Loading

thanks for the review, merging to master/3.1 (since it's a followup and polishes the code)!

@cloud-fan cloud-fan closed this in acc211d Dec 4, 2020
cloud-fan added a commit that referenced this pull request Dec 4, 2020
@cloud-fan
[SPARK-33141][SQL][FOLLOW-UP] Store the max nested view depth in Anal…
4dea510 
…ysisContext

### What changes were proposed in this pull request?

This is a followup of #30289. It removes the hack in `View.effectiveSQLConf`, by putting the max nested view depth in `AnalysisContext`. Then we don't get the max nested view depth from the active SQLConf, which keeps changing during nested view resolution.

### Why are the changes needed?

remove hacks.

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

If I just remove the hack, `SimpleSQLViewSuite.restrict the nested level of a view` fails. With this fix, it passes again.

Closes #30575 from cloud-fan/view.

Authored-by: Wenchen Fan <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
(cherry picked from commit acc211d)
Signed-off-by: Wenchen Fan <[email protected]>
@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Dec 4, 2020

Hi, @cloud-fan . This seems to break Scala 2.13.

@dongjoon-hyun dongjoon-hyun mentioned this pull request Dec 4, 2020
Closed
@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Dec 4, 2020

I made a follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@luluorta luluorta luluorta approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

SQL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cloud-fan @SparkQA @luluorta @dongjoon-hyun