From 67e95276fb719f035d3de875ecab3e2bf68e5a7d Mon Sep 17 00:00:00 2001
From: Kikyou1997 <33112463+Kikyou1997@users.noreply.github.com>
Date: Fri, 17 Jun 2022 10:59:33 +0800
Subject: [PATCH] [fix](optimizer) Fix the default join reorder algorithm
 (#10174)

Default join reorder algorithm not working for the most cases.
---
 fe/check/checkstyle/checkstyle.xml            |  1 -
 .../org/apache/doris/analysis/SelectStmt.java | 32 ++++++++++---------
 .../apache/doris/planner/QueryPlanTest.java   | 16 ++++++++++
 3 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/fe/check/checkstyle/checkstyle.xml b/fe/check/checkstyle/checkstyle.xml
index b941c376b7..2b005d16c2 100644
--- 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
index 9e296499c7..e9ae2aa66e 100644
--- 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 class SelectStmt extends QueryStmt {
 
     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 @@ public class SelectStmt extends QueryStmt {
 
         // 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 @@ public class SelectStmt extends QueryStmt {
                     // 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
index f580cb654a..a2a6c9e107 100644
--- 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 class QueryPlanTest extends TestWithFeService {
         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"));
+
+    }
 }