Skip to content
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

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

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

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 <wenchen@databricks.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit acc211d)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@dongjoon-hyun
Copy link
Member

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

I made a follow-up.

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

@luluorta luluorta luluorta approved these changes

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