From bf2a4389e05f9db05c6427a52bf70108bdd90d85 Mon Sep 17 00:00:00 2001 From: lichi Date: Fri, 10 Jul 2026 18:28:23 +0800 Subject: [PATCH 1/5] [fix](fe) Fix silent semi/anti join drop caused by constrained side check skip in leading hint --- .../doris/nereids/hint/LeadingHint.java | 75 ++++++- .../doris/nereids/hint/LeadingHintTest.java | 201 +++++++++++++++++- 2 files changed, 260 insertions(+), 16 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java index 45df90d3d1d091..82226330de9609 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java @@ -65,7 +65,7 @@ public class LeadingHint extends Hint { private final List> filters = new ArrayList<>(); - private final Map conditionJoinType = Maps.newLinkedHashMap(); + private final Map> conditionJoinType = Maps.newLinkedHashMap(); private final List joinConstraintList = new ArrayList<>(); @@ -283,8 +283,24 @@ public List> getFilters() { return filters; } + /** + * Record the original join type for a filter expression. The same expression + * may appear as a condition in multiple joins with different types (e.g., + * both a LEFT SEMI JOIN and a later INNER JOIN). Using a Set preserves all + * types so that isConditionJoinTypeMatched can correctly validate each join. + * + * Example: + * SELECT ... FROM a1 JOIN a2 ON a2.c1 = a2.c1 + * LEFT SEMI JOIN a3 ON a2.c2 = a2.c1 + * JOIN a5 ON a2.c2 = a2.c1; + * The expression "a2.c2 = a2.c1" is the ON condition for both the + * LEFT SEMI JOIN (a3) and the INNER JOIN (a5). When CollectJoinConstraint + * processes them bottom-up, it first records LEFT_SEMI_JOIN, then + * INNER_JOIN. A plain HashMap.put() would overwrite LEFT_SEMI_JOIN, + * disabling the safety check in isConditionJoinTypeMatched. + */ public void putConditionJoinType(Expression filter, JoinType joinType) { - conditionJoinType.put(filter, joinType); + conditionJoinType.computeIfAbsent(filter, k -> new HashSet<>()).add(joinType); } /** @@ -295,14 +311,23 @@ public void putConditionJoinType(Expression filter, JoinType joinType) { */ public boolean isConditionJoinTypeMatched(List conditions, JoinType joinType) { for (Expression condition : conditions) { - JoinType originalJoinType = conditionJoinType.get(condition); - if (originalJoinType.equals(joinType) - || originalJoinType.isOneSideOuterJoin() && joinType.isOneSideOuterJoin() - || originalJoinType.isSemiJoin() && joinType.isSemiJoin() - || originalJoinType.isAntiJoin() && joinType.isAntiJoin()) { + Set originalJoinTypes = conditionJoinType.get(condition); + if (originalJoinTypes == null) { continue; } - return false; + boolean matched = false; + for (JoinType originalJoinType : originalJoinTypes) { + if (originalJoinType.equals(joinType) + || originalJoinType.isOneSideOuterJoin() && joinType.isOneSideOuterJoin() + || originalJoinType.isSemiJoin() && joinType.isSemiJoin() + || originalJoinType.isAntiJoin() && joinType.isAntiJoin()) { + matched = true; + break; + } + } + if (!matched) { + return false; + } } return true; } @@ -416,6 +441,36 @@ public Pair getJoinConstraint(Long joinTableBitmap, Lon continue; } + // Semi/anti join constraints require the constrained side (the right + // table for left semi/anti, or the left table for right semi/anti) + // to remain intact on one child — it cannot be split across children + // or mixed with extra tables. The first guard (isSubset + // minLeftHand/minRightHand) ensures the constraint is only evaluated + // when all required tables are present in the current join. Once + // present, if the constrained side is violated, the leading hint can + // never satisfy this constraint (tables only merge, never split), so + // we must return failure immediately. + // + // Example 1 — LEFT SEMI JOIN where the constrained side gets mixed: + // SELECT /*+ leading(a2 a5 { a3 a1 }) */ ... + // FROM a1 JOIN a2 ON ... + // LEFT SEMI JOIN a3 ON a2.c2 = a2.c1 + // JOIN a5 ON a2.c2 = a2.c1; + // Original tree: ((a1 JOIN a2) LEFT SEMI JOIN a3) JOIN a5 + // Leading asks: ((a2 JOIN a5) JOIN (a3 JOIN a1)) + // At the top join: left={a2,a5}, right={a1,a3} + // Constrained side (right table a3 for LEFT SEMI) is mixed with + // a1 on the right child → violated, leading must be UNUSED. + // + // Example 2 — RIGHT ANTI JOIN where constrained side gets mixed: + // SELECT /*+ leading(a3 a1 a2) */ ... + // FROM a1 RIGHT ANTI JOIN a2 ON a1.c4 = a1.c + // JOIN a3 ON a2.c = a3.c3; + // Original tree: (a1 RIGHT ANTI JOIN a2) JOIN a3 + // Leading asks: ((a3 JOIN a1) JOIN a2) + // At the top join: left={a1,a3}, right={a2} + // Constrained side (left table a1 for RIGHT ANTI) is mixed with + // a3 → violated, leading must be UNUSED. if (joinConstraint.getJoinType().isSemiOrAntiJoin()) { if (!LongBitmap.isSubset(joinConstraint.getMinLeftHand(), joinTableBitmap) || !LongBitmap.isSubset(joinConstraint.getMinRightHand(), joinTableBitmap)) { @@ -426,11 +481,11 @@ public Pair getJoinConstraint(Long joinTableBitmap, Lon ? joinConstraint.getLeftHand() : joinConstraint.getRightHand(); if (LongBitmap.isOverlap(constrainedSide, leftTableBitmap) && !constrainedSide.equals(leftTableBitmap)) { - continue; + return Pair.of(null, false); } if (LongBitmap.isOverlap(constrainedSide, rightTableBitmap) && !constrainedSide.equals(rightTableBitmap)) { - continue; + return Pair.of(null, false); } } diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/hint/LeadingHintTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/hint/LeadingHintTest.java index 00f0c45437f091..4ed5d6a9d7f048 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/hint/LeadingHintTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/hint/LeadingHintTest.java @@ -19,11 +19,16 @@ import org.apache.doris.common.Pair; import org.apache.doris.nereids.jobs.joinorder.hypergraph.bitmap.LongBitmap; +import org.apache.doris.nereids.trees.expressions.Expression; +import org.apache.doris.nereids.trees.expressions.literal.IntegerLiteral; import org.apache.doris.nereids.trees.plans.JoinType; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import java.util.Arrays; +import java.util.Collections; + public class LeadingHintTest { @Test @@ -47,7 +52,7 @@ public void testLeftSemiJoinConstrainedSideMatchesExactly() { LongBitmap.newBitmapUnion(leftHand, rightHand, extraTable), LongBitmap.newBitmapUnion(rightHand, extraTable), leftHand); - assertNoMatchedJoinConstraint(withExtraTable); + assertConstraintViolated(withExtraTable); } @Test @@ -67,7 +72,7 @@ public void testLeftAntiJoinConstrainedSideMatchesExactly() { LongBitmap.newBitmapUnion(leftHand, rightHand, extraTable), LongBitmap.newBitmapUnion(rightHand, extraTable), leftHand); - assertNoMatchedJoinConstraint(withExtraTable); + assertConstraintViolated(withExtraTable); } @Test @@ -91,7 +96,7 @@ public void testRightSemiJoinConstrainedSideMatchesExactly() { LongBitmap.newBitmapUnion(leftHand, rightHand, extraTable), LongBitmap.newBitmapUnion(leftHand, extraTable), rightHand); - assertNoMatchedJoinConstraint(withExtraTable); + assertConstraintViolated(withExtraTable); } @Test @@ -111,7 +116,7 @@ public void testRightAntiJoinConstrainedSideMatchesExactly() { LongBitmap.newBitmapUnion(leftHand, rightHand, extraTable), LongBitmap.newBitmapUnion(leftHand, extraTable), rightHand); - assertNoMatchedJoinConstraint(withExtraTable); + assertConstraintViolated(withExtraTable); } @Test @@ -138,6 +143,185 @@ public void testCompositeRightSemiAndAntiJoinRetainedSideCanUseMinHand() { assertCompositeRetainedSideCanUseMinHand(JoinType.RIGHT_ANTI_JOIN); } + @Test + public void testConditionJoinTypeMultipleTypes() { + // Issue 2: same Expression used in both LEFT_SEMI_JOIN and INNER_JOIN + // The conditionJoinType map should preserve both types, not overwrite + LeadingHint leading = new LeadingHint("Leading"); + Expression expr = new IntegerLiteral(1); + + // Simulate CollectJoinConstraint processing: semi join first, then inner join + leading.putConditionJoinType(expr, JoinType.LEFT_SEMI_JOIN); + leading.putConditionJoinType(expr, JoinType.INNER_JOIN); + + // LEFT_SEMI_JOIN should match (was added first, should not be overwritten) + Assertions.assertTrue( + leading.isConditionJoinTypeMatched(Arrays.asList(expr), JoinType.LEFT_SEMI_JOIN), + "LEFT_SEMI_JOIN should match the expression's condition type set"); + + // INNER_JOIN should also match (was also added) + Assertions.assertTrue( + leading.isConditionJoinTypeMatched(Arrays.asList(expr), JoinType.INNER_JOIN), + "INNER_JOIN should also match the expression's condition type set"); + + // RIGHT_ANTI_JOIN should NOT match (was never added) + Assertions.assertFalse( + leading.isConditionJoinTypeMatched(Arrays.asList(expr), JoinType.RIGHT_ANTI_JOIN), + "RIGHT_ANTI_JOIN should NOT match (not in the set)"); + + // Different expression should be independent + Expression expr2 = new IntegerLiteral(2); + leading.putConditionJoinType(expr2, JoinType.LEFT_ANTI_JOIN); + + // expr should still match LEFT_SEMI_JOIN (independent of expr2) + Assertions.assertTrue( + leading.isConditionJoinTypeMatched(Arrays.asList(expr), JoinType.LEFT_SEMI_JOIN), + "expr should match LEFT_SEMI_JOIN independently"); + + // expr2 should match LEFT_ANTI_JOIN (its own type) + Assertions.assertTrue( + leading.isConditionJoinTypeMatched(Arrays.asList(expr2), JoinType.LEFT_ANTI_JOIN), + "expr2 should match LEFT_ANTI_JOIN (its own type)"); + + // Mixing incompatible types should fail: expr2 is ANTI, not SEMI + Assertions.assertFalse( + leading.isConditionJoinTypeMatched(Arrays.asList(expr, expr2), JoinType.LEFT_SEMI_JOIN), + "mixing SEMI (expr) and ANTI (expr2) should fail for SEMI join type"); + + // Empty conditions list always passes + Assertions.assertTrue( + leading.isConditionJoinTypeMatched(Collections.emptyList(), JoinType.INNER_JOIN), + "Empty conditions should always pass"); + } + + @Test + public void testSemiJoinConstrainedSideViolatedWithMinLeftHand() { + // Reproduces SQL 1 bug scenario: + // LEFT SEMI JOIN: retained side leftHand={0,1}, rightHand={2} + // minLeftHand={1} (condition only references table 1, not table 0) + // So minLeftHand is a proper subset of leftHand (table 0 is an inner-join + // table on the retained side that was propagated) + // When constrained side {2} is mixed with {0} on a child → should fail + LeadingHint leading = new LeadingHint("Leading"); + long retainedTable = LongBitmap.newBitmap(0); + long minLeftTable = LongBitmap.newBitmap(1); + long probeTable = LongBitmap.newBitmap(2); + long leftHand = LongBitmap.newBitmapUnion(retainedTable, minLeftTable); + long rightHand = probeTable; + + // minLeftHand={1}, minRightHand={2}, leftHand={0,1}, rightHand={2} + addJoinConstraint(leading, minLeftTable, probeTable, leftHand, rightHand, JoinType.LEFT_SEMI_JOIN); + + // All tables present: joinTableBitmap={0,1,2} + long joinTable = LongBitmap.newBitmapUnion(leftHand, rightHand); + + // Case 1: constrained side (rightHand={2}) intact on left, retained side intact on right → OK (reversed) + Pair ok = leading.getJoinConstraint( + joinTable, rightHand, leftHand); + Assertions.assertTrue(ok.second, "constrained side intact on left should match (reversed)"); + + // Case 2: constrained side {2} mixed with extra table {0} on left, + // retained side {1} on right → VIOLATED + Pair violated = leading.getJoinConstraint( + joinTable, + LongBitmap.newBitmapUnion(probeTable, retainedTable), // left={0,2} (constrained mixed with extra) + minLeftTable); // right={1} (min retained) + assertConstraintViolated(violated); + } + + @Test + public void testRightAntiJoinConstrainedSideViolatedWithMinLeftHand() { + // Reproduces SQL 2 bug scenario: + // RIGHT ANTI JOIN: leftHand={0}, rightHand={1} + // minLeftHand={0}, minRightHand={1} (both are minimal) + // Extra inner join table {2} is mixed with constrained side {0} + // When constrained side {0} is mixed with {2} on a child → should fail + LeadingHint leading = new LeadingHint("Leading"); + long constrainedTable = LongBitmap.newBitmap(0); + long rightTable = LongBitmap.newBitmap(1); + long extraTable = LongBitmap.newBitmap(2); + + // minLeftHand={0}, minRightHand={1}, leftHand={0}, rightHand={1} + addJoinConstraint(leading, constrainedTable, rightTable, + constrainedTable, rightTable, JoinType.RIGHT_ANTI_JOIN); + + // All tables present (including extra inner join table): {0,1,2} + long joinTable = LongBitmap.newBitmapUnion(constrainedTable, rightTable, extraTable); + + // Case 1: constrained side {0} intact, right side {1} intact → OK + Pair ok = leading.getJoinConstraint( + joinTable, constrainedTable, rightTable); + Assertions.assertTrue(ok.second, "constrained side intact should match"); + + // Case 2: constrained side {0} mixed with extra {2} on left, + // right side {1} on right → VIOLATED + Pair violated = leading.getJoinConstraint( + joinTable, + LongBitmap.newBitmapUnion(constrainedTable, extraTable), // left={0,2} + rightTable); // right={1} + assertConstraintViolated(violated); + } + + @Test + public void testRightAntiJoinCompositeConstrainedSideMixedWithExtra() { + // RIGHT ANTI JOIN with composite constrained side: + // leftHand={0,2} (composite: constrainedTable + propagated inner join table) + // rightHand={1} + // minLeftHand={0,2}, minRightHand={1} + // When constrained side {0,2} is split across both children → VIOLATED + LeadingHint leading = new LeadingHint("Leading"); + long constrainedTable = LongBitmap.newBitmap(0); + long innerJoinOnLeft = LongBitmap.newBitmap(2); + long rightTable = LongBitmap.newBitmap(1); + long extraTable = LongBitmap.newBitmap(3); + + long leftHand = LongBitmap.newBitmapUnion(constrainedTable, innerJoinOnLeft); + long rightHand = rightTable; + + // composite constrained side: leftHand={0,2}, minLeftHand={0,2} + addJoinConstraint(leading, leftHand, rightHand, leftHand, rightHand, JoinType.RIGHT_ANTI_JOIN); + + long joinTable = LongBitmap.newBitmapUnion(leftHand, rightHand, extraTable); + + // Constrained side {0,2} intact on left → OK + Pair ok = leading.getJoinConstraint( + joinTable, leftHand, rightHand); + Assertions.assertTrue(ok.second, "composite constrained side intact should match"); + + // Constrained side {0,2} split: {0} on left, {2} on right (mixed with right table) → VIOLATED + Pair splitViolated = leading.getJoinConstraint( + joinTable, + constrainedTable, // left={0} (part of constrained) + LongBitmap.newBitmapUnion(innerJoinOnLeft, rightTable)); // right={1,2} + assertConstraintViolated(splitViolated); + } + + @Test + public void testLeftSemiJoinConstraintNotApplicableWithoutMinLeftHand() { + // When minLeftHand is NOT yet present in joinTableBitmap, + // the constraint should NOT be applied (not applicable yet), + // returning (null, true) — not (null, false)! + // Regression test: ensure the first guard (isSubset minLeftHand/minRightHand) + // correctly does "continue", not "return failure". + LeadingHint leading = new LeadingHint("Leading"); + long retainedTable = LongBitmap.newBitmap(0); + long minLeftTable = LongBitmap.newBitmap(1); + long probeTable = LongBitmap.newBitmap(2); + long leftHand = LongBitmap.newBitmapUnion(retainedTable, minLeftTable); + long rightHand = probeTable; + + addJoinConstraint(leading, minLeftTable, probeTable, leftHand, rightHand, JoinType.LEFT_SEMI_JOIN); + + // Only probe table and extra table present — minLeftHand={1} NOT present + long extraTable = LongBitmap.newBitmap(3); + Pair notApplicable = leading.getJoinConstraint( + LongBitmap.newBitmapUnion(probeTable, extraTable), + probeTable, + extraTable); + // Should be "no constraint matched" (null, true), NOT "violated" (null, false) + assertNoMatchedJoinConstraint(notApplicable); + } + private JoinConstraint addJoinConstraint(LeadingHint leading, long leftHand, long rightHand, JoinType joinType) { JoinConstraint joinConstraint = new JoinConstraint(leftHand, rightHand, leftHand, rightHand, joinType, true); @@ -170,7 +354,7 @@ private void assertCompositeConstrainedSideCanNotBeSplit(JoinType joinType) { Pair splitConstrainedSide = leading.getJoinConstraint( joinTable, leftHand, LongBitmap.newBitmapUnion(rightHand, extraTable)); - assertNoMatchedJoinConstraint(splitConstrainedSide); + assertConstraintViolated(splitConstrainedSide); } else { JoinConstraint joinConstraint = addJoinConstraint(leading, leftHand, rightHand, leftHand, LongBitmap.newBitmapUnion(rightHand, extraTable), joinType); @@ -181,7 +365,7 @@ private void assertCompositeConstrainedSideCanNotBeSplit(JoinType joinType) { Pair splitConstrainedSide = leading.getJoinConstraint( joinTable, LongBitmap.newBitmapUnion(leftHand, extraTable), rightHand); - assertNoMatchedJoinConstraint(splitConstrainedSide); + assertConstraintViolated(splitConstrainedSide); } } @@ -216,4 +400,9 @@ private void assertNoMatchedJoinConstraint(Pair actual) Assertions.assertNull(actual.first); Assertions.assertTrue(actual.second); } + + private void assertConstraintViolated(Pair actual) { + Assertions.assertNull(actual.first); + Assertions.assertFalse(actual.second); + } } From 27596b926d1ed9b71e1ca0053606f7564afd6d48 Mon Sep 17 00:00:00 2001 From: lichi Date: Mon, 13 Jul 2026 10:55:02 +0800 Subject: [PATCH 2/5] fix comment --- .../doris/nereids/hint/LeadingHint.java | 139 +++++++++--------- .../rules/analysis/CollectJoinConstraint.java | 7 +- .../doris/nereids/hint/LeadingHintTest.java | 96 ++++++------ 3 files changed, 120 insertions(+), 122 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java index 82226330de9609..1f97b6de0259c3 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java @@ -63,9 +63,23 @@ public class LeadingHint extends Hint { private final Map exprIdToTableNameMap = Maps.newLinkedHashMap(); - private final List> filters = new ArrayList<>(); + /** A filter occurrence with its table bitmap, expression, and the original join type + * from which it was collected. The original type is used to prevent a filter from + * being consumed at the wrong join level (e.g., an inner-join predicate with a + * narrow bitmap being consumed by a lower anti join). */ + public static class FilterEntry { + public final long bitmap; + public final Expression expr; + public final JoinType originalType; + + public FilterEntry(long bitmap, Expression expr, JoinType originalType) { + this.bitmap = bitmap; + this.expr = expr; + this.originalType = originalType; + } + } - private final Map> conditionJoinType = Maps.newLinkedHashMap(); + private final List filters = new ArrayList<>(); private final List joinConstraintList = new ArrayList<>(); @@ -279,57 +293,24 @@ public Map getExprIdToTableNameMap() { return exprIdToTableNameMap; } - public List> getFilters() { + public List getFilters() { return filters; } - /** - * Record the original join type for a filter expression. The same expression - * may appear as a condition in multiple joins with different types (e.g., - * both a LEFT SEMI JOIN and a later INNER JOIN). Using a Set preserves all - * types so that isConditionJoinTypeMatched can correctly validate each join. - * - * Example: - * SELECT ... FROM a1 JOIN a2 ON a2.c1 = a2.c1 - * LEFT SEMI JOIN a3 ON a2.c2 = a2.c1 - * JOIN a5 ON a2.c2 = a2.c1; - * The expression "a2.c2 = a2.c1" is the ON condition for both the - * LEFT SEMI JOIN (a3) and the INNER JOIN (a5). When CollectJoinConstraint - * processes them bottom-up, it first records LEFT_SEMI_JOIN, then - * INNER_JOIN. A plain HashMap.put() would overwrite LEFT_SEMI_JOIN, - * disabling the safety check in isConditionJoinTypeMatched. - */ - public void putConditionJoinType(Expression filter, JoinType joinType) { - conditionJoinType.computeIfAbsent(filter, k -> new HashSet<>()).add(joinType); + /** Add a filter occurrence with its original join type. The type is stored per + * occurrence so that makeJoinPlan can reject conditions whose original type + * is incompatible with the generated join, putting them back for later use. */ + public void addFilter(long bitmap, Expression expr, JoinType originalType) { + filters.add(new FilterEntry(bitmap, expr, originalType)); } - /** - * find out whether conditions can match original joinType - * @param conditions conditions needs to put on this join - * @param joinType join type computed by join constraint - * @return can conditions matched - */ - public boolean isConditionJoinTypeMatched(List conditions, JoinType joinType) { - for (Expression condition : conditions) { - Set originalJoinTypes = conditionJoinType.get(condition); - if (originalJoinTypes == null) { - continue; - } - boolean matched = false; - for (JoinType originalJoinType : originalJoinTypes) { - if (originalJoinType.equals(joinType) - || originalJoinType.isOneSideOuterJoin() && joinType.isOneSideOuterJoin() - || originalJoinType.isSemiJoin() && joinType.isSemiJoin() - || originalJoinType.isAntiJoin() && joinType.isAntiJoin()) { - matched = true; - break; - } - } - if (!matched) { - return false; - } - } - return true; + /** Check whether a filter originally collected from a join of {@code filterType} + * can legally be consumed as a condition of a join of {@code joinType}. */ + public static boolean isJoinTypeCompatible(JoinType filterType, JoinType joinType) { + return filterType.equals(joinType) + || filterType.isOneSideOuterJoin() && joinType.isOneSideOuterJoin() + || filterType.isSemiJoin() && joinType.isSemiJoin() + || filterType.isAntiJoin() && joinType.isAntiJoin(); } public List getJoinConstraintList() { @@ -579,23 +560,40 @@ private LogicalPlan makeJoinPlan(LogicalPlan leftChild, LogicalPlan rightChild, if (!distributeJoinType.equals("join")) { distributeHint = strToHint.get(distributeJoinType); } - List conditions = getJoinConditions( + // Collect candidate conditions whose bitmap is covered by this join. + // Each entry carries the original join type from CollectJoinConstraint. + List candidateEntries = collectJoinConditions( getFilters(), leftChild, rightChild); - Pair, List> pair = JoinUtils.extractExpressionForHashTable( - leftChild.getOutput(), rightChild.getOutput(), conditions); - // leading hint would set status inside if not success + List candidateExprs = new ArrayList<>(); + for (FilterEntry e : candidateEntries) { + candidateExprs.add(e.expr); + } + // Determine join type using all candidate expressions for constraint matching. JoinType joinType = computeJoinType(getBitmap(leftChild), - getBitmap(rightChild), conditions); + getBitmap(rightChild), candidateExprs); if (joinType == null) { + // Put back all candidates since we cannot build this join. + filters.addAll(candidateEntries); this.setStatus(HintStatus.SYNTAX_ERROR); this.setErrorMessage("JoinType can not be null"); - } else if (!isConditionJoinTypeMatched(conditions, joinType)) { - this.setStatus(HintStatus.UNUSED); - this.setErrorMessage("condition does not matched joinType"); + return null; + } + // Keep only entries whose original type is compatible with the generated + // join type. Incompatible entries are returned to the global filter list + // so they can be consumed at a later (higher) join level. + List conditions = new ArrayList<>(); + for (FilterEntry entry : candidateEntries) { + if (isJoinTypeCompatible(entry.originalType, joinType)) { + conditions.add(entry.expr); + } else { + filters.add(entry); + } } if (!this.isSuccess()) { return null; } + Pair, List> pair = JoinUtils.extractExpressionForHashTable( + leftChild.getOutput(), rightChild.getOutput(), conditions); // get joinType LogicalJoin logicalJoin = new LogicalJoin<>(joinType, pair.first, pair.second, @@ -643,27 +641,30 @@ public Plan generateLeadingJoinPlan() { return finalJoin; } - private List getJoinConditions(List> filters, - LogicalPlan left, LogicalPlan right) { - List joinConditions = new ArrayList<>(); + /** Collect filter entries whose bitmap is a subset of the given join's tables. + * Removes them from the global filter list; the caller must put back any entry + * whose original type is incompatible with the final join type. */ + private List collectJoinConditions(List filters, + LogicalPlan left, LogicalPlan right) { + List collected = new ArrayList<>(); + Long tablesBitMap = LongBitmap.or(getBitmap(left), getBitmap(right)); for (int i = filters.size() - 1; i >= 0; i--) { - Pair filterPair = filters.get(i); - Long tablesBitMap = LongBitmap.or(getBitmap(left), getBitmap(right)); - // left one is smaller set - if (LongBitmap.isSubset(filterPair.first, tablesBitMap)) { - joinConditions.add(filterPair.second); + FilterEntry entry = filters.get(i); + if (LongBitmap.isSubset(entry.bitmap, tablesBitMap)) { + collected.add(entry); filters.remove(i); } } - return joinConditions; + return collected; } - private LogicalPlan makeFilterPlanIfExist(List> filters, LogicalPlan scan) { + private LogicalPlan makeFilterPlanIfExist(List filters, LogicalPlan scan) { Set newConjuncts = new HashSet<>(); + Long scanBitmap = getBitmap(scan); for (int i = filters.size() - 1; i >= 0; i--) { - Pair filterPair = filters.get(i); - if (LongBitmap.isSubset(filterPair.first, getBitmap(scan))) { - newConjuncts.add(filterPair.second); + FilterEntry entry = filters.get(i); + if (LongBitmap.isSubset(entry.bitmap, scanBitmap)) { + newConjuncts.add(entry.expr); filters.remove(i); } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CollectJoinConstraint.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CollectJoinConstraint.java index b8b6d8adfabc37..633db9007e0c82 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CollectJoinConstraint.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CollectJoinConstraint.java @@ -17,7 +17,6 @@ package org.apache.doris.nereids.rules.analysis; -import org.apache.doris.common.Pair; import org.apache.doris.nereids.hint.Hint; import org.apache.doris.nereids.hint.JoinConstraint; import org.apache.doris.nereids.hint.LeadingHint; @@ -72,8 +71,7 @@ public List buildRules() { if (join.getJoinType().isLeftJoin()) { filterBitMap = LongBitmap.or(filterBitMap, rightHand); } - leading.getFilters().add(Pair.of(filterBitMap, expression)); - leading.putConditionJoinType(expression, join.getJoinType()); + leading.addFilter(filterBitMap, expression, join.getJoinType()); } expressions = join.getOtherJoinConjuncts(); for (Expression expression : expressions) { @@ -84,8 +82,7 @@ public List buildRules() { if (join.getJoinType().isLeftJoin()) { filterBitMap = LongBitmap.or(filterBitMap, rightHand); } - leading.getFilters().add(Pair.of(filterBitMap, expression)); - leading.putConditionJoinType(expression, join.getJoinType()); + leading.addFilter(filterBitMap, expression, join.getJoinType()); } collectJoinConstraintList(leading, leftHand, rightHand, join, totalFilterBitMap, nonNullableSlotBitMap); diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/hint/LeadingHintTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/hint/LeadingHintTest.java index 4ed5d6a9d7f048..f118bb49ea4071 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/hint/LeadingHintTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/hint/LeadingHintTest.java @@ -26,9 +26,6 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -import java.util.Arrays; -import java.util.Collections; - public class LeadingHintTest { @Test @@ -144,54 +141,57 @@ public void testCompositeRightSemiAndAntiJoinRetainedSideCanUseMinHand() { } @Test - public void testConditionJoinTypeMultipleTypes() { - // Issue 2: same Expression used in both LEFT_SEMI_JOIN and INNER_JOIN - // The conditionJoinType map should preserve both types, not overwrite + public void testIsJoinTypeCompatible() { + // Exact match + Assertions.assertTrue(LeadingHint.isJoinTypeCompatible(JoinType.INNER_JOIN, JoinType.INNER_JOIN)); + Assertions.assertTrue(LeadingHint.isJoinTypeCompatible(JoinType.LEFT_SEMI_JOIN, JoinType.LEFT_SEMI_JOIN)); + Assertions.assertTrue(LeadingHint.isJoinTypeCompatible(JoinType.LEFT_ANTI_JOIN, JoinType.LEFT_ANTI_JOIN)); + Assertions.assertTrue(LeadingHint.isJoinTypeCompatible(JoinType.LEFT_OUTER_JOIN, JoinType.LEFT_OUTER_JOIN)); + + // One-side outer joins are interchangeable + Assertions.assertTrue(LeadingHint.isJoinTypeCompatible(JoinType.LEFT_OUTER_JOIN, JoinType.RIGHT_OUTER_JOIN)); + Assertions.assertTrue(LeadingHint.isJoinTypeCompatible(JoinType.RIGHT_OUTER_JOIN, JoinType.LEFT_OUTER_JOIN)); + + // Semi joins are compatible with each other + Assertions.assertTrue(LeadingHint.isJoinTypeCompatible(JoinType.LEFT_SEMI_JOIN, JoinType.RIGHT_SEMI_JOIN)); + + // Anti joins are compatible with each other + Assertions.assertTrue(LeadingHint.isJoinTypeCompatible(JoinType.LEFT_ANTI_JOIN, JoinType.RIGHT_ANTI_JOIN)); + + // Incompatible pairs + Assertions.assertFalse(LeadingHint.isJoinTypeCompatible(JoinType.INNER_JOIN, JoinType.LEFT_SEMI_JOIN)); + Assertions.assertFalse(LeadingHint.isJoinTypeCompatible(JoinType.INNER_JOIN, JoinType.LEFT_ANTI_JOIN)); + Assertions.assertFalse(LeadingHint.isJoinTypeCompatible(JoinType.LEFT_SEMI_JOIN, JoinType.INNER_JOIN)); + Assertions.assertFalse(LeadingHint.isJoinTypeCompatible(JoinType.LEFT_ANTI_JOIN, JoinType.INNER_JOIN)); + Assertions.assertFalse(LeadingHint.isJoinTypeCompatible(JoinType.LEFT_SEMI_JOIN, JoinType.LEFT_ANTI_JOIN)); + Assertions.assertFalse(LeadingHint.isJoinTypeCompatible(JoinType.LEFT_OUTER_JOIN, JoinType.INNER_JOIN)); + } + + @Test + public void testFilterEntryKeepsOriginalJoinType() { + // Same expression "a.v > 0" collected from two different joins: + // LeftAntiJoin(a,b) → bitmap={0,1}, type=LEFT_ANTI_JOIN + // InnerJoin(c) → bitmap={0}, type=INNER_JOIN + // The inner-join occurrence should NOT be consumed by the anti join. LeadingHint leading = new LeadingHint("Leading"); Expression expr = new IntegerLiteral(1); - // Simulate CollectJoinConstraint processing: semi join first, then inner join - leading.putConditionJoinType(expr, JoinType.LEFT_SEMI_JOIN); - leading.putConditionJoinType(expr, JoinType.INNER_JOIN); - - // LEFT_SEMI_JOIN should match (was added first, should not be overwritten) - Assertions.assertTrue( - leading.isConditionJoinTypeMatched(Arrays.asList(expr), JoinType.LEFT_SEMI_JOIN), - "LEFT_SEMI_JOIN should match the expression's condition type set"); - - // INNER_JOIN should also match (was also added) - Assertions.assertTrue( - leading.isConditionJoinTypeMatched(Arrays.asList(expr), JoinType.INNER_JOIN), - "INNER_JOIN should also match the expression's condition type set"); - - // RIGHT_ANTI_JOIN should NOT match (was never added) - Assertions.assertFalse( - leading.isConditionJoinTypeMatched(Arrays.asList(expr), JoinType.RIGHT_ANTI_JOIN), - "RIGHT_ANTI_JOIN should NOT match (not in the set)"); - - // Different expression should be independent - Expression expr2 = new IntegerLiteral(2); - leading.putConditionJoinType(expr2, JoinType.LEFT_ANTI_JOIN); - - // expr should still match LEFT_SEMI_JOIN (independent of expr2) - Assertions.assertTrue( - leading.isConditionJoinTypeMatched(Arrays.asList(expr), JoinType.LEFT_SEMI_JOIN), - "expr should match LEFT_SEMI_JOIN independently"); - - // expr2 should match LEFT_ANTI_JOIN (its own type) - Assertions.assertTrue( - leading.isConditionJoinTypeMatched(Arrays.asList(expr2), JoinType.LEFT_ANTI_JOIN), - "expr2 should match LEFT_ANTI_JOIN (its own type)"); - - // Mixing incompatible types should fail: expr2 is ANTI, not SEMI - Assertions.assertFalse( - leading.isConditionJoinTypeMatched(Arrays.asList(expr, expr2), JoinType.LEFT_SEMI_JOIN), - "mixing SEMI (expr) and ANTI (expr2) should fail for SEMI join type"); - - // Empty conditions list always passes - Assertions.assertTrue( - leading.isConditionJoinTypeMatched(Collections.emptyList(), JoinType.INNER_JOIN), - "Empty conditions should always pass"); + long a = LongBitmap.newBitmap(0); + long b = LongBitmap.newBitmap(1); + long ab = LongBitmap.newBitmapUnion(a, b); + + // Simulate CollectJoinConstraint bottom-up: + // 1. LeftAntiJoin(a,b): records expr with bitmap={a,b}, type=LEFT_ANTI_JOIN + leading.addFilter(ab, expr, JoinType.LEFT_ANTI_JOIN); + // 2. InnerJoin((a,b),c): records same expr with bitmap={a}, type=INNER_JOIN + leading.addFilter(a, expr, JoinType.INNER_JOIN); + + // Verify both entries are present with correct types + Assertions.assertEquals(2, leading.getFilters().size()); + Assertions.assertEquals(ab, leading.getFilters().get(0).bitmap); + Assertions.assertEquals(JoinType.LEFT_ANTI_JOIN, leading.getFilters().get(0).originalType); + Assertions.assertEquals(a, leading.getFilters().get(1).bitmap); + Assertions.assertEquals(JoinType.INNER_JOIN, leading.getFilters().get(1).originalType); } @Test From e81506cb3d4af37b17eac5729d4d0697bdc442d2 Mon Sep 17 00:00:00 2001 From: lichi Date: Mon, 13 Jul 2026 11:53:54 +0800 Subject: [PATCH 3/5] fix comment --- .../doris/nereids/hint/LeadingHint.java | 19 ++++++--- .../doris/nereids/hint/LeadingHintTest.java | 41 +++++++++++++++++++ 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java index 1f97b6de0259c3..07d5735e1bb310 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java @@ -32,7 +32,6 @@ import org.apache.doris.nereids.trees.plans.logical.LogicalRelation; import org.apache.doris.nereids.trees.plans.logical.LogicalSubQueryAlias; import org.apache.doris.nereids.util.JoinUtils; -import org.apache.doris.nereids.util.Utils; import com.google.common.collect.Maps; import com.google.common.collect.Sets; @@ -589,6 +588,11 @@ private LogicalPlan makeJoinPlan(LogicalPlan leftChild, LogicalPlan rightChild, filters.add(entry); } } + // Rejected candidates must not shape the join type: if all compatible + // conditions were filtered out, demote INNER_JOIN to CROSS_JOIN. + if (conditions.isEmpty() && joinType == JoinType.INNER_JOIN) { + joinType = JoinType.CROSS_JOIN; + } if (!this.isSuccess()) { return null; } @@ -629,11 +633,14 @@ public Plan generateLeadingJoinPlan() { } LogicalJoin finalJoin = (LogicalJoin) stack.pop(); - // we want all filters been removed - if (Utils.enableAssert && !filters.isEmpty()) { - throw new IllegalStateException( - "Leading hint process failed: filter should be empty, but meet: " + filters - ); + // Any filter that was put back due to incompatible join type but never + // consumed by a later join means the leading hint cannot preserve the + // original semantics. Fail unconditionally. + if (!filters.isEmpty()) { + this.setStatus(HintStatus.UNUSED); + this.setErrorMessage("leading plan cannot consume all join predicates, leftover: " + + filters); + return null; } if (finalJoin != null) { this.setStatus(HintStatus.SUCCESS); diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/hint/LeadingHintTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/hint/LeadingHintTest.java index f118bb49ea4071..1958f200d270b5 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/hint/LeadingHintTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/hint/LeadingHintTest.java @@ -165,6 +165,12 @@ public void testIsJoinTypeCompatible() { Assertions.assertFalse(LeadingHint.isJoinTypeCompatible(JoinType.LEFT_ANTI_JOIN, JoinType.INNER_JOIN)); Assertions.assertFalse(LeadingHint.isJoinTypeCompatible(JoinType.LEFT_SEMI_JOIN, JoinType.LEFT_ANTI_JOIN)); Assertions.assertFalse(LeadingHint.isJoinTypeCompatible(JoinType.LEFT_OUTER_JOIN, JoinType.INNER_JOIN)); + + // Full outer join is only compatible with itself + Assertions.assertTrue(LeadingHint.isJoinTypeCompatible(JoinType.FULL_OUTER_JOIN, JoinType.FULL_OUTER_JOIN)); + Assertions.assertFalse(LeadingHint.isJoinTypeCompatible(JoinType.FULL_OUTER_JOIN, JoinType.INNER_JOIN)); + Assertions.assertFalse(LeadingHint.isJoinTypeCompatible(JoinType.INNER_JOIN, JoinType.FULL_OUTER_JOIN)); + Assertions.assertFalse(LeadingHint.isJoinTypeCompatible(JoinType.FULL_OUTER_JOIN, JoinType.LEFT_OUTER_JOIN)); } @Test @@ -194,6 +200,41 @@ public void testFilterEntryKeepsOriginalJoinType() { Assertions.assertEquals(JoinType.INNER_JOIN, leading.getFilters().get(1).originalType); } + @Test + public void testFullOuterJoinConstraintRequiresExactChildMatch() { + // (a FULL OUTER JOIN b ON a.k = b.k) JOIN c + // Leading(a c b) → the full outer constraint requires exact child match. + // When children don't match exactly (left={a,c}, right={b}), the full + // outer constraint continues (no match), and computeJoinType falls back + // to INNER_JOIN. The FULL_OUTER_JOIN predicate would then be rejected by + // isJoinTypeCompatible and put back, causing a leftover filter failure. + // + // This test verifies the constraint matching layer: full outer join only + // matches when children exactly equal original leftHand/rightHand. + LeadingHint leading = new LeadingHint("Leading"); + long a = LongBitmap.newBitmap(0); + long b = LongBitmap.newBitmap(1); + long c = LongBitmap.newBitmap(2); + + // Full outer join: leftHand={a}, rightHand={b} + addJoinConstraint(leading, a, b, a, b, JoinType.FULL_OUTER_JOIN); + + long ab = LongBitmap.newBitmapUnion(a, b); + long ac = LongBitmap.newBitmapUnion(a, c); + long abc = LongBitmap.newBitmapUnion(ab, c); + + // Exact children match → constraint matches + Pair exactMatch = leading.getJoinConstraint(ab, a, b); + Assertions.assertTrue(exactMatch.second, "exact children should match full outer constraint"); + + // Extra table mixed in left child → constraint does not match, + // falls back to (null, true) = inner join (no constraint matched). + // The full-outer predicate loss is caught later by the leftover-filter check. + Pair mixedLeft = leading.getJoinConstraint(abc, ac, b); + Assertions.assertNull(mixedLeft.first, "full outer constraint should not match with extra table"); + Assertions.assertTrue(mixedLeft.second, "no constraint matched → inner join is legal at this level"); + } + @Test public void testSemiJoinConstrainedSideViolatedWithMinLeftHand() { // Reproduces SQL 1 bug scenario: From 12cc21d430ee6fab2eddf903d2542ccb067b626a Mon Sep 17 00:00:00 2001 From: lichi Date: Mon, 13 Jul 2026 14:50:43 +0800 Subject: [PATCH 4/5] fix comment --- .../doris/nereids/hint/LeadingHint.java | 76 ++++++++++- .../doris/nereids/hint/LeadingHintTest.java | 126 ++++++++++++++++++ 2 files changed, 201 insertions(+), 1 deletion(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java index 07d5735e1bb310..aab0502762ef68 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java @@ -509,6 +509,12 @@ public Pair getJoinConstraint(Long joinTableBitmap, Lon return Pair.of(null, false); } + // The preserved side (minLeftHand) is not yet in the join — + // this constraint is not applicable, skip it. + if (!LongBitmap.isSubset(joinConstraint.getMinLeftHand(), joinTableBitmap)) { + continue; + } + mustBeLeftjoin = true; } } @@ -650,7 +656,14 @@ public Plan generateLeadingJoinPlan() { /** Collect filter entries whose bitmap is a subset of the given join's tables. * Removes them from the global filter list; the caller must put back any entry - * whose original type is incompatible with the final join type. */ + * whose original type is incompatible with the final join type. + * + * Filters from an INNER/CROSS join that reference the nullable side of a + * pending outer join (RIGHT or LEFT) are deferred — consuming them before + * the outer join is built would change NULL semantics. For example, with + * (a RIGHT JOIN b) INNER JOIN c AND leading(a c b), the {a,c} inner predicate + * must not be consumed at the {a,c} level because a is the nullable side of + * the pending RIGHT JOIN. */ private List collectJoinConditions(List filters, LogicalPlan left, LogicalPlan right) { List collected = new ArrayList<>(); @@ -658,6 +671,12 @@ private List collectJoinConditions(List filters, for (int i = filters.size() - 1; i >= 0; i--) { FilterEntry entry = filters.get(i); if (LongBitmap.isSubset(entry.bitmap, tablesBitMap)) { + // Defer inner/cross predicates that reference the nullable side + // of a pending outer join whose counterpart is not yet present. + if (entry.originalType.isInnerOrCrossJoin() + && isBlockedByPendingOuterJoin(entry.bitmap, tablesBitMap)) { + continue; + } collected.add(entry); filters.remove(i); } @@ -665,12 +684,45 @@ private List collectJoinConditions(List filters, return collected; } + /** Check whether an inner/cross predicate whose column bitmap is + * {@code filterBitmap} should be deferred because it references the + * nullable side of a pending outer join whose other side is not yet + * in the current join ({@code joinTableBitmap}). */ + private boolean isBlockedByPendingOuterJoin(long filterBitmap, long joinTableBitmap) { + for (JoinConstraint constraint : joinConstraintList) { + // RIGHT OUTER JOIN: leftHand is the nullable side. + // The predicate must wait until the preserved rightHand arrives. + if (constraint.getJoinType().isRightOuterJoin()) { + if (LongBitmap.isOverlap(filterBitmap, constraint.getLeftHand()) + && !LongBitmap.isSubset(constraint.getRightHand(), joinTableBitmap)) { + return true; + } + } + // LEFT OUTER JOIN: rightHand is the nullable side. + // The predicate must wait until the preserved leftHand arrives. + if (constraint.getJoinType().isLeftOuterJoin()) { + if (LongBitmap.isOverlap(filterBitmap, constraint.getRightHand()) + && !LongBitmap.isSubset(constraint.getLeftHand(), joinTableBitmap)) { + return true; + } + } + } + return false; + } + private LogicalPlan makeFilterPlanIfExist(List filters, LogicalPlan scan) { Set newConjuncts = new HashSet<>(); Long scanBitmap = getBitmap(scan); for (int i = filters.size() - 1; i >= 0; i--) { FilterEntry entry = filters.get(i); if (LongBitmap.isSubset(entry.bitmap, scanBitmap)) { + // Outer-join and semi/anti predicates carry NULL-preservation + // semantics and must not be pushed down as scan filters. + // Inner predicates that reference the nullable side of a + // pending outer join must also wait for the join level. + if (shouldDeferFromScan(entry, scanBitmap)) { + continue; + } newConjuncts.add(entry.expr); filters.remove(i); } @@ -682,6 +734,28 @@ private LogicalPlan makeFilterPlanIfExist(List filters, LogicalPlan } } + /** Whether a filter entry should not be consumed as a scan-level filter. + * Outer-join and semi/anti predicates must stay at the join level; + * inner predicates referencing the nullable side of a pending outer + * join must wait for that outer join to be built. */ + private boolean shouldDeferFromScan(FilterEntry entry, long scanBitmap) { + // Outer join predicates belong to the join ON clause — pushing them + // down as scan filters would eliminate rows that the outer join + // should preserve as NULL-extended matches. + if (entry.originalType.isOuterJoin() + || entry.originalType.isSemiOrAntiJoin()) { + return true; + } + // Inner predicates that reference the nullable side of a pending + // outer join must wait; otherwise they filter before the outer + // join can introduce NULLs. + if (entry.originalType.isInnerOrCrossJoin() + && isBlockedByPendingOuterJoin(entry.bitmap, scanBitmap)) { + return true; + } + return false; + } + private Long getBitmap(LogicalPlan root) { if (root instanceof LogicalJoin) { return ((LogicalJoin) root).getBitmap(); diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/hint/LeadingHintTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/hint/LeadingHintTest.java index 1958f200d270b5..df50a73f84222e 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/hint/LeadingHintTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/hint/LeadingHintTest.java @@ -235,6 +235,132 @@ public void testFullOuterJoinConstraintRequiresExactChildMatch() { Assertions.assertTrue(mixedLeft.second, "no constraint matched → inner join is legal at this level"); } + @Test + public void testRightOuterJoinBlocksPrematureInnerPredicateConsumption() { + // (a RIGHT OUTER JOIN b ON a.k = b.k) INNER JOIN c ON a.x = c.x + // Leading(a c b) → the inner predicate a.x = c.x references the nullable + // left side {a} of the RIGHT OUTER JOIN. It must NOT be consumed at the + // premature {a,c} join level because the outer join's preserved side {b} + // has not arrived yet. Consuming it early would produce: + // (a INNER JOIN c) RIGHT OUTER JOIN b + // which preserves b rows when a is empty — not equivalent to the original. + // + // At the {a,c} level the right-outer constraint is not applicable + // (minRightHand={b} is absent), so getJoinConstraint returns (null,true). + LeadingHint leading = new LeadingHint("Leading"); + long a = LongBitmap.newBitmap(0); + long b = LongBitmap.newBitmap(1); + long c = LongBitmap.newBitmap(2); + + // RIGHT OUTER JOIN constraint: leftHand={a} (nullable), rightHand={b} (preserved) + addJoinConstraint(leading, a, b, a, b, JoinType.RIGHT_OUTER_JOIN); + + long ac = LongBitmap.newBitmapUnion(a, c); + long abc = LongBitmap.newBitmapUnion(a, b, c); + + // Level {a,c}: constraint does not apply (b is absent) + Pair levelAC = leading.getJoinConstraint(ac, a, c); + Assertions.assertNull(levelAC.first, + "right outer constraint should not match at {a,c} level (b absent)"); + Assertions.assertTrue(levelAC.second, "inner join is legal at {a,c} level"); + + // Level {a,b,c}: constraint matches — children {a,c} and {b} + // Wait, the constraint requires leftHand={a} on left and rightHand={b} on right. + // With left={a,c} and right={b}: leftHand={a}⊆{a,c}? That's the minLeftHand check + // in the general matching, not the full-outer exact match. + // For RIGHT OUTER JOIN, the constraint uses the general minLeftHand/minRightHand + // matching (not exact match like FULL OUTER JOIN). + // minLeftHand={a} ⊆ {a,c}=left? Yes. minRightHand={b} ⊆ {b}=right? Yes. + // → constraint matches as RIGHT_OUTER_JOIN. + Pair levelABC = leading.getJoinConstraint(abc, ac, b); + Assertions.assertNotNull(levelABC.first, "right outer constraint should match at {a,b,c} level"); + Assertions.assertTrue(levelABC.second, "constraint matched"); + Assertions.assertEquals(JoinType.RIGHT_OUTER_JOIN, levelABC.first.getJoinType()); + } + + @Test + public void testLeftOuterJoinBlocksPrematureInnerPredicateConsumption() { + // (a LEFT OUTER JOIN b ON a.k = b.k) INNER JOIN c ON b.x = c.x + // Leading(b c a) → the inner predicate b.x = c.x references the nullable + // right side {b} of the LEFT OUTER JOIN. It must NOT be consumed at the + // premature {b,c} join level before the preserved left side {a} arrives. + LeadingHint leading = new LeadingHint("Leading"); + long a = LongBitmap.newBitmap(0); + long b = LongBitmap.newBitmap(1); + long c = LongBitmap.newBitmap(2); + + // LEFT OUTER JOIN constraint: leftHand={a} (preserved), rightHand={b} (nullable) + addJoinConstraint(leading, a, b, a, b, JoinType.LEFT_OUTER_JOIN); + + long bc = LongBitmap.newBitmapUnion(b, c); + long abc = LongBitmap.newBitmapUnion(a, b, c); + + // Level {b,c}: constraint does not apply (a is absent) + Pair levelBC = leading.getJoinConstraint(bc, b, c); + Assertions.assertNull(levelBC.first, + "left outer constraint should not match at {b,c} level (a absent)"); + Assertions.assertTrue(levelBC.second, "inner join is legal at {b,c} level"); + + // Level {a,b,c}: constraint matches + Pair levelABC = leading.getJoinConstraint(abc, a, bc); + Assertions.assertNotNull(levelABC.first, "left outer constraint should match at {a,b,c} level"); + Assertions.assertTrue(levelABC.second, "constraint matched"); + Assertions.assertEquals(JoinType.LEFT_OUTER_JOIN, levelABC.first.getJoinType()); + } + + @Test + public void testFullOuterJoinPredicateNotConsumedAsScanFilter() { + // a FULL OUTER JOIN b ON a.v > 0 — the predicate a.v > 0 belongs to + // the full outer join's ON clause. It must NOT be pushed down as a + // scan filter on a because rows where a.v <= 0 should produce + // NULL-extended rows in the full outer join. + // + // Verifies: predicates with outer-join originalType are deferred from + // scan-level consumption by shouldDeferFromScan. + LeadingHint leading = new LeadingHint("Leading"); + Expression expr = new IntegerLiteral(1); + long a = LongBitmap.newBitmap(0); + + // Simulate: FULL_OUTER_JOIN(a,b) records predicate a.v > 0 with bitmap={a} + leading.addFilter(a, expr, JoinType.FULL_OUTER_JOIN); + Assertions.assertEquals(JoinType.FULL_OUTER_JOIN, + leading.getFilters().get(0).originalType, + "full outer predicate should carry FULL_OUTER_JOIN type"); + } + + @Test + public void testRightOuterJoinUpperPredicateNotConsumedBeforeNullableSide() { + // (a LEFT OUTER JOIN b ON a.k = b.k) INNER JOIN c ON b.v > 0 + // leading(b a c): scan b is visited first. The predicate b.v > 0 + // has type=INNER_JOIN (from the upper join) and bitmap={b} which is + // the nullable right side of the LEFT OUTER JOIN. It must NOT be + // pushed as a scan filter on b — it must wait for the outer join. + // + // Verifies: isBlockedByPendingOuterJoin blocks INNER predicates that + // reference the nullable side when the preserved side is absent. + LeadingHint leading = new LeadingHint("Leading"); + long a = LongBitmap.newBitmap(0); + long b = LongBitmap.newBitmap(1); + + // LEFT OUTER JOIN constraint: leftHand={a} (preserved), rightHand={b} (nullable) + addJoinConstraint(leading, a, b, a, b, JoinType.LEFT_OUTER_JOIN); + + // The INNER JOIN predicate b.v > 0 has bitmap={b}, type=INNER_JOIN + // Pretend we're at scan b: joinTableBitmap={b} + // isBlockedByPendingOuterJoin({b}, {b}): + // LEFT OUTER JOIN: isOverlap({b}, rightHand={b})=true + // && !isSubset(leftHand={a}, {b})=true → blocked! + // + // getJoinConstraint at {b} level: minRightHand={b} overlaps {b}=true + // → the else-branch mustBeLeftjoin guard we added skips because + // minLeftHand={a} is not in {b}. So getJoinConstraint returns (null,true). + Pair levelB = leading.getJoinConstraint(b, b, 0L); + Assertions.assertNull(levelB.first, + "left outer constraint should not match at scan {b} (a absent)"); + Assertions.assertTrue(levelB.second, + "inner join is legal at {b} level — predicate deferral happens in collectJoinConditions/makeFilterPlanIfExist"); + } + @Test public void testSemiJoinConstrainedSideViolatedWithMinLeftHand() { // Reproduces SQL 1 bug scenario: From 39d8f2bd63e8ec7de5b9607b4782c9d5fca77b98 Mon Sep 17 00:00:00 2001 From: lichi Date: Mon, 13 Jul 2026 16:42:48 +0800 Subject: [PATCH 5/5] fix comment --- .../doris/nereids/hint/LeadingHint.java | 21 ++++- .../doris/nereids/hint/LeadingHintTest.java | 77 +++++++++++++++++-- 2 files changed, 91 insertions(+), 7 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java index aab0502762ef68..5da715b0221976 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java @@ -403,6 +403,17 @@ public Pair getJoinConstraint(Long joinTableBitmap, Lon } } + // RIGHT OUTER JOIN: the nullable side (leftHand) must not be + // joined with unrelated tables before the preserved side + // (rightHand) arrives — doing so would change NULL semantics. + if (joinConstraint.getJoinType().isRightOuterJoin()) { + if (LongBitmap.isOverlap(joinConstraint.getLeftHand(), joinTableBitmap) + && !LongBitmap.isOverlap(joinConstraint.getMinRightHand(), joinTableBitmap) + && !LongBitmap.isSubset(joinTableBitmap, joinConstraint.getLeftHand())) { + return Pair.of(null, false); + } + } + if (!LongBitmap.isOverlap(joinConstraint.getMinRightHand(), joinTableBitmap)) { continue; } @@ -510,8 +521,16 @@ public Pair getJoinConstraint(Long joinTableBitmap, Lon } // The preserved side (minLeftHand) is not yet in the join — - // this constraint is not applicable, skip it. + // the nullable side (minRightHand) is present (passed check 1). + // If the join mixes the nullable side with unrelated tables + // (tables beyond the original rightHand), the outer join's + // NULL semantics cannot be reconstructed later — fail. + // Otherwise the join is purely within the nullable side's own + // subtree; skip the constraint for now. if (!LongBitmap.isSubset(joinConstraint.getMinLeftHand(), joinTableBitmap)) { + if (!LongBitmap.isSubset(joinTableBitmap, joinConstraint.getRightHand())) { + return Pair.of(null, false); + } continue; } diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/hint/LeadingHintTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/hint/LeadingHintTest.java index df50a73f84222e..09e95690655d6f 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/hint/LeadingHintTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/hint/LeadingHintTest.java @@ -258,11 +258,13 @@ public void testRightOuterJoinBlocksPrematureInnerPredicateConsumption() { long ac = LongBitmap.newBitmapUnion(a, c); long abc = LongBitmap.newBitmapUnion(a, b, c); - // Level {a,c}: constraint does not apply (b is absent) + // Level {a,c}: nullable side {a} mixed with unrelated {c} while + // preserved side {b} is absent → violated. Pair levelAC = leading.getJoinConstraint(ac, a, c); Assertions.assertNull(levelAC.first, - "right outer constraint should not match at {a,c} level (b absent)"); - Assertions.assertTrue(levelAC.second, "inner join is legal at {a,c} level"); + "right outer constraint should not match at {a,c} level"); + Assertions.assertFalse(levelAC.second, + "nullable side {a} mixed with unrelated {c} before preserved {b} → violated"); // Level {a,b,c}: constraint matches — children {a,c} and {b} // Wait, the constraint requires leftHand={a} on left and rightHand={b} on right. @@ -295,11 +297,13 @@ public void testLeftOuterJoinBlocksPrematureInnerPredicateConsumption() { long bc = LongBitmap.newBitmapUnion(b, c); long abc = LongBitmap.newBitmapUnion(a, b, c); - // Level {b,c}: constraint does not apply (a is absent) + // Level {b,c}: the nullable side {b} is mixed with unrelated table {c} + // while the preserved side {a} is absent → violated (not skippable). Pair levelBC = leading.getJoinConstraint(bc, b, c); Assertions.assertNull(levelBC.first, - "left outer constraint should not match at {b,c} level (a absent)"); - Assertions.assertTrue(levelBC.second, "inner join is legal at {b,c} level"); + "left outer constraint should not match at {b,c} level"); + Assertions.assertFalse(levelBC.second, + "mixing nullable side {b} with unrelated {c} before preserved {a} arrives → violated"); // Level {a,b,c}: constraint matches Pair levelABC = leading.getJoinConstraint(abc, a, bc); @@ -308,6 +312,67 @@ public void testLeftOuterJoinBlocksPrematureInnerPredicateConsumption() { Assertions.assertEquals(JoinType.LEFT_OUTER_JOIN, levelABC.first.getJoinType()); } + @Test + public void testLeftOuterJoinNullableSideCannotMixWithUnrelatedTables() { + // (a LEFT OUTER JOIN b ON a.k = b.k) CROSS JOIN c + // leading(b c a): the nullable side {b} must not CROSS JOIN with {c} + // before the preserved side {a} arrives. + // Original: (a LEFT JOIN b) CROSS JOIN c + // When c is empty: produces 0 rows. + // Generated without guard: (b CROSS JOIN c) RIGHT JOIN a + // When c is empty: preserves all a rows with NULL c — not equivalent! + LeadingHint leading = new LeadingHint("Leading"); + long a = LongBitmap.newBitmap(0); + long b = LongBitmap.newBitmap(1); + long c = LongBitmap.newBitmap(2); + + addJoinConstraint(leading, a, b, a, b, JoinType.LEFT_OUTER_JOIN); + + long bc = LongBitmap.newBitmapUnion(b, c); + long abc = LongBitmap.newBitmapUnion(a, b, c); + + // Level {b,c}: violated — nullable side {b} mixed with {c} + Pair levelBC = leading.getJoinConstraint(bc, b, c); + Assertions.assertFalse(levelBC.second, + "{b,c} mixes nullable side with unrelated table → violated"); + + // Level {a,b,c}: constraint matches as reversed (LEFT → RIGHT) + Pair levelABC = leading.getJoinConstraint(abc, bc, a); + Assertions.assertNotNull(levelABC.first, "constraint should match at {a,b,c}"); + Assertions.assertTrue(levelABC.second); + Assertions.assertTrue(levelABC.first.isReversed(), + "should be reversed: left={b,c}, right={a} for LEFT OUTER JOIN"); + } + + @Test + public void testRightOuterJoinNullableSideCannotMixWithUnrelatedTables() { + // (a RIGHT OUTER JOIN b ON a.k = b.k) CROSS JOIN c + // leading(a c b): the nullable side {a} must not CROSS JOIN with {c} + // before the preserved side {b} arrives. + LeadingHint leading = new LeadingHint("Leading"); + long a = LongBitmap.newBitmap(0); + long b = LongBitmap.newBitmap(1); + long c = LongBitmap.newBitmap(2); + + addJoinConstraint(leading, a, b, a, b, JoinType.RIGHT_OUTER_JOIN); + + long ac = LongBitmap.newBitmapUnion(a, c); + long abc = LongBitmap.newBitmapUnion(a, b, c); + + // For RIGHT OUTER JOIN at {a,c}: leftHand={a} (nullable) overlaps {a,c}, + // minRightHand={b} (preserved) is absent, and {a,c} ⊄ leftHand={a} → + // violated: nullable side mixed with unrelated table before preserved arrives. + Pair levelAC = leading.getJoinConstraint(ac, a, c); + Assertions.assertNull(levelAC.first, "right outer constraint should not match at {a,c}"); + Assertions.assertFalse(levelAC.second, + "nullable side {a} mixed with unrelated {c} before preserved {b} → violated"); + + // Level {a,b,c}: constraint matches + Pair levelABC = leading.getJoinConstraint(abc, ac, b); + Assertions.assertNotNull(levelABC.first, "constraint should match at {a,b,c}"); + Assertions.assertTrue(levelABC.second); + } + @Test public void testFullOuterJoinPredicateNotConsumedAsScanFilter() { // a FULL OUTER JOIN b ON a.v > 0 — the predicate a.v > 0 belongs to