You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by kr...@apache.org on 2019/03/20 15:11:12 UTC

[calcite] branch master updated: [CALCITE-2929] Simplification of IS NULL checks are incorrectly assuming that CAST-s are possible

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

krisden pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/master by this push:
     new a648f9c  [CALCITE-2929] Simplification of IS NULL checks are incorrectly assuming that CAST-s are possible
a648f9c is described below

commit a648f9c12309cc253628930b0cab98591caa66ab
Author: Zoltan Haindrich <ki...@rxd.hu>
AuthorDate: Mon Mar 18 09:56:07 2019 +0100

    [CALCITE-2929] Simplification of IS NULL checks are incorrectly assuming that CAST-s are possible
    
    * for non-nullable types the cast was discarded in case of an IS NULL check; so it might resulted in missing errors
      cast('invalid' as int) IS NULL
    * in case of non-nullable types; IS NULL have made the assumption that cast will not change its nullability (which is true); however it may raise an error - so it can't be removed.
    
    Close apache/calcite#1116
    
    Signed-off-by: Kevin Risden <kr...@apache.org>
---
 core/src/main/java/org/apache/calcite/rex/RexSimplify.java | 11 +++++++++--
 .../test/java/org/apache/calcite/test/RexProgramTest.java  | 14 ++++++++++++++
 core/src/test/resources/sql/sub-query.iq                   |  6 +++---
 .../java/org/apache/calcite/test/SparkAdapterTest.java     |  2 +-
 4 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
index f6eecd4..101bea7 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
@@ -708,12 +708,15 @@ public class RexSimplify {
   }
 
   private RexNode simplifyIsNotNull(RexNode a) {
-    if (!a.getType().isNullable()) {
+    if (!a.getType().isNullable() && isSafeExpression(a)) {
       return rexBuilder.makeLiteral(true);
     }
     if (predicates.pulledUpPredicates.contains(a)) {
       return rexBuilder.makeLiteral(true);
     }
+    if (a.getKind() == SqlKind.CAST) {
+      return null;
+    }
     switch (Strong.policy(a.getKind())) {
     case NOT_NULL:
       return rexBuilder.makeLiteral(true);
@@ -748,12 +751,15 @@ public class RexSimplify {
   }
 
   private RexNode simplifyIsNull(RexNode a) {
-    if (!a.getType().isNullable()) {
+    if (!a.getType().isNullable() && isSafeExpression(a)) {
       return rexBuilder.makeLiteral(false);
     }
     if (RexUtil.isNull(a)) {
       return rexBuilder.makeLiteral(true);
     }
+    if (a.getKind() == SqlKind.CAST) {
+      return null;
+    }
     switch (Strong.policy(a.getKind())) {
     case NOT_NULL:
       return rexBuilder.makeLiteral(false);
@@ -996,6 +1002,7 @@ public class RexSimplify {
       safeOps.add(SqlKind.NOT);
       safeOps.add(SqlKind.CASE);
       safeOps.add(SqlKind.LIKE);
+      safeOps.add(SqlKind.COALESCE);
       this.safeOps = Sets.immutableEnumSet(safeOps);
     }
 
diff --git a/core/src/test/java/org/apache/calcite/test/RexProgramTest.java b/core/src/test/java/org/apache/calcite/test/RexProgramTest.java
index 813d7e0..8f12c2a 100644
--- a/core/src/test/java/org/apache/calcite/test/RexProgramTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RexProgramTest.java
@@ -1956,6 +1956,20 @@ public class RexProgramTest extends RexProgramBuilderBase {
     checkSimplify(isNotNull(lt(i0, null_)), "false");
   }
 
+  /** Unit test for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-2929">[CALCITE-2929]
+   * Simplification of IS NULL checks are incorrectly assuming that CAST-s are possible</a>. */
+  @Test public void testSimplifyCastIsNull() {
+    checkSimplifyUnchanged(isNull(cast(vVarchar(), tInt(true))));
+  }
+
+  /** Unit test for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-2929">[CALCITE-2929]
+   * Simplification of IS NULL checks are incorrectly assuming that CAST-s are possible</a>. */
+  @Test public void testSimplifyCastIsNull2() {
+    checkSimplifyUnchanged(isNull(cast(vVarcharNotNull(), tInt(false))));
+  }
+
   @Test public void checkSimplifyDynamicParam() {
     checkSimplify(isNotNull(lt(vInt(0), vInt(1))),
         "AND(IS NOT NULL(?0.int0), IS NOT NULL(?0.int1))");
diff --git a/core/src/test/resources/sql/sub-query.iq b/core/src/test/resources/sql/sub-query.iq
index e0533a9..77492d3 100644
--- a/core/src/test/resources/sql/sub-query.iq
+++ b/core/src/test/resources/sql/sub-query.iq
@@ -1856,11 +1856,11 @@ select sal from "scott".emp e
 (11 rows)
 
 !ok
-EnumerableCalc(expr#0..4=[{inputs}], expr#5=[NOT($t4)], expr#6=[IS NOT NULL($t4)], expr#7=[OR($t5, $t6)], expr#8=[IS NOT TRUE($t7)], SAL=[$t1], $condition=[$t8])
-  EnumerableJoin(condition=[=($2, $3)], joinType=[left])
+EnumerableCalc(expr#0..4=[{inputs}], expr#5=[NOT($t3)], expr#6=[IS NOT NULL($t3)], expr#7=[OR($t5, $t6)], expr#8=[IS NOT TRUE($t7)], SAL=[$t1], $condition=[$t8])
+  EnumerableJoin(condition=[=($2, $4)], joinType=[left])
     EnumerableCalc(expr#0..7=[{inputs}], EMPNO=[$t0], SAL=[$t5], DEPTNO=[$t7])
       EnumerableTableScan(table=[[scott, EMP]])
-    EnumerableCalc(expr#0..2=[{inputs}], expr#3=[true], expr#4=[10], expr#5=[=($t4, $t0)], DEPTNO=[$t0], $f1=[$t3], $condition=[$t5])
+    EnumerableCalc(expr#0..2=[{inputs}], expr#3=[true], expr#4=[10], expr#5=[=($t4, $t0)], cs=[$t3], DEPTNO=[$t0], $condition=[$t5])
       EnumerableTableScan(table=[[scott, DEPT]])
 !plan
 
diff --git a/spark/src/test/java/org/apache/calcite/test/SparkAdapterTest.java b/spark/src/test/java/org/apache/calcite/test/SparkAdapterTest.java
index cf40f34..000efef 100644
--- a/spark/src/test/java/org/apache/calcite/test/SparkAdapterTest.java
+++ b/spark/src/test/java/org/apache/calcite/test/SparkAdapterTest.java
@@ -576,7 +576,7 @@ public class SparkAdapterTest {
         + "where x is null";
 
     final String plan = "PLAN="
-        + "EnumerableCalc(expr#0..1=[{inputs}], expr#2=[false], proj#0..1=[{exprs}], $condition=[$t2])\n"
+        + "EnumerableCalc(expr#0..1=[{inputs}], expr#2=[IS NULL($t0)], proj#0..1=[{exprs}], $condition=[$t2])\n"
         + "  EnumerableValues(tuples=[[{ 1, 'a' }, { 2, 'b' }, { 1, 'b' }, { 2, 'c' }, { 2, 'c' }]])\n\n";
 
     final String expectedResult = "";