[CALCITE-3675] SQL to Rel conversion is broken for coalesce on nullab…

…le field (DonnyZone)
sirensolutions · Jan 13, 2020 · 536d503 · 536d503
1 parent e387ded
commit 536d503
Show file tree

Hide file tree

Showing 6 changed files with 58 additions and 9 deletions.
diff --git a/core/src/main/java/org/apache/calcite/rex/RexCallBinding.java b/core/src/main/java/org/apache/calcite/rex/RexCallBinding.java
@@ -122,6 +122,10 @@ public static RexCallBinding create(RelDataTypeFactory typeFactory,
     return RexUtil.isLiteral(operands.get(ordinal), allowCast);
   }
 
+  public List<RexNode> operands() {
+    return operands;
+  }
+
   // implement SqlOperatorBinding
   public int getOperandCount() {
     return operands.size();

diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlCaseOperator.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlCaseOperator.java
@@ -18,6 +18,9 @@
 
 import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexCallBinding;
+import org.apache.calcite.rex.RexNode;
 import org.apache.calcite.sql.SqlBasicCall;
 import org.apache.calcite.sql.SqlCall;
 import org.apache.calcite.sql.SqlCallBinding;
@@ -216,9 +219,7 @@ public RelDataType inferReturnType(
       SqlOperatorBinding opBinding) {
     // REVIEW jvs 4-June-2005:  can't these be unified?
     if (!(opBinding instanceof SqlCallBinding)) {
-      return inferTypeFromOperands(
-          opBinding.getTypeFactory(),
-          opBinding.collectOperandTypes());
+      return inferTypeFromOperands(opBinding);
     }
     return inferTypeFromValidator((SqlCallBinding) opBinding);
   }
@@ -290,16 +291,29 @@ private RelDataType inferTypeFromValidator(
     return ret;
   }
 
-  private RelDataType inferTypeFromOperands(
-      RelDataTypeFactory typeFactory,
-      List<RelDataType> argTypes) {
+  private RelDataType inferTypeFromOperands(SqlOperatorBinding opBinding) {
+    final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+    final List<RelDataType> argTypes = opBinding.collectOperandTypes();
     assert (argTypes.size() % 2) == 1 : "odd number of arguments expected: "
         + argTypes.size();
     assert argTypes.size() > 1 : "CASE must have more than 1 argument. Given "
       + argTypes.size() + ", " + argTypes;
     List<RelDataType> thenTypes = new ArrayList<>();
     for (int j = 1; j < (argTypes.size() - 1); j += 2) {
-      thenTypes.add(argTypes.get(j));
+      RelDataType argType = argTypes.get(j);
+      if (opBinding instanceof RexCallBinding) {
+        final RexCallBinding rexCallBinding = (RexCallBinding) opBinding;
+        final RexNode whenNode = rexCallBinding.operands().get(j - 1);
+        final RexNode thenNode = rexCallBinding.operands().get(j);
+        if (whenNode.getKind() == SqlKind.IS_NOT_NULL && argType.isNullable()) {
+          // Type is not nullable if the kind is IS NOT NULL.
+          final RexCall isNotNullCall = (RexCall) whenNode;
+          if (isNotNullCall.getOperands().get(0).equals(thenNode)) {
+            argType = typeFactory.createTypeWithNullability(argType, false);
+          }
+        }
+      }
+      thenTypes.add(argType);
     }
 
     thenTypes.add(Iterables.getLast(argTypes));

diff --git a/core/src/test/java/org/apache/calcite/test/JdbcTest.java b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
@@ -3270,6 +3270,21 @@ public void checkOrderBy(final boolean desc,
             "deptno=20; C=1; S=8000.0");
   }
 
+  @Test public void testCaseWhenOnNullableField() {
+    CalciteAssert.hr()
+        .query("select case when \"commission\" is not null "
+            + "then \"commission\" else 100 end\n"
+            + "from \"hr\".\"emps\"\n")
+        .explainContains("PLAN=EnumerableCalc(expr#0..4=[{inputs}],"
+            + " expr#5=[IS NOT NULL($t4)], expr#6=[CAST($t4):INTEGER NOT NULL],"
+            + " expr#7=[100], expr#8=[CASE($t5, $t6, $t7)], EXPR$0=[$t8])\n"
+            + "  EnumerableTableScan(table=[[hr, emps]])")
+        .returns("EXPR$0=1000\n"
+            + "EXPR$0=500\n"
+            + "EXPR$0=100\n"
+            + "EXPR$0=250\n");
+  }
+
   @Test public void testSelectDistinct() {
     CalciteAssert.hr()
         .query("select distinct \"deptno\"\n"

diff --git a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
@@ -3645,6 +3645,11 @@ private Tester getExtendedTester() {
     sql(sql).ok();
   }
 
+  @Test public void testCoalesceOnNullableField() {
+    final String sql = "select coalesce(mgr, 0) from emp";
+    sql(sql).ok();
+  }
+
   /**
    * Visitor that checks that every {@link RelNode} in a tree is valid.
    *

diff --git a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -8196,7 +8196,7 @@ where coalesce(e1.mgr, -1) = coalesce(e2.mgr, -1)]]>
         <Resource name="planBefore">
             <![CDATA[
 LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8], EMPNO0=[$9], ENAME0=[$10], JOB0=[$11], MGR0=[$12], HIREDATE0=[$13], SAL0=[$14], COMM0=[$15], DEPTNO0=[$16], SLACKER0=[$17])
-  LogicalFilter(condition=[=(CASE(IS NOT NULL($3), $3, -1), CASE(IS NOT NULL($12), $12, -1))])
+  LogicalFilter(condition=[=(CASE(IS NOT NULL($3), CAST($3):INTEGER NOT NULL, -1), CASE(IS NOT NULL($12), CAST($12):INTEGER NOT NULL, -1))])
     LogicalJoin(condition=[true], joinType=[inner])
       LogicalTableScan(table=[[CATALOG, SALES, EMP]])
       LogicalTableScan(table=[[CATALOG, SALES, EMP]])
@@ -8205,7 +8205,7 @@ LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$
         <Resource name="planAfter">
             <![CDATA[
 LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8], EMPNO0=[$9], ENAME0=[$10], JOB0=[$11], MGR0=[$12], HIREDATE0=[$13], SAL0=[$14], COMM0=[$15], DEPTNO0=[$16], SLACKER0=[$17])
-  LogicalJoin(condition=[=(CASE(IS NOT NULL($3), $3, -1), CASE(IS NOT NULL($12), $12, -1))], joinType=[inner])
+  LogicalJoin(condition=[=(CASE(IS NOT NULL($3), CAST($3):INTEGER NOT NULL, -1), CASE(IS NOT NULL($12), CAST($12):INTEGER NOT NULL, -1))], joinType=[inner])
     LogicalTableScan(table=[[CATALOG, SALES, EMP]])
     LogicalTableScan(table=[[CATALOG, SALES, EMP]])
 ]]>

diff --git a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
@@ -6471,6 +6471,17 @@ LogicalProject(EMPNO=[$0], DEPTNO=[$1], DEPTNO0=[$3], NAME=[$4])
     LogicalProject(EMPNO=[$0], DEPTNO=[$7], $f2=[+($7, 20)])
       LogicalTableScan(table=[[CATALOG, SALES, EMP]])
     LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
+]]>
+        </Resource>
+    </TestCase>
+    <TestCase name="testCoalesceOnNullableField">
+        <Resource name="sql">
+            <![CDATA[select coalesce(mgr, 0) from emp]]>
+        </Resource>
+        <Resource name="plan">
+            <![CDATA[
+LogicalProject(EXPR$0=[CASE(IS NOT NULL($3), CAST($3):INTEGER NOT NULL, 0)])
+  LogicalTableScan(table=[[CATALOG, SALES, EMP]])
 ]]>
         </Resource>
     </TestCase>