Uh oh!
There was an error while loading. Please reload this page.
[feat](nereids)using salt-join automatically when encountering skew join - #54207
Conversation
Thearas
commented
Aug 1, 2025
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
englefly
commented
Aug 5, 2025
run buildall |
doris-robot
commented
Aug 5, 2025
TPC-H: Total hot run time: 33512 ms |
doris-robot
commented
Aug 5, 2025
TPC-DS: Total hot run time: 160552 ms |
doris-robot
commented
Aug 5, 2025
ClickBench: Total hot run time: 33.04 s |
englefly
commented
Aug 7, 2025
run buildall |
1 similar comment
englefly
commented
Aug 8, 2025
run buildall |
doris-robot
commented
Aug 8, 2025
TPC-H: Total hot run time: 33514 ms |
doris-robot
commented
Aug 8, 2025
TPC-DS: Total hot run time: 169861 ms |
doris-robot
commented
Aug 8, 2025
ClickBench: Total hot run time: 32.89 s |
englefly
commented
Aug 11, 2025
run buildall |
doris-robot
commented
Aug 11, 2025
TPC-H: Total hot run time: 33698 ms |
englefly
commented
Aug 11, 2025
run buildall |
doris-robot
commented
Aug 11, 2025
TPC-H: Total hot run time: 34138 ms |
englefly
commented
Aug 13, 2025
run buildall |
doris-robot
commented
Aug 13, 2025
TPC-H: Total hot run time: 33817 ms |
doris-robot
commented
Aug 13, 2025
TPC-DS: Total hot run time: 184905 ms |
doris-robot
commented
Aug 13, 2025
ClickBench: Total hot run time: 32.18 s |
hello-stephen
commented
Aug 13, 2025
FE UT Coverage ReportIncrement line coverage |
englefly
commented
Aug 14, 2025
run external |
| DistributeHint hint = new DistributeHint(DistributeType.SHUFFLE_RIGHT, | ||
| new JoinSkewInfo(skewExpr, hotValues, false)); | ||
| join.setHint(hint); | ||
| return SaltJoin.transform(join); |
There was a problem hiding this comment.
The current position of the saltJoin rule in the rewriter is placed after eliminate join. This is to prevent cases where some joins that could originally be eliminated (e.g., EliminateJoinByUnique) might no longer be eliminable after being rewritten by saltJoin. The code calling SaltJoin here could cause this problem. Is it possible to add the hint directly without applying SaltJoin.transform(join)? Then, when a salt join is encountered later and the pattern matches, the saltJoin rule will automatically be applied?
There was a problem hiding this comment.
this rule also need to be applies after EliminateJoinByUnique.
| * add salt | ||
| */ | ||
| public static Plan transform(LogicalJoin<Plan, Plan> join) { | ||
| DistributeHint hint = join.getDistributeHint(); |
There was a problem hiding this comment.
maybe we should check ConnectContext.get() != null
englefly
commented
Aug 15, 2025
run buildall |
doris-robot
commented
Aug 15, 2025
TPC-H: Total hot run time: 33775 ms |
doris-robot
commented
Aug 15, 2025
TPC-DS: Total hot run time: 184165 ms |
doris-robot
commented
Aug 15, 2025
ClickBench: Total hot run time: 32.65 s |
englefly
commented
Aug 19, 2025
run cloud_p0 |
PR approved by anyone and no changes requested. |
PR approved by at least one committer and no changes requested. |
Uh oh!
There was an error while loading. Please reload this page.
### What problem does this PR solve? Related PR: #59591#54207 Problem Summary: This PR restricts the auto salt join rewrite in two cases. First, ASOF joins are excluded from automatic salt join optimization because the current rewrite may introduce large build-side expansion and is not cost guarded for ASOF semantics. Second, SaltJoin now marks internally generated helper joins with `isSaltJoinGenerated` in `JoinReorderContext`. `SkewJoin` skips these generated joins to avoid applying salt rewrite recursively on SaltJoin's own expansion plan.
What problem does this PR solve?
When encountering a data-skewed join, there are currently two optimization methods: using salt-join or using broadcast join.
If we detect data skew during the RBO phase and the right table is relatively large, we will automatically add salt.
If no salt is automatically added during the RBO phase, we will try to choose broadcast join based on cost during the CBO phase.
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)