[CALCITE-1953] Rewrite "NOT (x IS FALSE)" to "x IS NOT FALSE"; "x IS TRUE" would be wrong
SqlKind.negate() != SqlKind.negateNullSafe() Close apache/calcite#521 Project: http://git-wip-us.apache.org/repos/asf/calcite/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/2156d826 Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/2156d826 Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/2156d826 Branch: refs/heads/branch-1.14 Commit: 2156d8265178f92c52f94ef47588a89ab1fdb7de Parents: d3a7c0d Author: MinJi Kim <[email protected]> Authored: Sat Oct 8 10:30:58 2016 -0700 Committer: Julian Hyde <[email protected]> Committed: Tue Aug 29 10:12:28 2017 -0700 ---------------------------------------------------------------------- .../org/apache/calcite/rex/RexSimplify.java | 2 +- .../java/org/apache/calcite/sql/SqlKind.java | 43 +++++++++++++++++--- .../apache/calcite/test/RelOptRulesTest.java | 15 +++++++ .../org/apache/calcite/test/RexProgramTest.java | 27 ++++++++++++ .../org/apache/calcite/test/RelOptRulesTest.xml | 25 ++++++++++++ .../calcite/test/SqlToRelConverterTest.xml | 2 +- 6 files changed, 106 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite/blob/2156d826/core/src/main/java/org/apache/calcite/rex/RexSimplify.java ---------------------------------------------------------------------- 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 34f01a0..67ac1b8 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java +++ b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java @@ -325,7 +325,7 @@ public class RexSimplify { // Note that // (NOT x) IS TRUE !=> x IS FALSE // because of null values. - final SqlOperator notKind = RexUtil.op(kind.negate()); + final SqlOperator notKind = RexUtil.op(kind.negateNullSafe()); final RexNode arg = ((RexCall) a).operands.get(0); return simplify(rexBuilder.makeCall(notKind, arg)); } http://git-wip-us.apache.org/repos/asf/calcite/blob/2156d826/core/src/main/java/org/apache/calcite/sql/SqlKind.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/sql/SqlKind.java b/core/src/main/java/org/apache/calcite/sql/SqlKind.java index 62dedb6..ad7c4e2 100644 --- a/core/src/main/java/org/apache/calcite/sql/SqlKind.java +++ b/core/src/main/java/org/apache/calcite/sql/SqlKind.java @@ -1170,19 +1170,31 @@ public enum SqlKind { /** Returns the kind that you get if you apply NOT to this kind. * - * <p>For example, {@code IS_NOT_NULL.negate()} returns {@link #IS_NULL}. */ + * <p>For example, {@code IS_NOT_NULL.negate()} returns {@link #IS_NULL}. + * + * <p>For {@link #IS_TRUE}, {@link #IS_FALSE}, {@link #IS_NOT_TRUE}, + * {@link #IS_NOT_FALSE}, nullable inputs need to be treated carefully. + * + * <p>{@code NOT(IS_TRUE(null))} = {@code NOT(false)} = {@code true}, + * while {@code IS_FALSE(null)} = {@code false}, + * so {@code NOT(IS_TRUE(X))} should be {@code IS_NOT_TRUE(X)}. + * On the other hand, + * {@code IS_TRUE(NOT(null))} = {@code IS_TRUE(null)} = {@code false}. + * + * <p>This is why negate() != negateNullSafe() for these operators. + */ public SqlKind negate() { switch (this) { case IS_TRUE: - return IS_FALSE; + return IS_NOT_TRUE; case IS_FALSE: - return IS_TRUE; + return IS_NOT_FALSE; case IS_NULL: return IS_NOT_NULL; case IS_NOT_TRUE: - return IS_NOT_FALSE; + return IS_TRUE; case IS_NOT_FALSE: - return IS_NOT_TRUE; + return IS_FALSE; case IS_NOT_NULL: return IS_NULL; case IS_DISTINCT_FROM: @@ -1195,7 +1207,18 @@ public enum SqlKind { } /** Returns the kind that you get if you negate this kind. - * To conform to null semantics, null value should not be compared. */ + * To conform to null semantics, null value should not be compared. + * + * <p>For {@link #IS_TRUE}, {@link #IS_FALSE}, {@link #IS_NOT_TRUE} and + * {@link #IS_NOT_FALSE}, nullable inputs need to be treated carefully: + * + * <ul> + * <li>NOT(IS_TRUE(null)) = NOT(false) = true + * <li>IS_TRUE(NOT(null)) = IS_TRUE(null) = false + * <li>IS_FALSE(null) = false + * <li>IS_NOT_TRUE(null) = true + * </ul> + */ public SqlKind negateNullSafe() { switch (this) { case EQUALS: @@ -1210,6 +1233,14 @@ public enum SqlKind { return GREATER_THAN; case GREATER_THAN_OR_EQUAL: return LESS_THAN; + case IS_TRUE: + return IS_FALSE; + case IS_FALSE: + return IS_TRUE; + case IS_NOT_TRUE: + return IS_NOT_FALSE; + case IS_NOT_FALSE: + return IS_NOT_TRUE; default: return this.negate(); } http://git-wip-us.apache.org/repos/asf/calcite/blob/2156d826/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 f43f624..d9aea0d 100644 --- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java @@ -161,6 +161,21 @@ public class RelOptRulesTest extends RelOptTestBase { return DiffRepository.lookup(RelOptRulesTest.class); } + @Test public void testReduceNot() { + HepProgram preProgram = new HepProgramBuilder() + .build(); + + HepProgramBuilder builder = new HepProgramBuilder(); + builder.addRuleClass(ReduceExpressionsRule.class); + HepPlanner hepPlanner = new HepPlanner(builder.build()); + hepPlanner.addRule(ReduceExpressionsRule.FILTER_INSTANCE); + + final String sql = "select *\n" + + "from (select (case when sal > 1000 then null else false end) as caseCol from emp)\n" + + "where NOT(caseCol)"; + checkPlanning(tester, preProgram, hepPlanner, sql, true); + } + @Test public void testReduceNestedCaseWhen() { HepProgram preProgram = new HepProgramBuilder() .build(); http://git-wip-us.apache.org/repos/asf/calcite/blob/2156d826/core/src/test/java/org/apache/calcite/test/RexProgramTest.java ---------------------------------------------------------------------- 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 ca9e04f..7cbcfa6 100644 --- a/core/src/test/java/org/apache/calcite/test/RexProgramTest.java +++ b/core/src/test/java/org/apache/calcite/test/RexProgramTest.java @@ -1640,6 +1640,33 @@ public class RexProgramTest { } return map2.toString(); } + + @Test public void testSimplifyNot() { + final RelDataType booleanNullableType = + typeFactory.createTypeWithNullability( + typeFactory.createSqlType(SqlTypeName.BOOLEAN), true); + final RexNode booleanInput = rexBuilder.makeInputRef(booleanNullableType, 0); + final RexNode isFalse = rexBuilder.makeCall(SqlStdOperatorTable.IS_FALSE, booleanInput); + final RexCall result = (RexCall) simplify(isFalse); + assertThat(result.getType().isNullable(), is(false)); + assertThat(result.getOperator(), is((SqlOperator) SqlStdOperatorTable.IS_FALSE)); + assertThat(result.getOperands().size(), is(1)); + assertThat(result.getOperands().get(0), is(booleanInput)); + + // Make sure that IS_FALSE(IS_FALSE(nullable boolean)) != IS_TRUE(nullable boolean) + // IS_FALSE(IS_FALSE(null)) = IS_FALSE(false) = true + // IS_TRUE(null) = false + final RexNode isFalseIsFalse = rexBuilder.makeCall(SqlStdOperatorTable.IS_FALSE, isFalse); + final RexCall result2 = (RexCall) simplify(isFalseIsFalse); + assertThat(result2.getType().isNullable(), is(false)); + assertThat(result2.getOperator(), is((SqlOperator) SqlStdOperatorTable.IS_NOT_FALSE)); + assertThat(result2.getOperands().size(), is(1)); + assertThat(result2.getOperands().get(0), is(booleanInput)); + } + + private RexNode simplify(RexNode e) { + return new RexSimplify(rexBuilder, false, RexUtil.EXECUTOR).simplify(e); + } } // End RexProgramTest.java http://git-wip-us.apache.org/repos/asf/calcite/blob/2156d826/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 28c3b9d..2da30be 100644 --- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml @@ -16,6 +16,31 @@ See the License for the specific language governing permissions and limitations under the License. --> <Root> + <TestCase name="testReduceNot"> + <Resource name="sql"> + <![CDATA[select sal +from emp +where case when (sal = 1000) then +(case when sal = 1000 then null else 1 end is null) else +(case when sal = 2000 then null else 1 end is null) end is true]]> + </Resource> + <Resource name="planAfter"> + <![CDATA[ +LogicalProject(CASECOL=[$0]) + LogicalFilter(condition=[NOT($0)]) + LogicalProject(CASECOL=[CASE(>($5, 1000), null, false)]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + </Resource> + <Resource name="planBefore"> + <![CDATA[ +LogicalProject(CASECOL=[$0]) + LogicalFilter(condition=[NOT($0)]) + LogicalProject(CASECOL=[CASE(>($5, 1000), null, false)]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + </Resource> + </TestCase> <TestCase name="testReduceNestedCaseWhen"> <Resource name="sql"> <![CDATA[select sal http://git-wip-us.apache.org/repos/asf/calcite/blob/2156d826/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 f96583d..2e8f904 100644 --- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml @@ -2001,7 +2001,7 @@ from emp]]> </Resource> <Resource name="plan"> <![CDATA[ -LogicalProject(EMPNO=[$0], EXPR$1=[IS FALSE($11)]) +LogicalProject(EMPNO=[$0], EXPR$1=[IS NOT TRUE($11)]) LogicalJoin(condition=[=($9, $10)], joinType=[left]) LogicalProject($f0=[$0], $f1=[$1], $f2=[$2], $f3=[$3], $f4=[$4], $f5=[$5], $f6=[$6], $f7=[$7], $f8=[$8], $f9=[$7]) LogicalTableScan(table=[[CATALOG, SALES, EMP]])
