github-actions[bot] commented on code in PR #65205:
URL: https://github.com/apache/doris/pull/65205#discussion_r3522961528
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java:
##########
@@ -418,7 +418,7 @@ public Pair<JoinConstraint, Boolean> getJoinConstraint(Long
joinTableBitmap, Lon
if (joinConstraint.getJoinType().isSemiJoin()) {
if (LongBitmap.isSubset(joinConstraint.getRightHand(),
leftTableBitmap)
- && !LongBitmap.isSubset(joinConstraint.getRightHand(),
leftTableBitmap)) {
+ &&
!joinConstraint.getRightHand().equals(leftTableBitmap)) {
Review Comment:
This guard is still too narrow. First, it only runs for `isSemiJoin()`, so
`A LEFT ANTI JOIN B` can still match a leading step like `(B JOIN C) JOIN A`,
skip this block, take the reversed branch below, and become a `RIGHT_ANTI_JOIN`
that tests absence from `B JOIN C` instead of absence from `B`. Second, direct
right semi/right anti constraints need the exact check on the original
left/filtering side, not `rightHand`: for `A RIGHT SEMI JOIN B`,
`getJoinConstraint(A | B | C, A | C, B)` passes this check because `rightHand
== B`, then applies the semi test against `A JOIN C`. Please make the
exact-side check cover anti joins and choose the constrained side by join
direction, with left/right semi and anti unit cases.
##########
fe/fe-core/src/test/java/org/apache/doris/nereids/hint/LeadingHintTest.java:
##########
@@ -0,0 +1,51 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.hint;
+
+import org.apache.doris.common.Pair;
+import org.apache.doris.nereids.jobs.joinorder.hypergraph.bitmap.LongBitmap;
+import org.apache.doris.nereids.trees.plans.JoinType;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+public class LeadingHintTest {
+
+ @Test
+ public void testSemiJoinRightHandMatchesExactlyInLeadingConstraint() {
+ LeadingHint leading = new LeadingHint("Leading");
+ long leftHand = LongBitmap.newBitmap(0);
+ long rightHand = LongBitmap.newBitmap(1);
+ long extraTable = LongBitmap.newBitmap(2);
+ JoinConstraint semiJoinConstraint = new JoinConstraint(
+ leftHand, rightHand, leftHand, rightHand,
JoinType.LEFT_SEMI_JOIN, true);
+ leading.getJoinConstraintList().add(semiJoinConstraint);
+
+ Pair<JoinConstraint, Boolean> exactReversed =
leading.getJoinConstraint(
+ LongBitmap.newBitmapUnion(leftHand, rightHand), rightHand,
leftHand);
+ Assertions.assertSame(semiJoinConstraint, exactReversed.first);
+ Assertions.assertTrue(exactReversed.first.isReversed());
+
+ Pair<JoinConstraint, Boolean> withExtraTable =
leading.getJoinConstraint(
Review Comment:
The new test covers `rightHand + extra` being in one child, but it still
misses the case where the leading order tries to build `rightHand` with some
unrelated table before the semi join's original left side is present. For a
constraint from `A LEFT SEMI JOIN B`, `getJoinConstraint(B | C, B, C)` passes
this new guard because `rightHand.equals(leftTableBitmap)`, then falls through
to the reversed semi case at lines 451-458 and returns a `RIGHT_SEMI_JOIN` even
though `A` is not part of the current join. That can build the semi join before
the retained side exists, and the later plan still carries the bitmap for `B`
even though the semi output dropped it. Please add this shape to the test and
make semi constraints refuse to match until the required left side is also in
the current join.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
