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-35675][SQL] EnsureRequirements remove shuffle should respect PartitioningCollection #32815

Closed
wants to merge 3 commits into from
Closed

[SPARK-35675][SQL] EnsureRequirements remove shuffle should respect PartitioningCollection #32815

wants to merge 3 commits into from

Conversation

ulysses-you
Copy link
Contributor

What changes were proposed in this pull request?

Add PartitioningCollection in EnsureRequirements during remove shuffle.

Why are the changes needed?

Currently EnsureRequirements only check if child has semantic equal HashPartitioning and remove
redundant shuffle. We can enhance this case using PartitioningCollection.

Does this PR introduce any user-facing change?

Yes, plan might be changed.

How was this patch tested?

Add test.

@github-actions github-actions bot added the SQL label Jun 8, 2021
@SparkQA
Copy link

SparkQA commented Jun 8, 2021

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

@SparkQA
Copy link

SparkQA commented Jun 8, 2021

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

@SparkQA
Copy link

SparkQA commented Jun 8, 2021

Test build #139457 has finished for PR 32815 at commit fa56cf7.

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

@ulysses-you
Copy link
Contributor Author

cc @maropu @cloud-fan @yaooqinn

cloud-fan
cloud-fan reviewed Jun 9, 2021
View reviewed changes
sql/core/src/test/scala/org/apache/spark/sql/execution/exchange/EnsureRequirementsSuite.scala Outdated

test("SPARK-35675: EnsureRequirements remove shuffle should respect PartitioningCollection") {
val local = spark
import local.implicits._
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: usually we do import testImplicits.*

yaooqinn
yaooqinn approved these changes Jun 9, 2021
View reviewed changes
Copy link
Member

@yaooqinn yaooqinn left a comment

Choose a reason for hiding this comment

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

LGTM

@SparkQA
Copy link

SparkQA commented Jun 9, 2021

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

@SparkQA
Copy link

SparkQA commented Jun 9, 2021

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

@SparkQA
Copy link

SparkQA commented Jun 9, 2021

Test build #139552 has finished for PR 32815 at commit 7a906a5.

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

@SparkQA
Copy link

SparkQA commented Jun 10, 2021

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

@SparkQA
Copy link

SparkQA commented Jun 10, 2021

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

@yaooqinn yaooqinn closed this in 8dde20a Jun 10, 2021
@yaooqinn
Copy link
Member

thanks, merged to master

@ulysses-you
Copy link
Contributor Author

thanks for merging !

@ulysses-you ulysses-you deleted the shuffle-node branch June 10, 2021 05:19
@SparkQA
Copy link

SparkQA commented Jun 10, 2021

Test build #139605 has finished for PR 32815 at commit 7caddac.

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

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 approved these changes

@yaooqinn yaooqinn yaooqinn approved these changes

Assignees
No one assigned
Labels
SQL
Projects
None yet
Milestone
No milestone
4 participants
@ulysses-you @SparkQA @yaooqinn @cloud-fan