Skip to content

[Bug] Fix bug of predicate pushdown logic #3475

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
morningman merged 1 commit into from
May 6, 2020
Merged

[Bug] Fix bug of predicate pushdown logic #3475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions fe/src/main/java/org/apache/doris/analysis/Predicate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -109,7 +110,23 @@ public static boolean canPushDownPredicate(Expr 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;
}
Expand Down
44 changes: 42 additions & 2 deletions fe/src/test/java/org/apache/doris/planner/QueryPlanTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,32 @@ public static void beforeClass() throws Exception {
"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
Expand Down Expand Up @@ -532,7 +558,7 @@ public void testDateTypeEquality() throws Exception {
}

@Test
public void testJoinPredicateTransitivity() throws Exception {
public void testJoinPredicateTransitivity() throws Exception {
connectContext.setDatabase("default_cluster:test");

// test left join : left table where binary predicate
Expand Down Expand Up @@ -624,10 +650,24 @@ public void testJoinPredicateTransitivity() throws Exception {
// 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"));
}
}