[SPARK-35984][SQL][TEST] Config to force applying shuffled hash join by linhongliu-db · Pull Request #33182 · apache/spark

linhongliu-db · 2021-07-02T03:27:36Z

What changes were proposed in this pull request?

Add a config spark.sql.join.forceApplyShuffledHashJoin to force applying shuffled hash join
during the join selection.

Why are the changes needed?

In the SQLQueryTestSuite, we want to cover 3 kinds of join (BHJ, SHJ, SMJ) in join.sql. But even
if the spark.sql.join.preferSortMergeJoin is set to false, shuffled hash join is still not guaranteed.
Thus, we need another config to force the selection.

Does this PR introduce any user-facing change?

No, only for testing

How was this patch tested?

newly added tests
Verified all queries in join.sql will use ShuffledHashJoin when the config set to true

linhongliu-db · 2021-07-02T04:03:57Z

cc @cloud-fan

SparkQA · 2021-07-02T04:04:25Z

Test build #140559 has finished for PR 33182 at commit 16a0791.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-07-02T04:28:49Z

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

SparkQA · 2021-07-02T04:45:46Z

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

SparkQA · 2021-07-02T05:03:12Z

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

SparkQA · 2021-07-02T05:21:27Z

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

SparkQA · 2021-07-02T08:03:28Z

Test build #140556 has finished for PR 33182 at commit 24b39a9.

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

c21 · 2021-07-02T08:55:06Z

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

nit: we are on 3.3.0 now I think?

c21 · 2021-07-02T08:56:00Z

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

nit: PREFER_SORTMERGEJOIN.key instead of spark.sql.join.perferSortMergejoin.

c21 · 2021-07-02T09:00:00Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/joins.scala

I think we don't want user to use this config, and this should be only taking effect in testing right? Should we add condition e.g. Utils.isTesting?

SparkQA · 2021-07-03T09:43:43Z

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

SparkQA · 2021-07-03T10:17:01Z

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

SparkQA · 2021-07-03T13:25:11Z

Test build #140613 has finished for PR 33182 at commit f3474a2.

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

cloud-fan · 2021-07-06T08:58:35Z

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

    .booleanConf
    .createWithDefault(true)

+  val FORCE_APPLY_SHUFFLEDHASHJOIN = buildConf("spark.sql.join.forceApplyShuffledHashJoin")


we can just hardcode test-only configs.

SparkQA · 2021-07-06T10:55:39Z

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

SparkQA · 2021-07-06T11:30:43Z

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

cloud-fan · 2021-07-06T13:33:36Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/joins.scala

        (!conf.preferSortMergeJoin && canBuildLocalHashMapBySize(left, conf) &&
-          muchSmaller(left, right))
+          muchSmaller(left, right)) ||
+        (Utils.isTesting && forceApplyShuffledHashJoin(conf))


nit: we can even move Utils.isTesting into forceApplyShuffledHashJoin

SparkQA · 2021-07-06T14:42:57Z

Test build #140701 has finished for PR 33182 at commit d0dfd8d.

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

SparkQA · 2021-07-06T15:59:43Z

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

SparkQA · 2021-07-06T16:33:29Z

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

cloud-fan · 2021-07-06T16:58:12Z

thanks, merging to master/3.2 (to improve test coverage)

### What changes were proposed in this pull request? Add a config `spark.sql.join.forceApplyShuffledHashJoin` to force applying shuffled hash join during the join selection. ### Why are the changes needed? In the `SQLQueryTestSuite`, we want to cover 3 kinds of join (BHJ, SHJ, SMJ) in join.sql. But even if the `spark.sql.join.preferSortMergeJoin` is set to `false`, shuffled hash join is still not guaranteed. Thus, we need another config to force the selection. ### Does this PR introduce _any_ user-facing change? No, only for testing ### How was this patch tested? newly added tests Verified all queries in join.sql will use `ShuffledHashJoin` when the config set to `true` Closes #33182 from linhongliu-db/SPARK-35984-hash-join-config. Authored-by: Linhong Liu <[email protected]> Signed-off-by: Wenchen Fan <[email protected]> (cherry picked from commit 7566db6) Signed-off-by: Wenchen Fan <[email protected]>

SparkQA · 2021-07-06T20:12:20Z

Test build #140710 has finished for PR 33182 at commit 81fdaae.

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

…in test in-joins.sql ### What changes were proposed in this pull request? We found the `in-join.sql` does not test shuffled hash join properly in https://issues.apache.org/jira/browse/SPARK-32577, but didn't find a good way to fix it. Given we now have a test config to enforce shuffled hash join in #33182, we can fix the test here now as well. ### Why are the changes needed? Fix test to have better test coverage. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Reran the test to compare the output, and verified the query plan manually to make sure shuffled hash join being used. Closes #33236 from c21/join-test. Authored-by: Cheng Su <[email protected]> Signed-off-by: Hyukjin Kwon <[email protected]>

…in test in-joins.sql ### What changes were proposed in this pull request? We found the `in-join.sql` does not test shuffled hash join properly in https://issues.apache.org/jira/browse/SPARK-32577, but didn't find a good way to fix it. Given we now have a test config to enforce shuffled hash join in #33182, we can fix the test here now as well. ### Why are the changes needed? Fix test to have better test coverage. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Reran the test to compare the output, and verified the query plan manually to make sure shuffled hash join being used. Closes #33236 from c21/join-test. Authored-by: Cheng Su <[email protected]> Signed-off-by: Hyukjin Kwon <[email protected]> (cherry picked from commit f3c1159) Signed-off-by: Hyukjin Kwon <[email protected]>

Add a config `spark.sql.join.forceApplyShuffledHashJoin` to force applying shuffled hash join during the join selection. In the `SQLQueryTestSuite`, we want to cover 3 kinds of join (BHJ, SHJ, SMJ) in join.sql. But even if the `spark.sql.join.preferSortMergeJoin` is set to `false`, shuffled hash join is still not guaranteed. Thus, we need another config to force the selection. No, only for testing newly added tests Verified all queries in join.sql will use `ShuffledHashJoin` when the config set to `true` Closes #33182 from linhongliu-db/SPARK-35984-hash-join-config. Authored-by: Linhong Liu <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>

github-actions bot added the SQL label Jul 2, 2021

c21 reviewed Jul 2, 2021

View reviewed changes

linhongliu-db added 3 commits July 3, 2021 17:00

[SPARK-35984][SQL] Config to force applying shuffled hash join

ea219c0

fix test

b197720

address comments

f3474a2

linhongliu-db force-pushed the SPARK-35984-hash-join-config branch from 16a0791 to f3474a2 Compare July 3, 2021 09:00

cloud-fan reviewed Jul 6, 2021

View reviewed changes

code clean

d0dfd8d

cloud-fan reviewed Jul 6, 2021

View reviewed changes

cloud-fan approved these changes Jul 6, 2021

View reviewed changes

move isTesting

81fdaae

cloud-fan changed the title ~~[SPARK-35984][SQL] Config to force applying shuffled hash join~~ [SPARK-35984][SQL][TEST] Config to force applying shuffled hash join Jul 6, 2021

cloud-fan closed this in 7566db6 Jul 6, 2021

c21 mentioned this pull request Jul 6, 2021

[SPARK-32577][SQL][TEST] Fix the config value for shuffled hash join in test in-joins.sql #33236

Closed

singhpk234 mentioned this pull request Mar 12, 2024

feat: Support HashJoin operator apache/datafusion-comet#194

Merged

Comments

Conversation

linhongliu-db commented Jul 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

linhongliu-db commented Jul 2, 2021

Uh oh!

SparkQA commented Jul 2, 2021

Uh oh!

SparkQA commented Jul 2, 2021

Uh oh!

SparkQA commented Jul 2, 2021

Uh oh!

SparkQA commented Jul 2, 2021

Uh oh!

SparkQA commented Jul 2, 2021

Uh oh!

SparkQA commented Jul 2, 2021

Uh oh!

c21 Jul 2, 2021

Choose a reason for hiding this comment

Uh oh!

c21 Jul 2, 2021

Choose a reason for hiding this comment

Uh oh!

c21 Jul 2, 2021

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 3, 2021

Uh oh!

SparkQA commented Jul 3, 2021

Uh oh!

SparkQA commented Jul 3, 2021

Uh oh!

cloud-fan Jul 6, 2021

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 6, 2021

Uh oh!

SparkQA commented Jul 6, 2021

Uh oh!

cloud-fan Jul 6, 2021

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 6, 2021

Uh oh!

SparkQA commented Jul 6, 2021

Uh oh!

SparkQA commented Jul 6, 2021

Uh oh!

cloud-fan commented Jul 6, 2021

Uh oh!

SparkQA commented Jul 6, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

linhongliu-db commented Jul 2, 2021 •

edited

Loading