[fix](optimizer) Fix the default join reorder algorithm (#10174)

Default join reorder algorithm not working for the most cases.
apache · Jun 17, 2022 · 67e9527 · 67e9527
1 parent fd0bd39
commit 67e9527
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 16 deletions.
diff --git a/fe/check/checkstyle/checkstyle.xml b/fe/check/checkstyle/checkstyle.xml
@@ -178,7 +178,6 @@ under the License.
         <module name="UnusedLocalVariable">
             <property name="severity" value="error"/>
         </module>
-        <module name="VariableDeclarationUsageDistance"/>
 
         <!-- Headers -->
         <!-- Imports -->

diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java
@@ -735,7 +735,7 @@ public void materializeRequiredSlots(Analyzer analyzer) throws AnalysisException
 
     protected void reorderTable(Analyzer analyzer) throws AnalysisException {
         List<Pair<TableRef, Long>> candidates = Lists.newArrayList();
-
+        List<TableRef> originOrderBackUp = Lists.newArrayList(fromClause.getTableRefs());
         // New pair of table ref and row count
         for (TableRef tblRef : fromClause) {
             if (tblRef.getJoinOp() != JoinOperator.INNER_JOIN || tblRef.hasJoinHints()) {
@@ -773,8 +773,8 @@ protected void reorderTable(Analyzer analyzer) throws AnalysisException {
 
         // can not get AST only with equal join, MayBe cross join can help
         fromClause.clear();
-        for (Pair<TableRef, Long> candidate : candidates) {
-            fromClause.add(candidate.first);
+        for (TableRef tableRef : originOrderBackUp) {
+            fromClause.add(tableRef);
         }
     }
 
@@ -819,19 +819,21 @@ protected boolean reorderTable(Analyzer analyzer, TableRef firstRef)
                     // is being added.
                     Preconditions.checkState(tid == candidateTableRef.getId());
                     List<Expr> candidateEqJoinPredicates = analyzer.getEqJoinConjunctsExcludeAuxPredicates(tid);
-                    List<TupleId> candidateTupleList = Lists.newArrayList();
-                    Expr.getIds(candidateEqJoinPredicates, candidateTupleList, null);
-                    int count = candidateTupleList.size();
-                    for (TupleId tupleId : candidateTupleList) {
-                        if (validTupleId.contains(tupleId) || tid == tupleId) {
-                            count--;
+                    for (Expr candidateEqJoinPredicate : candidateEqJoinPredicates) {
+                        List<TupleId> candidateTupleList = Lists.newArrayList();
+                        Expr.getIds(Lists.newArrayList(candidateEqJoinPredicate), candidateTupleList, null);
+                        int count = candidateTupleList.size();
+                        for (TupleId tupleId : candidateTupleList) {
+                            if (validTupleId.contains(tupleId) || tid.equals(tupleId)) {
+                                count--;
+                            }
+                        }
+                        if (count == 0) {
+                            fromClause.add(candidateTableRef);
+                            validTupleId.add(tid);
+                            tableRefMap.remove(tid);
+                            break;
                         }
-                    }
-
-                    if (count == 0) {
-                        fromClause.add(candidateTableRef);
-                        validTupleId.add(tid);
-                        tableRefMap.remove(tid);
                     }
                 }
             }

diff --git a/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java b/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java
@@ -2118,4 +2118,20 @@ public void testOutJoinWithOnFalse() throws Exception {
         Assert.assertFalse(explainString.contains("non-equal FULL OUTER JOIN is not supported"));
 
     }
+
+    @Test
+    public void testDefaultJoinReorder() throws Exception {
+        connectContext.setDatabase("default_cluster:test");
+        createTable("CREATE TABLE t1 (col1 varchar, col2 varchar, col3 int)\n" + "DISTRIBUTED BY HASH(col3)\n"
+                + "BUCKETS 3\n" + "PROPERTIES(\n" + "    \"replication_num\"=\"1\"\n" + ");");
+        createTable("CREATE TABLE t2 (col1 varchar, col2 varchar, col3 int)\n" + "DISTRIBUTED BY HASH(col3)\n"
+                + "BUCKETS 3\n" + "PROPERTIES(\n" + "    \"replication_num\"=\"1\"\n" + ");");
+        createTable("CREATE TABLE t3 (col1 varchar, col2 varchar, col3 int)\n" + "DISTRIBUTED BY HASH(col3)\n"
+                + "BUCKETS 3\n" + "PROPERTIES(\n" + "    \"replication_num\"=\"1\"\n" + ");");
+        String sql = "explain select x.col2 from t1,t2,t3 x,t3 y "
+                + "where x.col1=t2.col1 and y.col1=t2.col2 and t1.col1=y.col1";
+        String explainString = getSQLPlanOrErrorMsg(sql);
+        Assert.assertFalse(explainString.contains("CROSS JOIN"));
+
+    }
 }