[SPARK-44000][SQL] Add hint to disable broadcasting and replicating one side of join #41499

aokolnychyi · 2023-06-07T16:00:12Z

What changes were proposed in this pull request?

This PR adds a new internal join hint to disable broadcasting and replicating one side of join.

Why are the changes needed?

These changes are needed to disable broadcasting and replicating one side of join when it is not permitted, such as the cardinality check in MERGE operations in PR #41448.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

This PR comes with tests. More tests are in #41448.

…ne side of join

aokolnychyi · 2023-06-07T16:02:31Z

cc @dongjoon-hyun

dongjoon-hyun · 2023-06-07T16:04:27Z

Thank you, @aokolnychyi !

sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala

dongjoon-hyun

+1, LGTM (only minor comment). Thank you again, @aokolnychyi .

dongjoon-hyun

Also, cc @sunchao , @viirya , @huaxingao , too

aokolnychyi · 2023-06-07T18:28:19Z

@dongjoon-hyun, I'll submit a follow-up PR with the comments.

viirya · 2023-06-07T18:28:27Z

sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala

@@ -354,27 +367,29 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
        }

        def createCartesianProduct() = {
-          if (joinType.isInstanceOf[InnerLike]) {
+          if (joinType.isInstanceOf[InnerLike] && !hintToNotBroadcastAndReplicate(hint)) {


Is CartesianProduct related to broadcast?

It is related to replication. If I am correct, it would zip each partition on one side with each partition on the other side so the same target record would appear in multiple places.

Oh, this is for replicating?

EDIT: saw your reply after adding this comment.

The new hint prohibits both broadcasting and replication so it applies to 3 cases:

Broadcast hash

Cartesian product

BNLJ build side

It would be nice to update this to the pr description too.

dongjoon-hyun · 2023-06-07T18:30:29Z

Thank you, @viirya and @aokolnychyi . Now, pending CIs.

dongjoon-hyun · 2023-06-07T19:09:05Z

It seems that sql test pipeline is killed by receiving shutdown signal somehow.

...
2023-06-07T18:39:01.1867247Z ##[error]The runner has received a shutdown signal. This can happen when the runner service is stopped, or a manually started runner is canceled.
2023-06-07T18:39:01.5173829Z Session terminated, killing shell...
2023-06-07T18:39:01.7418237Z ##[error]The operation was canceled.

dongjoon-hyun · 2023-06-07T21:50:45Z

When you have a chance, could you re-trigger the failed test pipeline?

aokolnychyi · 2023-06-07T22:25:16Z

Triggered again: https://github.com/aokolnychyi/spark/actions/runs/5202190900

dongjoon-hyun · 2023-06-07T22:29:19Z

Thank you so much!

dongjoon-hyun · 2023-06-08T03:14:35Z

Unfortunately, master branch still has the issue on sql - other tests module and the last test of this PR seems to be running over 4h 40minutes as of now.

This PR passed catalyst and all hive modules. I verified JoinSuite (and new test cases) manually.

$ build/sbt "sql/testOnly *.JoinSuite"
...
[info] JoinSuite:
20:13:26.224 WARN org.apache.hadoop.util.NativeCodeLoader: Unable to load native-hadoop library for your platform... using builtin-java classes where applicable
[info] - equi-join is hash-join (69 milliseconds)
[info] - NO_BROADCAST_AND_REPLICATION hint is respected in cross joins (19 milliseconds)
...
[info] Run completed in 25 seconds, 333 milliseconds.
[info] Total number of tests run: 50
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 50, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
[success] Total time: 60 s, completed Jun 7, 2023, 8:13:50 PM

dongjoon-hyun · 2023-06-08T03:15:33Z

Thank you, @aokolnychyi and @viirya .

aokolnychyi · 2023-06-08T05:29:51Z

Thank you, @dongjoon-hyun @viirya! I've created #41509 to add comments.

…ne side of join ### What changes were proposed in this pull request? This PR adds a new internal join hint to disable broadcasting and replicating one side of join. ### Why are the changes needed? These changes are needed to disable broadcasting and replicating one side of join when it is not permitted, such as the cardinality check in MERGE operations in PR apache#41448. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? This PR comes with tests. More tests are in apache#41448. Closes apache#41499 from aokolnychyi/spark-44000. Authored-by: aokolnychyi <aokolnychyi@apple.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

andylam-db · 2023-06-12T17:41:06Z

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

+  def getBroadcastNestedLoopJoinBuildSide(hint: JoinHint): Option[BuildSide] = {
+    if (hintToNotBroadcastAndReplicateLeft(hint)) {
+      Some(BuildRight)
+    } else if (hintToNotBroadcastAndReplicateRight(hint)) {
+      Some(BuildLeft)
+    } else {
+      None
+    }
+  }


This logic is saying that if we hint to NOT broadcast left, we can broadcast right, and vice-versa. Shouldn't we check the hint on the other side as well?
i.e.

if (hintToNotBroadcastAndReplicateLeft(hint) && !hintToNotBroadcastAndReplicateRight(hint)) { Some(BuildRight) } else if (hintToNotBroadcastAndReplicateRight(hint) && !hintToNotBroadcastAndReplicateLeft(hint)) { Some(BuildLeft) } else { None }

cc @aokolnychyi

At the moment, the new hint can be only set on one side and never on both. BNLJ is considered as the default join strategy and having no broadcast and replicate hints on both sides would mean there is no applicable fallback join strategy to use. If we were to adapt the method above, we can't keep the existing default logic that picks the broadcast side based on size (that could cause a correctness problem). What about adding validation the new hint is set only on one side?

What about adding validation the new hint is set only on one side?
Sounds good.

Cool, I will work on it. Created SPARK-44148.

…ne side of join ### What changes were proposed in this pull request? This PR adds a new internal join hint to disable broadcasting and replicating one side of join. ### Why are the changes needed? These changes are needed to disable broadcasting and replicating one side of join when it is not permitted, such as the cardinality check in MERGE operations in PR apache#41448. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? This PR comes with tests. More tests are in apache#41448. Closes apache#41499 from aokolnychyi/spark-44000. Authored-by: aokolnychyi <aokolnychyi@apple.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit d88633a)

[SPARK-44000][SQL] Add hint to disable broadcasting and replicating o…

ce393f8

…ne side of join

github-actions bot added the SQL label Jun 7, 2023

aokolnychyi mentioned this pull request Jun 7, 2023

[SPARK-43885][SQL] DataSource V2: Handle MERGE commands for delta-based sources #41448

Closed

dongjoon-hyun reviewed Jun 7, 2023

View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala Show resolved Hide resolved

dongjoon-hyun approved these changes Jun 7, 2023

View reviewed changes

dongjoon-hyun reviewed Jun 7, 2023

View reviewed changes

viirya reviewed Jun 7, 2023

View reviewed changes

viirya approved these changes Jun 7, 2023

View reviewed changes

dongjoon-hyun closed this in d88633a Jun 8, 2023

andylam-db reviewed Jun 12, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-44000][SQL] Add hint to disable broadcasting and replicating one side of join #41499

[SPARK-44000][SQL] Add hint to disable broadcasting and replicating one side of join #41499

aokolnychyi commented Jun 7, 2023

aokolnychyi commented Jun 7, 2023

dongjoon-hyun commented Jun 7, 2023

dongjoon-hyun left a comment

dongjoon-hyun left a comment

aokolnychyi commented Jun 7, 2023

viirya Jun 7, 2023

aokolnychyi Jun 7, 2023 •

edited

Loading

viirya Jun 7, 2023 •

edited

Loading

aokolnychyi Jun 7, 2023

viirya Jun 7, 2023

dongjoon-hyun commented Jun 7, 2023

dongjoon-hyun commented Jun 7, 2023

dongjoon-hyun commented Jun 7, 2023

aokolnychyi commented Jun 7, 2023

dongjoon-hyun commented Jun 7, 2023

dongjoon-hyun commented Jun 8, 2023

dongjoon-hyun commented Jun 8, 2023

aokolnychyi commented Jun 8, 2023

andylam-db Jun 12, 2023

dongjoon-hyun Jun 12, 2023

aokolnychyi Jun 13, 2023 •

edited

Loading

andylam-db Jun 21, 2023

aokolnychyi Jun 22, 2023

[SPARK-44000][SQL] Add hint to disable broadcasting and replicating one side of join #41499

[SPARK-44000][SQL] Add hint to disable broadcasting and replicating one side of join #41499

Conversation

aokolnychyi commented Jun 7, 2023

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?

aokolnychyi commented Jun 7, 2023

dongjoon-hyun commented Jun 7, 2023

dongjoon-hyun left a comment

Choose a reason for hiding this comment

dongjoon-hyun left a comment

Choose a reason for hiding this comment

aokolnychyi commented Jun 7, 2023

viirya Jun 7, 2023

Choose a reason for hiding this comment

aokolnychyi Jun 7, 2023 • edited Loading

Choose a reason for hiding this comment

viirya Jun 7, 2023 • edited Loading

Choose a reason for hiding this comment

aokolnychyi Jun 7, 2023

Choose a reason for hiding this comment

viirya Jun 7, 2023

Choose a reason for hiding this comment

dongjoon-hyun commented Jun 7, 2023

dongjoon-hyun commented Jun 7, 2023

dongjoon-hyun commented Jun 7, 2023

aokolnychyi commented Jun 7, 2023

dongjoon-hyun commented Jun 7, 2023

dongjoon-hyun commented Jun 8, 2023

dongjoon-hyun commented Jun 8, 2023

aokolnychyi commented Jun 8, 2023

andylam-db Jun 12, 2023

Choose a reason for hiding this comment

dongjoon-hyun Jun 12, 2023

Choose a reason for hiding this comment

aokolnychyi Jun 13, 2023 • edited Loading

Choose a reason for hiding this comment

andylam-db Jun 21, 2023

Choose a reason for hiding this comment

aokolnychyi Jun 22, 2023

Choose a reason for hiding this comment

aokolnychyi Jun 7, 2023 •

edited

Loading

viirya Jun 7, 2023 •

edited

Loading

aokolnychyi Jun 13, 2023 •

edited

Loading