[CALCITE-2840] RexNode simplification logic should use more specific …

…UnknownAs modes

Close apache#1045

core/src/main/java/org/apache/calcite/rex/RexSimplify.java

@@ -411,14 +411,14 @@ private void simplifyList(List<RexNode> terms, RexUnknownAs unknownAs) {
     }
   }
 
-  private void simplifyAndTerms(List<RexNode> terms) {
+  private void simplifyAndTerms(List<RexNode> terms, RexUnknownAs unknownAs) {
     RexSimplify simplify = this;
     for (int i = 0; i < terms.size(); i++) {
       RexNode t = terms.get(i);
       if (Predicate.of(t) == null) {
         continue;
       }
-      terms.set(i, simplify.simplify(t, UNKNOWN));
+      terms.set(i, simplify.simplify(t, unknownAs));
       RelOptPredicateList newPredicates = simplify.predicates.union(rexBuilder,
           RelOptPredicateList.of(rexBuilder, terms.subList(i, i + 1)));
       simplify = simplify.withPredicates(newPredicates);
@@ -428,11 +428,11 @@ private void simplifyAndTerms(List<RexNode> terms) {
       if (Predicate.of(t) != null) {
         continue;
       }
-      terms.set(i, simplify.simplify(t, UNKNOWN));
+      terms.set(i, simplify.simplify(t, unknownAs));
     }
   }
 
-  private void simplifyOrTerms(List<RexNode> terms) {
+  private void simplifyOrTerms(List<RexNode> terms, RexUnknownAs unknownAs) {
     // Suppose we are processing "e1(x) OR e2(x) OR e3(x)". When we are
     // visiting "e3(x)" we know both "e1(x)" and "e2(x)" are not true (they
     // may be unknown), because if either of them were true we would have
@@ -443,7 +443,7 @@ private void simplifyOrTerms(List<RexNode> terms) {
       if (Predicate.of(t) == null) {
         continue;
       }
-      final RexNode t2 = simplify.simplify(t, RexUnknownAs.UNKNOWN);
+      final RexNode t2 = simplify.simplify(t, unknownAs);
       terms.set(i, t2);
       final RexNode inverse =
           simplify.simplify(rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_TRUE, t2),
@@ -457,7 +457,7 @@ private void simplifyOrTerms(List<RexNode> terms) {
       if (Predicate.of(t) != null) {
         continue;
       }
-      terms.set(i, simplify.simplify(t, RexUnknownAs.UNKNOWN));
+      terms.set(i, simplify.simplify(t, unknownAs));
     }
   }
 
@@ -1044,12 +1044,12 @@ RexNode simplifyAnd(RexCall e, RexUnknownAs unknownAs) {
     RelOptUtil.decomposeConjunction(e, terms, notTerms);
 
     if (unknownAs == FALSE && predicateElimination) {
-      simplifyAndTerms(terms);
+      simplifyAndTerms(terms, FALSE);
     } else {
       simplifyList(terms, unknownAs);
     }
 
-    simplifyList(notTerms, UNKNOWN); // TODO could be unknownAs.negate()?
+    simplifyList(notTerms, unknownAs.negate());
 
     switch (unknownAs) {
     case FALSE:
@@ -1447,7 +1447,7 @@ private RexNode simplifyOr(RexCall call, RexUnknownAs unknownAs) {
     assert call.getKind() == SqlKind.OR;
     final List<RexNode> terms = RelOptUtil.disjunctions(call);
     if (predicateElimination) {
-      simplifyOrTerms(terms);
+      simplifyOrTerms(terms, unknownAs);
     }
     return simplifyOrs(terms, unknownAs);
   }

core/src/main/java/org/apache/calcite/rex/RexUnknownAs.java

@@ -94,6 +94,17 @@ public boolean toBoolean() {
       throw new IllegalArgumentException("unknown");
     }
   }
+
+  public RexUnknownAs negate() {
+    switch (this) {
+    case TRUE:
+      return FALSE;
+    case FALSE:
+      return TRUE;
+    default:
+      return UNKNOWN;
+    }
+  }
 }
 
 // End RexUnknownAs.java

core/src/test/java/org/apache/calcite/test/RexProgramTest.java

@@ -1719,22 +1719,33 @@ private void checkExponentialCnf(int n) {
   }
 
   @Test public void testSimplifyAnd3() {
-    final RelDataType boolType = typeFactory.createSqlType(SqlTypeName.BOOLEAN);
-    final RelDataType rowType = typeFactory.builder()
-        .add("a", boolType).nullable(true)
-        .build();
-
-    final RexDynamicParam range = rexBuilder.makeDynamicParam(rowType, 0);
-    final RexNode aRef = rexBuilder.makeFieldAccess(range, 0);
-
     // in the case of 3-valued logic, the result must be unknown if a is unknown
     checkSimplify2(
-        and(aRef,
-            not(aRef)),
-        "AND(null, IS NULL(?0.a))",
+        and(vBool(), not(vBool())),
+        "AND(null, IS NULL(?0.bool0))",
         "false");
   }
 
+  /** Unit test for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-2840">[CALCITE-2840]
+   * Simplification should use more specific UnknownAs modes during simplification</a>. */
+  @Test public void testNestedAndSimplification() {
+    // to have the correct mode for the AND at the bottom,
+    // both the OR and AND parent should retain the UnknownAs mode
+    checkSimplify2(
+        and(
+            eq(vInt(2), literal(2)),
+            or(
+                eq(vInt(3), literal(3)),
+                and(
+                    ge(vInt(), literal(1)),
+                    le(vInt(), literal(1))))),
+        "AND(=(?0.int2, 2), OR(=(?0.int3, 3), AND(>=(?0.int0, 1), <=(?0.int0, 1))))",
+        "AND(=(?0.int2, 2), OR(=(?0.int3, 3), =(?0.int0, 1)))"
+
+    );
+  }
+
   @Test public void fieldAccessEqualsHashCode() {
     assertEquals("vBool() instances should be equal", vBool(), vBool());
     assertEquals("vBool().hashCode()", vBool().hashCode(), vBool().hashCode());

core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml

@@ -8818,7 +8818,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])
-  LogicalFilter(condition=[AND(>($3, 0), CASE(>($3, 0), >(/($7, $3), 1), null:BOOLEAN))])
+  LogicalFilter(condition=[AND(>($3, 0), CASE(>($3, 0), >(/($7, $3), 1), false))])
     LogicalTableScan(table=[[CATALOG, SALES, EMP]])
 ]]>
         </Resource>