[CALCITE-2687] Is distinct from could lead to Exceptions in ReduceExpressionRule (Zoltan Haindrich)
Close apache/calcite#929 Project: http://git-wip-us.apache.org/repos/asf/calcite/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/463255c7 Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/463255c7 Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/463255c7 Branch: refs/heads/master Commit: 463255c75f7e83081f56e657549d69d4feb7dbdf Parents: da57c90 Author: Zoltan Haindrich <[email protected]> Authored: Tue Nov 20 09:28:20 2018 +0100 Committer: Jesus Camacho Rodriguez <[email protected]> Committed: Tue Nov 27 20:33:11 2018 -0800 ---------------------------------------------------------------------- .../org/apache/calcite/plan/RelOptUtil.java | 48 ++++++++------------ .../apache/calcite/test/RelOptRulesTest.java | 14 +++++- .../org/apache/calcite/test/RelOptRulesTest.xml | 27 ++++++++++- .../calcite/test/SqlToRelConverterTest.xml | 23 ++++++++-- 4 files changed, 74 insertions(+), 38 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite/blob/463255c7/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java index e15e491..0e68a11 100644 --- a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java +++ b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java @@ -1927,38 +1927,26 @@ public abstract class RelOptUtil { RexNode x, RexNode y, boolean neg) { - SqlOperator nullOp; - SqlOperator eqOp; + if (neg) { - nullOp = SqlStdOperatorTable.IS_NULL; - eqOp = SqlStdOperatorTable.EQUALS; + // x is not distinct from y + // x=y IS TRUE or ((x is null) and (y is null)), + return rexBuilder.makeCall(SqlStdOperatorTable.OR, + rexBuilder.makeCall(SqlStdOperatorTable.AND, + rexBuilder.makeCall(SqlStdOperatorTable.IS_NULL, x), + rexBuilder.makeCall(SqlStdOperatorTable.IS_NULL, y)), + rexBuilder.makeCall(SqlStdOperatorTable.IS_TRUE, + rexBuilder.makeCall(SqlStdOperatorTable.EQUALS, x, y))); } else { - nullOp = SqlStdOperatorTable.IS_NOT_NULL; - eqOp = SqlStdOperatorTable.NOT_EQUALS; + // x is distinct from y + // x=y IS NOT TRUE and ((x is not null) or (y is not null)), + return rexBuilder.makeCall(SqlStdOperatorTable.AND, + rexBuilder.makeCall(SqlStdOperatorTable.OR, + rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, x), + rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, y)), + rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_TRUE, + rexBuilder.makeCall(SqlStdOperatorTable.EQUALS, x, y))); } - // By the time the ELSE is reached, x and y are known to be not null; - // therefore the whole CASE is not null. - RexNode[] whenThenElse = { - // when x is null - rexBuilder.makeCall(SqlStdOperatorTable.IS_NULL, x), - - // then return y is [not] null - rexBuilder.makeCall(nullOp, y), - - // when y is null - rexBuilder.makeCall(SqlStdOperatorTable.IS_NULL, y), - - // then return x is [not] null - rexBuilder.makeCall(nullOp, x), - - // else return x compared to y - rexBuilder.makeCall(eqOp, - rexBuilder.makeNotNull(x), - rexBuilder.makeNotNull(y)) - }; - return rexBuilder.makeCall( - SqlStdOperatorTable.CASE, - whenThenElse); } /** @@ -2703,7 +2691,7 @@ public abstract class RelOptUtil { //noinspection unchecked return (T) rel.copy( rel.getTraitSet().replace(trait), - (List) rel.getInputs()); + rel.getInputs()); } /** http://git-wip-us.apache.org/repos/asf/calcite/blob/463255c7/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java index e91af51..3db0ccc 100644 --- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java @@ -1791,10 +1791,22 @@ public class RelOptRulesTest extends RelOptTestBase { .addRuleInstance(ReduceExpressionsRule.JOIN_INSTANCE) .build(); - checkPlanUnchanged(new HepPlanner(program), + checkPlanning(new HepPlanner(program), "select p1 is not distinct from p0 from (values (2, cast(null as integer))) as t(p0, p1)"); } + @Test public void testReduceConstants3() throws Exception { + HepProgram program = new HepProgramBuilder() + .addRuleInstance(ReduceExpressionsRule.PROJECT_INSTANCE) + .addRuleInstance(ReduceExpressionsRule.FILTER_INSTANCE) + .addRuleInstance(ReduceExpressionsRule.JOIN_INSTANCE) + .build(); + + final String sql = "select e.mgr is not distinct from f.mgr " + + "from emp e join emp f on (e.mgr=f.mgr) where e.mgr is null"; + checkPlanning(new HepPlanner(program), sql); + } + /** Test case for * <a href="https://issues.apache.org/jira/browse/CALCITE-902";>[CALCITE-902] * Match nullability when reducing expressions in a Project</a>. */ http://git-wip-us.apache.org/repos/asf/calcite/blob/463255c7/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml ---------------------------------------------------------------------- diff --git a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml index 7d90b75..883338a 100644 --- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml @@ -2194,7 +2194,7 @@ LogicalCalc(expr#0=[{inputs}], expr#1=['TABLE '], expr#2=['t'], U=[$t1], </Resource> <Resource name="planBefore"> <![CDATA[ -LogicalProject(EXPR$0=[false]) +LogicalProject(EXPR$0=[IS TRUE(null)]) LogicalValues(tuples=[[{ 0 }]]) ]]> </Resource> @@ -2590,7 +2590,7 @@ LogicalFilter(condition=[IS NOT DISTINCT FROM($7, 20)]) </Resource> <Resource name="planAfter"> <![CDATA[ -LogicalFilter(condition=[CASE(IS NULL($7), false, =(CAST($7):TINYINT NOT NULL, 20))]) +LogicalFilter(condition=[IS TRUE(=($7, 20))]) LogicalTableScan(table=[[scott, EMP]]) ]]> </Resource> @@ -6200,6 +6200,29 @@ LogicalProject(QX=[CAST(CASE(=($0, 1), 1, 2)):INTEGER]) ]]> </Resource> </TestCase> + <TestCase name="testReduceConstants3"> + <Resource name="sql"> + <![CDATA[select e.mgr is not distinct from f.mgr from emp e join emp f on (e.mgr=f.mgr) where e.mgr is null]]> + </Resource> + <Resource name="planBefore"> + <![CDATA[ +LogicalProject(EXPR$0=[OR(AND(IS NULL($3), IS NULL($12)), IS TRUE(=($3, $12)))]) + LogicalFilter(condition=[IS NULL($3)]) + LogicalJoin(condition=[=($3, $12)], joinType=[inner]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + </Resource> + <Resource name="planAfter"> + <![CDATA[ +LogicalProject(EXPR$0=[IS NULL($12)]) + LogicalFilter(condition=[IS NULL($3)]) + LogicalJoin(condition=[=($3, $12)], joinType=[inner]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + </Resource> + </TestCase> <TestCase name="testReduceConstantsDynamicFunction"> <Resource name="sql"> <![CDATA[select sal, t http://git-wip-us.apache.org/repos/asf/calcite/blob/463255c7/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml ---------------------------------------------------------------------- diff --git a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml index db6d2a9..a289675 100644 --- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml @@ -645,7 +645,7 @@ LogicalProject(DEPTNO=[$7]) <TestCase name="testIsDistinctFrom"> <Resource name="plan"> <![CDATA[ -LogicalProject(EXPR$0=[CASE(IS NULL($0), IS NOT NULL($1), IS NULL($1), IS NOT NULL($0), <>(CAST($0):INTEGER NOT NULL, CAST($1):INTEGER NOT NULL))]) +LogicalProject(EXPR$0=[AND(OR(IS NOT NULL($0), IS NOT NULL($1)), IS NOT TRUE(=($0, $1)))]) LogicalUnion(all=[true]) LogicalProject(EXPR$0=[null], EXPR$1=[1]) LogicalValues(tuples=[[{ 0 }]]) @@ -662,7 +662,7 @@ from (values (cast(null as int), 1), <TestCase name="testIsNotDistinctFrom"> <Resource name="plan"> <![CDATA[ -LogicalProject(EXPR$0=[CASE(IS NULL($0), IS NULL($1), IS NULL($1), IS NULL($0), =(CAST($0):INTEGER NOT NULL, CAST($1):INTEGER NOT NULL))]) +LogicalProject(EXPR$0=[OR(AND(IS NULL($0), IS NULL($1)), IS TRUE(=($0, $1)))]) LogicalUnion(all=[true]) LogicalProject(EXPR$0=[null], EXPR$1=[1]) LogicalValues(tuples=[[{ 0 }]]) @@ -5298,7 +5298,10 @@ LogicalProject(ANYEMPNO=[$1]) </TestCase> <TestCase name="testWithinGroup1"> <Resource name="sql"> - <![CDATA[select deptno, collect(empno) within group (order by deptno, hiredate desc) from emp group by deptno]]> + <![CDATA[select deptno, + collect(empno) within group (order by deptno, hiredate desc) +from emp +group by deptno]]> </Resource> <Resource name="plan"> <![CDATA[ @@ -5310,7 +5313,14 @@ LogicalAggregate(group=[{0}], EXPR$1=[COLLECT($1) WITHIN GROUP ([1, 2 DESC])]) </TestCase> <TestCase name="testWithinGroup2"> <Resource name="sql"> - <![CDATA[select dept.deptno, collect(sal) within group (order by sal desc) as s, collect(sal) within group (order by 1)as s1, collect(sal) within group (order by sal) filter (where sal > 2000) as s2 from emp join dept using (deptno) group by dept.deptn]]> + <![CDATA[select dept.deptno, + collect(sal) within group (order by sal desc) as s, + collect(sal) within group (order by 1)as s1, + collect(sal) within group (order by sal) + filter (where sal > 2000) as s2 +from emp +join dept using (deptno) +group by dept.deptno]]> </Resource> <Resource name="plan"> <![CDATA[ @@ -5324,7 +5334,10 @@ LogicalAggregate(group=[{0}], S=[COLLECT($1) WITHIN GROUP ([1 DESC])], S1=[COLLE </TestCase> <TestCase name="testWithinGroup3"> <Resource name="sql"> - <![CDATA[select deptno, collect(empno) filter (where empno not in (1, 2)), count(*) from emp group by deptno]]> + <![CDATA[select deptno, + collect(empno) within group (order by empno not in (1, 2)), count(*) +from emp +group by deptno]]> </Resource> <Resource name="plan"> <