Skip to content

[SPARK-29145][SQL][FOLLOWUP] Clean up code about support sub-queries in join conditions #32499

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
AngersZhuuuu wants to merge 3 commits into from
Closed

[SPARK-29145][SQL][FOLLOWUP] Clean up code about support sub-queries in join conditions #32499

AngersZhuuuu wants to merge 3 commits into from

Conversation

@AngersZhuuuu
Copy link
Contributor

@AngersZhuuuu AngersZhuuuu commented May 11, 2021

What changes were proposed in this pull request?

According to discuss #25854 (comment)

Why are the changes needed?

Clean code

Does this PR introduce any user-facing change?

No

How was this patch tested?

Existed UT

@AngersZhuuuu
Copy link
Contributor Author

AngersZhuuuu commented May 11, 2021

FYI @cloud-fan

@github-actions github-actions bot added the SQL label May 11, 2021
cloud-fan
cloud-fan reviewed May 11, 2021
View reviewed changes
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala Outdated
resolveSubQueries(q, q.children)
case j: Join if j.childrenResolved =>
resolveSubQueries(j, Seq(j, j.left, j.right))
resolveSubQueries(j, Seq(j.left, j.right))
Copy link
Contributor

@cloud-fan cloud-fan May 11, 2021

Choose a reason for hiding this comment

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

nit: j.children

Copy link
Contributor Author

@AngersZhuuuu AngersZhuuuu May 11, 2021

Choose a reason for hiding this comment

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

nit: j.children

Done

@SparkQA
Copy link

SparkQA commented May 11, 2021

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

@SparkQA
Copy link

SparkQA commented May 11, 2021

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

@SparkQA
Copy link

SparkQA commented May 11, 2021

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

@SparkQA
Copy link

SparkQA commented May 11, 2021

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

@SparkQA
Copy link

SparkQA commented May 11, 2021

Test build #138370 has finished for PR 32499 at commit 3aa629b.

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

@SparkQA
Copy link

SparkQA commented May 11, 2021

Test build #138368 has finished for PR 32499 at commit 504f821.

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

cloud-fan
cloud-fan reviewed May 11, 2021
View reviewed changes
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala Outdated
// Only a few unary nodes (Project/Filter/Aggregate) can contain subqueries.
case q: UnaryNode if q.childrenResolved =>
resolveSubQueries(q, q.children)
case j: Join if j.childrenResolved =>
Copy link
Contributor

@cloud-fan cloud-fan May 11, 2021

Choose a reason for hiding this comment

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

seems we should also check j.duplicateResolved

Copy link
Contributor Author

@AngersZhuuuu AngersZhuuuu May 11, 2021

Choose a reason for hiding this comment

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

Done, but I am confused witch case will cause error.

@SparkQA
Copy link

SparkQA commented May 11, 2021

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

@SparkQA
Copy link

SparkQA commented May 11, 2021

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

@SparkQA
Copy link

SparkQA commented May 11, 2021

Test build #138381 has finished for PR 32499 at commit d9cad03.

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

@maropu
Copy link
Member

maropu commented May 12, 2021

Is the title correct? It seems a clean-up fix.

@AngersZhuuuu AngersZhuuuu changed the title [SPARK-29145][SQL][FOLLOWUP] Support sub-queries in join conditions [SPARK-29145][SQL][FOLLOWUP] Clean up code about support sub-queries in join conditions May 12, 2021
@AngersZhuuuu
Copy link
Contributor Author

AngersZhuuuu commented May 12, 2021

Is the title correct? It seems a clean-up fix.

Hmmm how about current?

@cloud-fan
Copy link
Contributor

cloud-fan commented May 12, 2021

thanks, merging to master!

@cloud-fan cloud-fan closed this in ed05954 May 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cloud-fan cloud-fan cloud-fan left review comments

Assignees

No one assigned

Labels

SQL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@AngersZhuuuu @SparkQA @maropu @cloud-fan