branch:4.2: [fix](fe) Move SemiJoinCommute to rewrite phase #66182 - #68288
Merged
Merged
Conversation
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: SemiJoinCommute structurally normalizes RIGHT OUTER, RIGHT SEMI, and RIGHT ANTI joins. Running it during analysis made analyzed plan shapes depend on join-reorder state, so consumers that identify analyzed plans could observe inconsistent signatures for the same query. Move the rule to both regular and CTE rewrite pipelines before the semi-join transpose rules, keeping analysis stable while preserving rewrite assumptions.
### Release note
None
### Check List (For Author)
- Test: Unit Test
- ReorderJoinTest
- SaltJoinTest
- Behavior changed: Yes. Analyze preserves RIGHT JOIN shape and Rewrite performs the existing normalization.
- Does this need documentation: No
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: SemiJoinCommute now runs in rewrite pipelines instead of analysis. Add coverage for the disable_join_reorder boundary, the materialized-view pre-rewrite pipeline, and the ordering between SemiJoinCommute and semi-join transpose rules.
### Release note
None
### Check List (For Author)
- Test: Unit Test
- ReorderJoinTest
- SaltJoinTest
- Behavior changed: No
- Does this need documentation: No
Issue Number: None Related PR: apache#66182 Problem Summary: Moving SemiJoinCommute out of analysis exposed that CollectJoinConstraint assumed right outer, semi, and anti joins had already been normalized to left joins. A preserved-side ON predicate could then be treated as a single-table filter and pushed below the join, changing query results. Canonicalize right-join constraint metadata to the existing left-join model and update unit-test expectations for the equivalent child order produced by the new rewrite stage. Fix incorrect LEADING query results for right outer, semi, and anti joins. - Test: Regression test / Unit Test - InferTest, ReorderJoinTest, SaltJoinTest - query_p0/hint/fix_leading - query_p0/hint/test_leading - Behavior changed: Yes, LEADING no longer pushes preserved-side right join ON predicates below the join. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#66182 Problem Summary: Moving SemiJoinCommute out of analysis exposes a pre-existing LEADING dependency on Analyzer normalizing right joins to left joins. The attempted hint fix is independent of the rewrite-stage migration and needs dedicated coverage for right outer, semi, and anti joins. Remove that hint implementation from this PR and temporarily disable only the failing select4_2 regression case until the follow-up fix is ready. ### Release note None ### Check List (For Author) - Test: Regression test / Unit Test - ./run-fe-ut.sh --run org.apache.doris.nereids.sqltest.InferTest,org.apache.doris.nereids.rules.rewrite.ReorderJoinTest,org.apache.doris.nereids.rules.rewrite.SaltJoinTest - ./run-regression-test.sh --run -d query_p0 -s fix_leading - Behavior changed: Yes, defer LEADING handling for original right joins to a follow-up PR and temporarily skip its failing regression case. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#66182 Problem Summary: Moving SemiJoinCommute out of analysis exposes a pre-existing LEADING dependency on Analyzer normalizing right joins. Temporarily disable the corresponding select4_3 shape assertion alongside select4_2 until the independent LEADING fix is ready. ### Release note None ### Check List (For Author) - Test: Regression test - Verified the P0 failure is isolated to select4_3; select2_2 and select2_5 complete before it - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#66182 Problem Summary: Moving SemiJoinCommute out of analysis exposes that LEADING constraint collection assumes right joins have already been normalized to left joins. RIGHT OUTER and RIGHT ANTI preserved-side predicates can then be pushed below their scans, while a RIGHT SEMI join can consume its condition before the complete non-output side is built and duplicate output rows. Canonicalize right-join constraint metadata to the equivalent left-join model without changing the analyzed plan, and restore or add focused regression coverage. ### Release note Fix incorrect LEADING query results for right outer, semi, and anti joins. ### Check List (For Author) - Test: Unit Test - ./run-fe-ut.sh --run org.apache.doris.nereids.rules.rewrite.ReorderJoinTest,org.apache.doris.nereids.sqltest.SqlPlanSuiteTest (34 tests passed) - Regression test not run yet; waiting for FE to be built and restarted with this commit - Behavior changed: Yes. LEADING preserves right-join boundaries and ON predicate semantics. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#66182 Problem Summary: Refresh the fix_leading expected output after restoring right outer join coverage and adding focused right semi and right anti join cases. The generated results verify correct row counts and preserve the complete non-output side of the right semi join. ### Release note None ### Check List (For Author) - Test: Regression test - ./run-regression-test.sh --run -d query_p0 -s fix_leading -forceGenOut - ./run-regression-test.sh --run -d query_p0 -s fix_leading - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#66182 Problem Summary: The right semi join regression test compared a complete physical plan shape even though equivalent CROSS JOIN children may be reordered by cost and statistics. Remove the unstable shape assertion while retaining result checks that reproduce the incorrect right semi and right anti join behavior. ### Release note None ### Check List (For Author) - Test: Regression test - ./run-regression-test.sh --run -d query_p0 -s fix_leading -forceGenOut - ./run-regression-test.sh --run -d query_p0 -s fix_leading - Behavior changed: No - Does this need documentation: No
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
morrySnow
changed the title
Contributor
Author
|
run buildall |
Contributor
FE UT Coverage ReportIncrement line coverage |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
picked from #66182