From ab3f5b0e800b2166f130fa75fc3a67f6961b5082 Mon Sep 17 00:00:00 2001 From: macroguo Date: Sun, 10 Sep 2023 16:26:27 +0800 Subject: [PATCH] [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE' Simplify 'p OR NOT p' to 'TRUE' (if p is not nullable), or to 'null AND p IS NOT NULL' (if p is nullable). GeodeFilter support converting 'p IS NOT NULL' to 'p <> null'. --- .../org/apache/calcite/rex/RexSimplify.java | 20 +++++++++ .../apache/calcite/rex/RexProgramTest.java | 44 ++++++++++++++++++- .../adapter/geode/rel/GeodeFilter.java | 3 ++ .../calcite/adapter/geode/rel/GeodeRules.java | 6 +-- .../geode/rel/GeodeAllDataTypesTest.java | 10 +++++ 5 files changed, 78 insertions(+), 5 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 d3c1f93aa53..312e61b1dcf 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java +++ b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java @@ -2060,6 +2060,26 @@ private RexNode simplifyOrs(List terms, RexUnknownAs unknownAs) { } } break; + case IS_NOT_TRUE: + RexNode arg = ((RexCall) term).getOperands().get(0); + if (isSafeExpression(arg) && terms.contains(arg)) { + return trueLiteral; + } + break; + case NOT: + RexNode x = ((RexCall) term).getOperands().get(0); + if (isSafeExpression(x) && terms.contains(x)) { + if (!x.getType().isNullable()) { + return trueLiteral; + } + + final RexNode isNotNull = + rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, x); + terms.set(terms.indexOf(x), simplifyIs((RexCall) isNotNull, unknownAs)); + terms.set(i, rexBuilder.makeNullLiteral(x.getType())); + i--; + } + break; default: break; } diff --git a/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java b/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java index e4775935ef7..ba7e4f18bf5 100644 --- a/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java +++ b/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java @@ -2318,7 +2318,7 @@ trueLiteral, literal(2), isTrue(vBool()), literal(1), isNotTrue(vBool()), literal(1), literal(2)), - "CASE(OR(?0.bool0, IS NOT TRUE(?0.bool0)), 1, 2)"); + "1"); } @Test void testSimplifyCaseBranchesCollapse2() { @@ -3304,6 +3304,48 @@ private static String getString(Map map) { "IS NULL(?0.int1)"); } + /** Unit test for + * [CALCITE-4189] + * Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'. */ + @Test public void testSimplifyOrNot2() { + final SqlOperator func = + SqlBasicFunction.create("func", ReturnTypes.BOOLEAN_NULLABLE, OperandTypes.VARIADIC); + final RexNode unsafeRel = rexBuilder.makeCall(func, div(vInt(0), literal(2))); + // x OR x IS NOT TRUE ==> "true" + checkSimplify(or(vBool(), isNotTrue(vBool())), "true"); + checkSimplify(or(vBoolNotNull(), isNotTrue(vBoolNotNull())), "true"); + // outside unsafe expression will not prevent simplification + checkSimplify(or(unsafeRel, vBool(), isNotTrue(vBool())), "true"); + + // x OR NOT x ==> "true" (if x is not nullable) + checkSimplify(or(vBoolNotNull(), not(vBoolNotNull())), "true"); + checkSimplify(or(unsafeRel, vBoolNotNull(), not(vBoolNotNull())), "true"); + + // x OR NOT x ==> x IS NOT NULL OR NULL (if x is nullable) + checkSimplify3(or(vBool(), not(vBool())), + "OR(IS NOT NULL(?0.bool0), null)", + "IS NOT NULL(?0.bool0)", + "true"); + checkSimplify3(or(unsafeRel, vBool(), not(vBool())), + "OR(func(/(?0.int0, 2)), IS NOT NULL(?0.bool0), null)", + "OR(func(/(?0.int0, 2)), IS NOT NULL(?0.bool0))", + "true"); + // remove all redundant NULL + checkSimplify3(or(vBool(0), not(vBool(0)), vBool(1), not(vBool(1))), + "OR(IS NOT NULL(?0.bool0), null, IS NOT NULL(?0.bool1))", + "OR(IS NOT NULL(?0.bool0), IS NOT NULL(?0.bool1))", + "true"); + + // unsafe expression can not be simplified + checkSimplify3(or(unsafeRel, isNotTrue(unsafeRel)), + "OR(func(/(?0.int0, 2)), IS NOT TRUE(func(/(?0.int0, 2))))", + "OR(func(/(?0.int0, 2)), IS NOT TRUE(func(/(?0.int0, 2))))", + "OR(func(/(?0.int0, 2)), NOT(func(/(?0.int0, 2))))"); + checkSimplifyUnchanged(or(unsafeRel, not(unsafeRel))); + + checkSimplify(or(unsafeRel, trueLiteral), "true"); + } + private void checkSarg(String message, Sarg sarg, Matcher complexityMatcher, Matcher stringMatcher) { assertThat(message, sarg.complexity(), complexityMatcher); diff --git a/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeFilter.java b/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeFilter.java index afdc6e468b7..0a401fdfc06 100644 --- a/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeFilter.java +++ b/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeFilter.java @@ -320,6 +320,9 @@ private String translateMatch2(RexNode node) { return translateBinary(">=", "<=", (RexCall) node); case INPUT_REF: return translateBinary2("=", node, rexBuilder.makeLiteral(true)); + case IS_NOT_NULL: + child = ((RexCall) node).getOperands().get(0); + return translateBinary2("<>", child, rexBuilder.makeNullLiteral(node.getType())); case NOT: child = ((RexCall) node).getOperands().get(0); if (child.getKind() == SqlKind.CAST) { diff --git a/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeRules.java b/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeRules.java index 5fa2b563402..b407d368088 100644 --- a/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeRules.java +++ b/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeRules.java @@ -41,6 +41,7 @@ import org.apache.calcite.sql.validate.SqlValidatorUtil; import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; import org.immutables.value.Value; @@ -314,10 +315,7 @@ private static boolean isEqualityOnKey(RexNode node, List fieldNames) { private static boolean isBooleanColumnReference(RexNode node, List fieldNames) { // FIXME Ignore casts for rel and assume they aren't really necessary - if (node.isA(SqlKind.CAST)) { - node = ((RexCall) node).getOperands().get(0); - } - if (node.isA(SqlKind.NOT)) { + while (node.isA(ImmutableList.of(SqlKind.NOT, SqlKind.CAST, SqlKind.IS_NOT_NULL))) { node = ((RexCall) node).getOperands().get(0); } if (node.isA(SqlKind.INPUT_REF)) { diff --git a/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeAllDataTypesTest.java b/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeAllDataTypesTest.java index edbce076cca..dafec6efac0 100644 --- a/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeAllDataTypesTest.java +++ b/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeAllDataTypesTest.java @@ -110,6 +110,16 @@ private CalciteAssert.AssertThat calciteAssert() { + "WHERE booleanValue = true")); } + @Test void testSqlBooleanColumnIsNotNullFilter() { + calciteAssert() + .query("SELECT booleanValue as booleanValue " + + "FROM geode.allDataTypesRegion WHERE booleanValue is not null") + .returnsCount(3) + .queryContains( + GeodeAssertions.query("SELECT booleanValue AS booleanValue FROM /allDataTypesRegion " + + "WHERE booleanValue <> null")); + } + @Test void testSqlBooleanColumnFilter() { calciteAssert() .query("SELECT booleanValue as booleanValue "