[FOLLOW-UP][SPARK-26065][SQL] Revert hint behavior in join reordering #23524
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maryannxue
changed the title
|
Test build #101103 has finished for PR 23524 at commit
|
|
retest this please |
|
Test build #101110 has finished for PR 23524 at commit
|
|
Retest this please. |
|
Test build #101115 has finished for PR 23524 at commit
|
|
Retest this please. |
|
Test build #101121 has finished for PR 23524 at commit
|
gatorsmile
reviewed
Jan 13, 2019
|
LGTM Thanks! Merged to master. |
|
+1 for this. I was reading the PR #23036 and wondering if that's okay. |
jackylee-ch
pushed a commit
to jackylee-ch/spark
that referenced
this pull request
Feb 18, 2019
## What changes were proposed in this pull request? This is to fix a bug in apache#23036 that would cause a join hint to be applied on node it is not supposed to after join reordering. For example, ``` val join = df.join(df, "id") val broadcasted = join.hint("broadcast") val join2 = join.join(broadcasted, "id").join(broadcasted, "id") ``` There should only be 2 broadcast hints on `join2`, but after join reordering there would be 4. It is because the hint application in join reordering compares the attribute set for testing relation equivalency. Moreover, it could still be problematic even if the child relations were used in testing relation equivalency, due to the potential exprId conflict in nested self-join. As a result, this PR simply reverts the join reorder hint behavior change introduced in apache#23036, which means if a join hint is present, the join node itself will not participate in the join reordering, while the sub-joins within its children still can. ## How was this patch tested? Added new tests Closes apache#23524 from maryannxue/query-hint-followup-2. Authored-by: maryannxue <maryannxue@apache.org> Signed-off-by: gatorsmile <gatorsmile@gmail.com>
jackylee-ch
pushed a commit
to jackylee-ch/spark
that referenced
this pull request
Feb 18, 2019
…hints ## What changes were proposed in this pull request? This is a fix for apache#23524, which did not stop cost-based join reorder when the CostBasedJoinReorder rule recurses down the tree and applies join reorder for nested joins with hints. The issue had not been detected by the existing tests because CBO is disabled by default. ## How was this patch tested? Enabled CBO for JoinHintSuite. Closes apache#23759 from maryannxue/spark-26840. Lead-authored-by: maryannxue <maryannxue@apache.org> Co-authored-by: Dongjoon Hyun <dhyun@apple.com> Signed-off-by: gatorsmile <gatorsmile@gmail.com>
j-baker
pushed a commit
to palantir/spark
that referenced
this pull request
Jan 25, 2020
…hints ## What changes were proposed in this pull request? This is a fix for apache#23524, which did not stop cost-based join reorder when the CostBasedJoinReorder rule recurses down the tree and applies join reorder for nested joins with hints. The issue had not been detected by the existing tests because CBO is disabled by default. ## How was this patch tested? Enabled CBO for JoinHintSuite. Closes apache#23759 from maryannxue/spark-26840. Lead-authored-by: maryannxue <maryannxue@apache.org> Co-authored-by: Dongjoon Hyun <dhyun@apple.com> Signed-off-by: gatorsmile <gatorsmile@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
This is to fix a bug in #23036 that would cause a join hint to be applied on node it is not supposed to after join reordering. For example,
There should only be 2 broadcast hints on
join2, but after join reordering there would be 4. It is because the hint application in join reordering compares the attribute set for testing relation equivalency.Moreover, it could still be problematic even if the child relations were used in testing relation equivalency, due to the potential exprId conflict in nested self-join.
As a result, this PR simply reverts the join reorder hint behavior change introduced in #23036, which means if a join hint is present, the join node itself will not participate in the join reordering, while the sub-joins within its children still can.
How was this patch tested?
Added new tests