From 2082d0a01faf20ba5f6477209d0613cb643cd0fb Mon Sep 17 00:00:00 2001
From: morrySnow <101034200+morrySnow@users.noreply.github.com>
Date: Wed, 1 Jun 2022 23:43:08 +0800
Subject: [PATCH] [optimize](planner)remove redundant conjuncts on plan node
 (#9819)

---
 .../org/apache/doris/planner/PlanNode.java    | 12 +++--
 .../rewrite/ExtractCommonFactorsRule.java     |  6 +++
 .../data/performance/redundant_conjuncts.out  | 39 ++++++++++++++++
 .../performance/redundant_conjuncts.groovy    | 45 +++++++++++++++++++
 4 files changed, 99 insertions(+), 3 deletions(-)
 create mode 100644 regression-test/data/performance/redundant_conjuncts.out
 create mode 100644 regression-test/suites/performance/redundant_conjuncts.groovy

diff --git a/fe/fe-core/src/main/java/org/apache/doris/planner/PlanNode.java b/fe/fe-core/src/main/java/org/apache/doris/planner/PlanNode.java
index 4b1bc1cde0..a68655eb61 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/planner/PlanNode.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/planner/PlanNode.java
@@ -375,14 +375,18 @@ abstract public class PlanNode extends TreeNode<PlanNode> {
         if (conjuncts == null) {
             return;
         }
-        this.conjuncts.addAll(conjuncts);
+        for (Expr conjunct : conjuncts) {
+            addConjunct(conjunct);
+        }
     }
 
     public void addConjunct(Expr conjunct) {
         if (conjuncts == null) {
             conjuncts = Lists.newArrayList();
         }
-        conjuncts.add(conjunct);
+        if (!conjuncts.contains(conjunct)) {
+            conjuncts.add(conjunct);
+        }
     }
 
     public void setAssignedConjuncts(Set<ExprId> conjuncts) {
@@ -636,7 +640,9 @@ abstract public class PlanNode extends TreeNode<PlanNode> {
      */
     protected void assignConjuncts(Analyzer analyzer) {
         List<Expr> unassigned = analyzer.getUnassignedConjuncts(this);
-        conjuncts.addAll(unassigned);
+        for (Expr unassignedConjunct : unassigned) {
+            addConjunct(unassignedConjunct);
+        }
         analyzer.markConjunctsAssigned(unassigned);
     }
 
diff --git a/fe/fe-core/src/main/java/org/apache/doris/rewrite/ExtractCommonFactorsRule.java b/fe/fe-core/src/main/java/org/apache/doris/rewrite/ExtractCommonFactorsRule.java
index dca24e664b..a7d7d7fa7c 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/rewrite/ExtractCommonFactorsRule.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/rewrite/ExtractCommonFactorsRule.java
@@ -25,6 +25,7 @@ import org.apache.doris.analysis.InPredicate;
 import org.apache.doris.analysis.LiteralExpr;
 import org.apache.doris.analysis.SlotRef;
 import org.apache.doris.common.AnalysisException;
+import org.apache.doris.planner.PlanNode;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.BoundType;
@@ -53,6 +54,11 @@ import java.util.Set;
  *        -> (1<k1<4) and k2 in('Marry','Tom') and (1<k1<3 and k2 in ('Marry')) or (2<k1<4 and k2 in ('Tom'))
  *
  * The second rewriting can be controlled by session variable 'extract_wide_range_expr'
+ *
+ * TODO: extract wide common factors could generate redundant conjuncts when whole expression could be extracted.
+ * Currently, redundant conjuncts will be removed when be assigned to {@link PlanNode}
+ * by calling {@link PlanNode#addConjunct(Expr)}, {@link PlanNode#addConjuncts(List)}, {@link PlanNode#init(Analyzer)}.
+ * But, we should remove redundant conjuncts generated by redundant conjuncts in this rule.
  */
 public class ExtractCommonFactorsRule implements ExprRewriteRule {
     private final static Logger LOG = LogManager.getLogger(ExtractCommonFactorsRule.class);
diff --git a/regression-test/data/performance/redundant_conjuncts.out b/regression-test/data/performance/redundant_conjuncts.out
new file mode 100644
index 0000000000..ab2eadd2ea
--- /dev/null
+++ b/regression-test/data/performance/redundant_conjuncts.out
@@ -0,0 +1,39 @@
+-- This file is automatically generated. You should know what you did if you want to edit this
+-- !redundant_conjuncts --
+PLAN FRAGMENT 0
+ OUTPUT EXPRS:`v1`
+  PARTITION: HASH_PARTITIONED: `default_cluster:regression_test`.`redundant_conjuncts`.`k1`
+
+  VRESULT SINK
+
+  0:VOlapScanNode
+     TABLE: redundant_conjuncts
+     PREAGGREGATION: OFF. Reason: No AggregateInfo
+     PREDICATES: `k1` = 1
+     partitions=0/1
+     rollup: null
+     tabletRatio=0/0
+     tabletList=
+     cardinality=0
+     avgRowSize=8.0
+     numNodes=1
+
+-- !redundant_conjuncts_gnerated_by_extract_common_filter --
+PLAN FRAGMENT 0
+ OUTPUT EXPRS:`v1`
+  PARTITION: HASH_PARTITIONED: `default_cluster:regression_test`.`redundant_conjuncts`.`k1`
+
+  VRESULT SINK
+
+  0:VOlapScanNode
+     TABLE: redundant_conjuncts
+     PREAGGREGATION: OFF. Reason: No AggregateInfo
+     PREDICATES: (`k1` = 1 OR `k1` = 2)
+     partitions=0/1
+     rollup: null
+     tabletRatio=0/0
+     tabletList=
+     cardinality=0
+     avgRowSize=8.0
+     numNodes=1
+
diff --git a/regression-test/suites/performance/redundant_conjuncts.groovy b/regression-test/suites/performance/redundant_conjuncts.groovy
new file mode 100644
index 0000000000..9ffee97a6a
--- /dev/null
+++ b/regression-test/suites/performance/redundant_conjuncts.groovy
@@ -0,0 +1,45 @@
+// 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("redundant_conjuncts", "performance") {
+    sql """
+        SET enable_vectorized_engine = true;
+    """
+
+    sql """
+    DROP TABLE IF EXISTS redundant_conjuncts;
+    """
+    sql """
+    CREATE TABLE `redundant_conjuncts` (
+      `k1` int(11) NULL COMMENT "",
+      `v1` int(11) NULL COMMENT ""
+    ) ENGINE=OLAP
+    DUPLICATE KEY(`k1`, `v1`)
+    DISTRIBUTED BY HASH(`k1`) BUCKETS 10
+    PROPERTIES (
+      "replication_allocation" = "tag.location.default: 1"
+    );
+    """
+    
+    qt_redundant_conjuncts """
+    EXPLAIN SELECT v1 FROM redundant_conjuncts WHERE k1 = 1 AND k1 = 1;
+    """
+
+    qt_redundant_conjuncts_gnerated_by_extract_common_filter """
+    EXPLAIN SELECT v1 FROM redundant_conjuncts WHERE k1 = 1 OR k1 = 2;
+    """
+}