You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jc...@apache.org on 2018/11/28 05:37:17 UTC

[2/2] calcite git commit: [CALCITE-2687] Is distinct from could lead to Exceptions in ReduceExpressionRule (Zoltan Haindrich)

[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 <ki...@rxd.hu>
Authored: Tue Nov 20 09:28:20 2018 +0100
Committer: Jesus Camacho Rodriguez <jc...@apache.org>
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">
             <![CDATA[