This is an automated email from the ASF dual-hosted git repository.

yiguolei pushed a commit to branch branch-2.1
in repository https://gitbox.apache.org/repos/asf/doris.git

commit 17359d59a397de6cdf12058ad99d9b87e32dca89
Author: morrySnow <[email protected]>
AuthorDate: Thu Feb 29 11:10:03 2024 +0800

    [fix](Nereids) reorder join generate plan is not stable (#31539)
---
 .../doris/nereids/rules/rewrite/ReorderJoin.java   | 23 ++++---
 .../reorder_join/more_than_one_cross_join.groovy   | 77 ++++++++++++++++++++++
 2 files changed, 91 insertions(+), 9 deletions(-)

diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ReorderJoin.java
 
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ReorderJoin.java
index 200e1796a8d..97af548ce02 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ReorderJoin.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ReorderJoin.java
@@ -88,7 +88,8 @@ public class ReorderJoin extends OneRewriteRuleFactory {
 
                 Map<Plan, JoinDistributeType> planToHintType = 
Maps.newHashMap();
                 Plan plan = joinToMultiJoin(filter, planToHintType);
-                Preconditions.checkState(plan instanceof MultiJoin);
+                Preconditions.checkState(plan instanceof MultiJoin, "join to 
multi join should return MultiJoin,"
+                        + " but return plan is " + plan.getType());
                 MultiJoin multiJoin = (MultiJoin) plan;
                 ctx.statementContext.addJoinFilters(multiJoin.getJoinFilter());
                 
ctx.statementContext.setMaxNAryInnerJoin(multiJoin.children().size());
@@ -331,14 +332,13 @@ public class ReorderJoin extends OneRewriteRuleFactory {
      */
     private LogicalJoin<? extends Plan, ? extends Plan> findInnerJoin(Plan 
left, List<Plan> candidates,
             Set<Expression> joinFilter, Set<Integer> usedPlansIndex, Map<Plan, 
JoinDistributeType> planToHintType) {
-        List<Expression> otherJoinConditions = Lists.newArrayList();
+        List<Expression> firstOtherJoinConditions = 
ExpressionUtils.EMPTY_CONDITION;
+        int firstCandidate = -1;
         Set<ExprId> leftOutputExprIdSet = left.getOutputExprIdSet();
-        int candidateIndex = 0;
         for (int i = 0; i < candidates.size(); i++) {
             if (usedPlansIndex.contains(i)) {
                 continue;
             }
-            candidateIndex = i;
 
             Plan candidate = candidates.get(i);
             Set<ExprId> rightOutputExprIdSet = candidate.getOutputExprIdSet();
@@ -356,24 +356,29 @@ public class ReorderJoin extends OneRewriteRuleFactory {
             Pair<List<Expression>, List<Expression>> pair = 
JoinUtils.extractExpressionForHashTable(
                     left.getOutput(), candidate.getOutput(), 
currentJoinFilter);
             List<Expression> hashJoinConditions = pair.first;
-            otherJoinConditions = pair.second;
             if (!hashJoinConditions.isEmpty()) {
                 usedPlansIndex.add(i);
                 return new LogicalJoin<>(JoinType.INNER_JOIN,
-                        hashJoinConditions, otherJoinConditions,
+                        hashJoinConditions, pair.second,
                         new 
DistributeHint(DistributeType.fromRightPlanHintType(
                                 planToHintType.getOrDefault(candidate, 
JoinDistributeType.NONE))),
                         Optional.empty(),
                         left, candidate, null);
+            } else {
+                if (firstCandidate == -1) {
+                    firstCandidate = i;
+                    firstOtherJoinConditions = pair.second;
+                }
             }
         }
         // All { left -> one in [candidates] } is CrossJoin
         // Generate a CrossJoin
-        usedPlansIndex.add(candidateIndex);
-        Plan right = candidates.get(candidateIndex);
+        // NOTICE: we must traverse for head to tail to ensure result is 
stable.
+        usedPlansIndex.add(firstCandidate);
+        Plan right = candidates.get(firstCandidate);
         return new LogicalJoin<>(JoinType.CROSS_JOIN,
                 ExpressionUtils.EMPTY_CONDITION,
-                otherJoinConditions,
+                firstOtherJoinConditions,
                 new DistributeHint(DistributeType.fromRightPlanHintType(
                         planToHintType.getOrDefault(right, 
JoinDistributeType.NONE))),
                 Optional.empty(),
diff --git 
a/regression-test/suites/nereids_rules_p0/reorder_join/more_than_one_cross_join.groovy
 
b/regression-test/suites/nereids_rules_p0/reorder_join/more_than_one_cross_join.groovy
new file mode 100644
index 00000000000..66897070d00
--- /dev/null
+++ 
b/regression-test/suites/nereids_rules_p0/reorder_join/more_than_one_cross_join.groovy
@@ -0,0 +1,77 @@
+// 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.
+
+suite("more_than_one_cross_join") {
+    sql "SET enable_nereids_planner=true"
+    sql "SET enable_fallback_to_original_planner=false"
+
+    sql """
+        DROP TABLE IF EXISTS more_than_one_cross_join1
+    """
+    sql """
+        DROP TABLE IF EXISTS more_than_one_cross_join2
+    """
+
+    sql """
+        CREATE TABLE `more_than_one_cross_join1` (
+          `pk` INT NULL,
+          `cv10` VARCHAR(10) NULL,
+          `ci` INT NULL,
+          `cv1024` VARCHAR(1024) NULL
+        ) ENGINE=OLAP
+        DUPLICATE KEY(`pk`, `cv10`)
+        COMMENT 'OLAP'
+        DISTRIBUTED BY HASH(`pk`) BUCKETS 10
+        PROPERTIES (
+        "replication_allocation" = "tag.location.default: 1"
+        )
+    """
+
+    sql """
+        CREATE TABLE `more_than_one_cross_join2` (
+          `pk` INT NULL,
+          `cv10` VARCHAR(10) NULL,
+          `ci` INT NULL,
+          `cv1024` VARCHAR(1024) NULL
+        ) ENGINE=OLAP
+        DUPLICATE KEY(`pk`, `cv10`)
+        COMMENT 'OLAP'
+        DISTRIBUTED BY HASH(`pk`) BUCKETS 10
+        PROPERTIES (
+        "replication_allocation" = "tag.location.default: 1"
+        )
+    """
+
+    sql """
+        SELECT
+          alias1.pk AS field1, 
+          alias1.pk AS field2,
+          alias2.ci AS field3
+        FROM
+          more_than_one_cross_join1 AS alias1, 
+          more_than_one_cross_join1 AS alias2, 
+          more_than_one_cross_join2 AS alias3 
+        WHERE
+          alias1.pk != alias3.ci
+        GROUP BY
+          field1, field2, field3
+        HAVING
+          (((field3 < 1 AND field1 = 3) AND field2 = 5) OR field3 > 0)
+        ORDER BY
+          field1, field2, field3
+    """
+}


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to