Uh oh!
There was an error while loading. Please reload this page.
[fix](join) Restrict auto salt join rewrite - #64518
Conversation
hello-stephen
commented
Jun 15, 2026
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
feiniaofeiafei
commented
Jun 15, 2026
run buildall |
hello-stephen
commented
Jun 15, 2026
TPC-H: Total hot run time: 29052 ms |
hello-stephen
commented
Jun 15, 2026
TPC-DS: Total hot run time: 168401 ms |
hello-stephen
commented
Jun 15, 2026
FE UT Coverage ReportIncrement line coverage |
hello-stephen
commented
Jun 15, 2026
FE Regression Coverage ReportIncrement line coverage |
PR approved by at least one committer and no changes requested. |
PR approved by anyone and no changes requested. |
morrySnow
commented
Jun 16, 2026
/review |
There was a problem hiding this comment.
Reviewed the four-file PR scope against the repository code-review checklist. I did not find a blocking correctness issue.
Checkpoint conclusions:
- Goal: The change prevents
SkewJoinfrom recursively applying automatic salt rewrite to SaltJoin-generated helper joins, and removes ASOF joins from automatic skew rewrite. The code accomplishes this:SaltJoinmarks the generated RIGHT_OUTER_JOIN helper,LogicalJoincopies the context, andSkewJoinskips marked joins;JoinType.isOneSideOuterJoin()does not include ASOF joins, so ASOF is excluded from the rule entry predicate. - Scope: The implementation is small and focused on the relevant rule guard and metadata propagation.
- Concurrency/lifecycle: No runtime concurrency or special lifecycle concerns; this is optimizer plan metadata carried inside per-plan objects.
- Configuration/compatibility/persistence: No new config, wire format, storage format, or persistence compatibility impact.
- Parallel paths: Explicit
SaltJoinremains available for ASOF hint paths; automaticSkewJoinis the only path being restricted, which matches the PR description. - Tests: Existing
SaltJoinTestchecks the generated-helper marker for explicit salt-hint right joins. I did not see a focused test that creates hot-value stats and proves the automaticSkewJoinpass skips the generated helper, nor one for ASOF auto exclusion; this is a residual coverage gap but I am not treating it as blocking for this small guard change. - Observability/performance: No new observability is needed. The added predicate avoids redundant recursive rewrite work and should not add meaningful overhead.
User focus: no additional review focus was provided.
Uh oh!
There was an error while loading. Please reload this page.
What problem does this PR solve?
Issue Number: close #xxx
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
isSaltJoinGeneratedinJoinReorderContext.SkewJoinskips these generated joins to avoid applying salt rewrite recursively on SaltJoin's own expansion plan.Release note
None
Check List (For Author)