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.

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

[SPARK-39447][SQL] Avoid AssertionError in AdaptiveSparkPlanExec.doExecuteBroadcast #36974

Closed
wants to merge 1 commit into from

Conversation

ulysses-you
Copy link
Contributor

What changes were proposed in this pull request?

Change currentPhysicalPlan to inputPlan when we restore the broadcast exchange for DPP.

Why are the changes needed?

The currentPhysicalPlan can be wrapped with broadcast query stage so it is not safe to match it. For example:
The broadcast exchange which is added by DPP is running before than the normal broadcast exchange(e.g. introduced by join).

Does this PR introduce any user-facing change?

yes bug fix

How was this patch tested?

add test

@cloud-fan
Copy link
Contributor

also cc @maryannxue

@ulysses-you
Copy link
Contributor Author

@cloud-fan @maryannxue any comments for this patch ? thank you

@cloud-fan
Copy link
Contributor

how far shall we backport this?

@ulysses-you
Copy link
Contributor Author

3.2 should be enough which we began to support AQE +DPP

@cloud-fan cloud-fan closed this in c320a5d Jul 5, 2022
cloud-fan pushed a commit that referenced this pull request Jul 5, 2022
…ecuteBroadcast

### What changes were proposed in this pull request?

Change `currentPhysicalPlan` to `inputPlan ` when we restore the broadcast exchange for DPP.

### Why are the changes needed?

The currentPhysicalPlan can be wrapped with broadcast query stage so it is not safe to match it. For example:
 The broadcast exchange which is added by DPP is running before than the normal broadcast exchange(e.g. introduced by join).

### Does this PR introduce _any_ user-facing change?

yes bug fix

### How was this patch tested?

add test

Closes #36974 from ulysses-you/inputplan.

Authored-by: ulysses-you <ulyssesyou18@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit c320a5d)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@cloud-fan
Copy link
Contributor

thanks, merging to master/3.3!

@ulysses-you can you open a backport pr for 3.2?

@ulysses-you
Copy link
Contributor Author

thank you @cloud-fan , created #37087 for 3.2

@ulysses-you ulysses-you deleted the inputplan branch July 5, 2022 09:39
cloud-fan pushed a commit that referenced this pull request Jul 7, 2022
….doExecuteBroadcast

This is a backport of #36974 for branch-3.2

### What changes were proposed in this pull request?

Change `currentPhysicalPlan` to `inputPlan ` when we restore the broadcast exchange for DPP.

### Why are the changes needed?

The currentPhysicalPlan can be wrapped with broadcast query stage so it is not safe to match it. For example:
 The broadcast exchange which is added by DPP is running before than the normal broadcast exchange(e.g. introduced by join).

### Does this PR introduce _any_ user-facing change?

yes bug fix

### How was this patch tested?

add test

Closes #37087 from ulysses-you/inputplan-3.2.

Authored-by: ulysses-you <ulyssesyou18@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
sunchao pushed a commit to sunchao/spark that referenced this pull request Jun 2, 2023
….doExecuteBroadcast

This is a backport of apache#36974 for branch-3.2

### What changes were proposed in this pull request?

Change `currentPhysicalPlan` to `inputPlan ` when we restore the broadcast exchange for DPP.

### Why are the changes needed?

The currentPhysicalPlan can be wrapped with broadcast query stage so it is not safe to match it. For example:
 The broadcast exchange which is added by DPP is running before than the normal broadcast exchange(e.g. introduced by join).

### Does this PR introduce _any_ user-facing change?

yes bug fix

### How was this patch tested?

add test

Closes apache#37087 from ulysses-you/inputplan-3.2.

Authored-by: ulysses-you <ulyssesyou18@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit 32aff86)
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
lwz9103 pushed a commit to lwz9103/spark that referenced this pull request Apr 26, 2024
…ecuteBroadcast

Change `currentPhysicalPlan` to `inputPlan ` when we restore the broadcast exchange for DPP.

The currentPhysicalPlan can be wrapped with broadcast query stage so it is not safe to match it. For example:
 The broadcast exchange which is added by DPP is running before than the normal broadcast exchange(e.g. introduced by join).

yes bug fix

add test

Closes apache#36974 from ulysses-you/inputplan.

Authored-by: ulysses-you <ulyssesyou18@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
lwz9103 pushed a commit to lwz9103/spark that referenced this pull request May 8, 2024
…ecuteBroadcast

Change `currentPhysicalPlan` to `inputPlan ` when we restore the broadcast exchange for DPP.

The currentPhysicalPlan can be wrapped with broadcast query stage so it is not safe to match it. For example:
 The broadcast exchange which is added by DPP is running before than the normal broadcast exchange(e.g. introduced by join).

yes bug fix

add test

Closes apache#36974 from ulysses-you/inputplan.

Authored-by: ulysses-you <ulyssesyou18@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
lwz9103 pushed a commit to lwz9103/spark that referenced this pull request May 10, 2024
…ecuteBroadcast

Change `currentPhysicalPlan` to `inputPlan ` when we restore the broadcast exchange for DPP.

The currentPhysicalPlan can be wrapped with broadcast query stage so it is not safe to match it. For example:
 The broadcast exchange which is added by DPP is running before than the normal broadcast exchange(e.g. introduced by join).

yes bug fix

add test

Closes apache#36974 from ulysses-you/inputplan.

Authored-by: ulysses-you <ulyssesyou18@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
lwz9103 pushed a commit to lwz9103/spark that referenced this pull request May 10, 2024
…ecuteBroadcast

### What changes were proposed in this pull request?

Change `currentPhysicalPlan` to `inputPlan ` when we restore the broadcast exchange for DPP.

### Why are the changes needed?

The currentPhysicalPlan can be wrapped with broadcast query stage so it is not safe to match it. For example:
 The broadcast exchange which is added by DPP is running before than the normal broadcast exchange(e.g. introduced by join).

### Does this PR introduce _any_ user-facing change?

yes bug fix

### How was this patch tested?

add test

Closes apache#36974 from ulysses-you/inputplan.

Authored-by: ulysses-you <ulyssesyou18@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit c320a5d)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit 3e28f33)
lwz9103 pushed a commit to Kyligence/spark that referenced this pull request May 13, 2024
…ecuteBroadcast

### What changes were proposed in this pull request?

Change `currentPhysicalPlan` to `inputPlan ` when we restore the broadcast exchange for DPP.

### Why are the changes needed?

The currentPhysicalPlan can be wrapped with broadcast query stage so it is not safe to match it. For example:
 The broadcast exchange which is added by DPP is running before than the normal broadcast exchange(e.g. introduced by join).

### Does this PR introduce _any_ user-facing change?

yes bug fix

### How was this patch tested?

add test

Closes apache#36974 from ulysses-you/inputplan.

Authored-by: ulysses-you <ulyssesyou18@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit c320a5d)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit 3e28f33)
lwz9103 pushed a commit to Kyligence/spark that referenced this pull request Jun 5, 2024
…ecuteBroadcast

### What changes were proposed in this pull request?

Change `currentPhysicalPlan` to `inputPlan ` when we restore the broadcast exchange for DPP.

### Why are the changes needed?

The currentPhysicalPlan can be wrapped with broadcast query stage so it is not safe to match it. For example:
 The broadcast exchange which is added by DPP is running before than the normal broadcast exchange(e.g. introduced by join).

### Does this PR introduce _any_ user-facing change?

yes bug fix

### How was this patch tested?

add test

Closes apache#36974 from ulysses-you/inputplan.

Authored-by: ulysses-you <ulyssesyou18@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit c320a5d)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit 3e28f33)
lwz9103 pushed a commit to lwz9103/spark that referenced this pull request Jul 3, 2024
…ecuteBroadcast

### What changes were proposed in this pull request?

Change `currentPhysicalPlan` to `inputPlan ` when we restore the broadcast exchange for DPP.

### Why are the changes needed?

The currentPhysicalPlan can be wrapped with broadcast query stage so it is not safe to match it. For example:
 The broadcast exchange which is added by DPP is running before than the normal broadcast exchange(e.g. introduced by join).

### Does this PR introduce _any_ user-facing change?

yes bug fix

### How was this patch tested?

add test

Closes apache#36974 from ulysses-you/inputplan.

Authored-by: ulysses-you <ulyssesyou18@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit c320a5d)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit 3e28f33)
lwz9103 pushed a commit to Kyligence/spark that referenced this pull request Jul 4, 2024
…ecuteBroadcast

### What changes were proposed in this pull request?

Change `currentPhysicalPlan` to `inputPlan ` when we restore the broadcast exchange for DPP.

### Why are the changes needed?

The currentPhysicalPlan can be wrapped with broadcast query stage so it is not safe to match it. For example:
 The broadcast exchange which is added by DPP is running before than the normal broadcast exchange(e.g. introduced by join).

### Does this PR introduce _any_ user-facing change?

yes bug fix

### How was this patch tested?

add test

Closes apache#36974 from ulysses-you/inputplan.

Authored-by: ulysses-you <ulyssesyou18@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit c320a5d)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit 3e28f33)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants