You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by mo...@apache.org on 2020/05/06 07:15:47 UTC

[incubator-doris] branch master updated: [Bug] Fix bug of predicate pushdown logic (#3475)

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

morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-doris.git


The following commit(s) were added to refs/heads/master by this push:
     new 101628c  [Bug] Fix bug of predicate pushdown logic (#3475)
101628c is described below

commit 101628c81314f1597e1018faa41414b7a19037d8
Author: Mingyu Chen <mo...@gmail.com>
AuthorDate: Wed May 6 15:15:37 2020 +0800

    [Bug] Fix bug of predicate pushdown logic (#3475)
    
    When there is subquery in where clause, the query will be rewritten to join operation.
    And some auxiliary binary predicates will be generated. These binary predicates
    will not go through the ExprRewriteRule, so they are not normalized as
    "column to the left and constant to the right" format.
    
    We need to take this case into account so that the `canPushDownPredicate()` judgement
    will not throw exception.
---
 .../java/org/apache/doris/analysis/Predicate.java  | 21 ++++++++++-
 .../org/apache/doris/planner/QueryPlanTest.java    | 44 +++++++++++++++++++++-
 2 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/fe/src/main/java/org/apache/doris/analysis/Predicate.java b/fe/src/main/java/org/apache/doris/analysis/Predicate.java
index d345cdb..3e2c22b 100644
--- a/fe/src/main/java/org/apache/doris/analysis/Predicate.java
+++ b/fe/src/main/java/org/apache/doris/analysis/Predicate.java
@@ -17,12 +17,13 @@
 
 package org.apache.doris.analysis;
 
-import com.google.common.base.Preconditions;
 import org.apache.doris.catalog.Type;
 import org.apache.doris.common.AnalysisException;
 import org.apache.doris.common.Pair;
 import org.apache.doris.common.Reference;
 
+import com.google.common.base.Preconditions;
+
 public abstract class Predicate extends Expr {
     protected boolean isEqJoinConjunct;
 
@@ -109,7 +110,23 @@ public abstract class Predicate extends Expr {
 
                 // because isSingleColumnPredicate
                 Preconditions.checkState(right != null);
-                Preconditions.checkState(right.isConstant());
+
+                // ATTN(cmy): Usually, the BinaryPredicate in the query will be rewritten through ExprRewriteRule,
+                // and all SingleColumnPredicate will be rewritten as "column on the left and the constant on the right".
+                // So usually the right child is constant.
+                //
+                // But if there is a subquery in where clause, the planner will rewrite the subquery to join.
+                // During the rewrite, some auxiliary BinaryPredicate will be automatically generated,
+                // and these BinaryPredicates will not go through ExprRewriteRule.
+                // As a result, these BinaryPredicates may be as "column on the right and the constant on the left".
+                // Example can be found in QueryPlanTest.java -> testJoinPredicateTransitivityWithSubqueryInWhereClause().
+                //
+                // Because our current planner implementation is very error-prone, so when this happens,
+                // we simply assume that these kind of BinaryPredicates cannot be pushed down,
+                // to ensure that this change will not affect other query plans.
+                if (!right.isConstant()) {
+                    return false;
+                }
 
                 return right instanceof LiteralExpr;
             }
diff --git a/fe/src/test/java/org/apache/doris/planner/QueryPlanTest.java b/fe/src/test/java/org/apache/doris/planner/QueryPlanTest.java
index 53a3c15..c192d18 100644
--- a/fe/src/test/java/org/apache/doris/planner/QueryPlanTest.java
+++ b/fe/src/test/java/org/apache/doris/planner/QueryPlanTest.java
@@ -242,6 +242,32 @@ public class QueryPlanTest {
                 "PROPERTIES (\n" +
                 " \"replication_num\" = \"1\"\n" +
                 ");");
+        
+        createTable("CREATE TABLE test.`pushdown_test` (\n" +
+                "  `k1` tinyint(4) NULL COMMENT \"\",\n" + 
+                "  `k2` smallint(6) NULL COMMENT \"\",\n" + 
+                "  `k3` int(11) NULL COMMENT \"\",\n" + 
+                "  `k4` bigint(20) NULL COMMENT \"\",\n" + 
+                "  `k5` decimal(9, 3) NULL COMMENT \"\",\n" + 
+                "  `k6` char(5) NULL COMMENT \"\",\n" + 
+                "  `k10` date NULL COMMENT \"\",\n" + 
+                "  `k11` datetime NULL COMMENT \"\",\n" + 
+                "  `k7` varchar(20) NULL COMMENT \"\",\n" + 
+                "  `k8` double MAX NULL COMMENT \"\",\n" + 
+                "  `k9` float SUM NULL COMMENT \"\"\n" + 
+                ") ENGINE=OLAP\n" + 
+                "AGGREGATE KEY(`k1`, `k2`, `k3`, `k4`, `k5`, `k6`, `k10`, `k11`, `k7`)\n" + 
+                "COMMENT \"OLAP\"\n" + 
+                "PARTITION BY RANGE(`k1`)\n" + 
+                "(PARTITION p1 VALUES [(\"-128\"), (\"-64\")),\n" + 
+                "PARTITION p2 VALUES [(\"-64\"), (\"0\")),\n" + 
+                "PARTITION p3 VALUES [(\"0\"), (\"64\")))\n" + 
+                "DISTRIBUTED BY HASH(`k1`) BUCKETS 5\n" + 
+                "PROPERTIES (\n" + 
+                "\"replication_num\" = \"1\",\n" + 
+                "\"in_memory\" = \"false\",\n" + 
+                "\"storage_format\" = \"DEFAULT\"\n" + 
+                ");");
     }
 
     @AfterClass
@@ -532,7 +558,7 @@ public class QueryPlanTest {
     }
 
     @Test
-    public void  testJoinPredicateTransitivity() throws Exception {
+    public void testJoinPredicateTransitivity() throws Exception {
         connectContext.setDatabase("default_cluster:test");
 
         // test left join : left table where binary predicate
@@ -624,10 +650,24 @@ public class QueryPlanTest {
         // test inner join: right table join predicate, both push down left table and right table
         sql = "select *\n from join1\n" +
                 "join join2 on join1.id = join2.id\n" +
-                "and join2.id > 1;";
+                "and 1 < join2.id;";
         explainString = UtFrameUtils.getSQLPlanOrErrorMsg(connectContext, "explain " + sql);
         System.out.println(explainString);
         Assert.assertTrue(explainString.contains("PREDICATES: `join1`.`id` > 1"));
         Assert.assertTrue(explainString.contains("PREDICATES: `join2`.`id` > 1"));
     }
+    
+    @Test
+    public void testJoinPredicateTransitivityWithSubqueryInWhereClause() throws Exception {
+        connectContext.setDatabase("default_cluster:test");
+        String sql = "SELECT *\n" + 
+                "FROM test.pushdown_test\n" +
+                "WHERE 0 < (\n" +
+                "    SELECT MAX(k9)\n" + 
+                "    FROM test.pushdown_test);";
+        String explainString = UtFrameUtils.getSQLPlanOrErrorMsg(connectContext, "explain " + sql);
+        Assert.assertTrue(explainString.contains("PLAN FRAGMENT"));
+        Assert.assertTrue(explainString.contains("CROSS JOIN"));
+        Assert.assertTrue(!explainString.contains("PREDICATES"));
+    }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org