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

zabetak 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 233d9dd  [CALCITE-2582] FilterProjectTransposeRule does not always 
simplify the new filter condition
233d9dd is described below

commit 233d9dd5671477ce06d777cd5ef3ee56df4664f8
Author: Stamatis Zampetakis <zabe...@gmail.com>
AuthorDate: Thu Feb 21 09:56:22 2019 +0100

    [CALCITE-2582] FilterProjectTransposeRule does not always simplify the new 
filter condition
    
    1. Simplify newCondition in a similar manner to RelBuilder#filter method.
    
    2. Update tests in misc.iq, sub-query.iq, GeodeZipsTest, 
GeodeAllDataTypesTest to reflect the applied simplifications.
---
 .../rel/rules/FilterProjectTransposeRule.java      | 30 ++++++++++++++++++++--
 core/src/test/resources/sql/misc.iq                |  2 +-
 core/src/test/resources/sql/sub-query.iq           | 16 ++++++------
 .../adapter/geode/rel/GeodeAllDataTypesTest.java   |  4 +--
 .../calcite/adapter/geode/rel/GeodeZipsTest.java   |  2 +-
 5 files changed, 40 insertions(+), 14 deletions(-)

diff --git 
a/core/src/main/java/org/apache/calcite/rel/rules/FilterProjectTransposeRule.java
 
b/core/src/main/java/org/apache/calcite/rel/rules/FilterProjectTransposeRule.java
index ac0ad83..c9b0fd3 100644
--- 
a/core/src/main/java/org/apache/calcite/rel/rules/FilterProjectTransposeRule.java
+++ 
b/core/src/main/java/org/apache/calcite/rel/rules/FilterProjectTransposeRule.java
@@ -16,6 +16,7 @@
  */
 package org.apache.calcite.rel.rules;
 
+import org.apache.calcite.plan.RelOptPredicateList;
 import org.apache.calcite.plan.RelOptRule;
 import org.apache.calcite.plan.RelOptRuleCall;
 import org.apache.calcite.plan.RelOptRuleOperand;
@@ -24,11 +25,15 @@ import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.core.Filter;
 import org.apache.calcite.rel.core.Project;
 import org.apache.calcite.rel.core.RelFactories;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexExecutor;
 import org.apache.calcite.rex.RexNode;
 import org.apache.calcite.rex.RexOver;
+import org.apache.calcite.rex.RexSimplify;
 import org.apache.calcite.rex.RexUtil;
 import org.apache.calcite.tools.RelBuilder;
 import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.Util;
 
 import java.util.function.Predicate;
 
@@ -151,8 +156,7 @@ public class FilterProjectTransposeRule extends RelOptRule {
     RelNode newFilterRel;
     if (copyFilter) {
       newFilterRel = filter.copy(filter.getTraitSet(), project.getInput(),
-          RexUtil.removeNullabilityCast(relBuilder.getTypeFactory(),
-              newCondition));
+          simplifyFilterCondition(newCondition, call));
     } else {
       newFilterRel =
           relBuilder.push(project.getInput()).filter(newCondition).build();
@@ -168,6 +172,28 @@ public class FilterProjectTransposeRule extends RelOptRule 
{
 
     call.transformTo(newProjRel);
   }
+
+  /**
+   * Simplifies the filter condition using a simplifier created by the 
information in the
+   * current call.
+   *
+   * The method is an attempt to replicate the simplification behavior of
+   * {@link RelBuilder#filter(RexNode...)} which cannot be used in the case of 
copying nodes. The
+   * main difference with the behavior of that method is that it does not drop 
entirely the filter
+   * if the condition is always false.
+   */
+  private RexNode simplifyFilterCondition(RexNode condition, RelOptRuleCall 
call) {
+    final RexBuilder xBuilder = call.builder().getRexBuilder();
+    final RexExecutor executor =
+        Util.first(call.getPlanner().getContext().unwrap(RexExecutor.class),
+            Util.first(call.getPlanner().getExecutor(), RexUtil.EXECUTOR));
+    // unknownAsFalse => true since in the WHERE clause:
+    // 1>null evaluates to unknown and WHERE unknown behaves exactly like 
WHERE false
+    RexSimplify simplifier =
+        new RexSimplify(xBuilder, RelOptPredicateList.EMPTY, executor);
+    return RexUtil.removeNullabilityCast(
+        xBuilder.getTypeFactory(), 
simplifier.simplifyUnknownAsFalse(condition));
+  }
 }
 
 // End FilterProjectTransposeRule.java
diff --git a/core/src/test/resources/sql/misc.iq 
b/core/src/test/resources/sql/misc.iq
index 1f3ed72..aada308 100644
--- a/core/src/test/resources/sql/misc.iq
+++ b/core/src/test/resources/sql/misc.iq
@@ -473,7 +473,7 @@ EnumerableCalc(expr#0..7=[{inputs}], expr#8=[IS NULL($t5)], 
expr#9=[IS NULL($t7)
           EnumerableCalc(expr#0..3=[{inputs}], expr#4=[true], deptno=[$t0], 
$f0=[$t4])
             EnumerableTableScan(table=[[hr, depts]])
     EnumerableAggregate(group=[{0}], agg#0=[MIN($1)])
-      EnumerableCalc(expr#0..3=[{inputs}], expr#4=[90], expr#5=[+($t0, $t4)], 
expr#6=[true], $f4=[$t5], $f0=[$t6], $condition=[$t6])
+      EnumerableCalc(expr#0..3=[{inputs}], expr#4=[90], expr#5=[+($t0, $t4)], 
expr#6=[true], $f4=[$t5], $f0=[$t6])
         EnumerableTableScan(table=[[hr, depts]])
 !plan
 
diff --git a/core/src/test/resources/sql/sub-query.iq 
b/core/src/test/resources/sql/sub-query.iq
index 9b8c179..e75a690 100644
--- a/core/src/test/resources/sql/sub-query.iq
+++ b/core/src/test/resources/sql/sub-query.iq
@@ -624,7 +624,7 @@ where exists (
 !ok
 EnumerableSemiJoin(condition=[=($0, $10)], joinType=[inner])
   EnumerableTableScan(table=[[scott, DEPT]])
-  EnumerableCalc(expr#0..7=[{inputs}], expr#8=[=($t7, $t7)], 
expr#9=['SMITH':VARCHAR(10)], expr#10=[=($t1, $t9)], expr#11=[AND($t8, $t10)], 
proj#0..7=[{exprs}], $condition=[$t11])
+  EnumerableCalc(expr#0..7=[{inputs}], expr#8=['SMITH':VARCHAR(10)], 
expr#9=[=($t1, $t8)], expr#10=[IS NOT NULL($t7)], expr#11=[AND($t9, $t10)], 
proj#0..7=[{exprs}], $condition=[$t11])
     EnumerableTableScan(table=[[scott, EMP]])
 !plan
 
@@ -831,7 +831,7 @@ EnumerableCalc(expr#0..3=[{inputs}], expr#4=[false], 
expr#5=[=($t2, $t4)], expr#
     EnumerableLimit(fetch=[1])
       EnumerableSort(sort0=[$0], dir0=[DESC])
         EnumerableAggregate(group=[{0}], c=[COUNT()])
-          EnumerableCalc(expr#0..2=[{inputs}], expr#3=[false], expr#4=[123], 
expr#5=[null:INTEGER], expr#6=[=($t4, $t5)], expr#7=[IS NULL($t5)], 
expr#8=[OR($t6, $t7)], cs=[$t3], $condition=[$t8])
+          EnumerableCalc(expr#0..2=[{inputs}], expr#3=[false], cs=[$t3])
             EnumerableTableScan(table=[[scott, DEPT]])
 !plan
 
@@ -1009,7 +1009,7 @@ EnumerableCalc(expr#0..3=[{inputs}], expr#4=[false], 
expr#5=[=($t2, $t4)], expr#
     EnumerableLimit(fetch=[1])
       EnumerableSort(sort0=[$0], dir0=[DESC])
         EnumerableAggregate(group=[{0}], c=[COUNT()])
-          EnumerableCalc(expr#0..2=[{inputs}], expr#3=[true], expr#4=[10], 
expr#5=[CAST($t0):TINYINT], expr#6=[=($t4, $t5)], expr#7=[IS NULL($t5)], 
expr#8=[OR($t6, $t7)], cs=[$t3], $condition=[$t8])
+          EnumerableCalc(expr#0..2=[{inputs}], expr#3=[true], expr#4=[10], 
expr#5=[=($t4, $t0)], cs=[$t3], $condition=[$t5])
             EnumerableTableScan(table=[[scott, DEPT]])
 !plan
 
@@ -1081,7 +1081,7 @@ EnumerableCalc(expr#0..3=[{inputs}], expr#4=[IS 
NULL($t3)], expr#5=[false], expr
     EnumerableLimit(fetch=[1])
       EnumerableSort(sort0=[$0], dir0=[DESC])
         EnumerableAggregate(group=[{0}], c=[COUNT()])
-          EnumerableCalc(expr#0..2=[{inputs}], expr#3=[false], expr#4=[123], 
expr#5=[null:INTEGER], expr#6=[=($t4, $t5)], expr#7=[IS NULL($t5)], 
expr#8=[OR($t6, $t7)], cs=[$t3], $condition=[$t8])
+          EnumerableCalc(expr#0..2=[{inputs}], expr#3=[false], cs=[$t3])
             EnumerableTableScan(table=[[scott, DEPT]])
 !plan
 
@@ -1259,7 +1259,7 @@ EnumerableCalc(expr#0..3=[{inputs}], expr#4=[IS 
NULL($t3)], expr#5=[false], expr
     EnumerableLimit(fetch=[1])
       EnumerableSort(sort0=[$0], dir0=[DESC])
         EnumerableAggregate(group=[{0}], c=[COUNT()])
-          EnumerableCalc(expr#0..2=[{inputs}], expr#3=[true], expr#4=[10], 
expr#5=[CAST($t0):TINYINT], expr#6=[=($t4, $t5)], expr#7=[IS NULL($t5)], 
expr#8=[OR($t6, $t7)], cs=[$t3], $condition=[$t8])
+          EnumerableCalc(expr#0..2=[{inputs}], expr#3=[true], expr#4=[10], 
expr#5=[=($t4, $t0)], cs=[$t3], $condition=[$t5])
             EnumerableTableScan(table=[[scott, DEPT]])
 !plan
 
@@ -1424,7 +1424,7 @@ select sal from "scott".emp
 EnumerableCalc(expr#0..2=[{inputs}], SAL=[$t2])
   EnumerableJoin(condition=[true], joinType=[inner])
     EnumerableAggregate(group=[{0}])
-      EnumerableCalc(expr#0..2=[{inputs}], expr#3=[true], expr#4=[10], 
expr#5=[CAST($t0):TINYINT], expr#6=[=($t4, $t5)], cs=[$t3], $condition=[$t6])
+      EnumerableCalc(expr#0..2=[{inputs}], expr#3=[true], expr#4=[10], 
expr#5=[=($t4, $t0)], cs=[$t3], $condition=[$t5])
         EnumerableTableScan(table=[[scott, DEPT]])
     EnumerableCalc(expr#0..7=[{inputs}], EMPNO=[$t0], SAL=[$t5])
       EnumerableTableScan(table=[[scott, EMP]])
@@ -1468,7 +1468,7 @@ EnumerableCalc(expr#0..3=[{inputs}], expr#4=[false], 
expr#5=[=($t2, $t4)], expr#
     EnumerableLimit(fetch=[1])
       EnumerableSort(sort0=[$0], dir0=[DESC])
         EnumerableAggregate(group=[{0}], c=[COUNT()])
-          EnumerableCalc(expr#0..2=[{inputs}], expr#3=[false], expr#4=[123], 
expr#5=[null:INTEGER], expr#6=[=($t4, $t5)], expr#7=[IS NULL($t5)], 
expr#8=[OR($t6, $t7)], cs=[$t3], $condition=[$t8])
+          EnumerableCalc(expr#0..2=[{inputs}], expr#3=[false], cs=[$t3])
             EnumerableTableScan(table=[[scott, DEPT]])
 !plan
 
@@ -1573,7 +1573,7 @@ EnumerableCalc(expr#0..3=[{inputs}], expr#4=[false], 
expr#5=[=($t2, $t4)], expr#
     EnumerableLimit(fetch=[1])
       EnumerableSort(sort0=[$0], dir0=[DESC])
         EnumerableAggregate(group=[{0}], c=[COUNT()])
-          EnumerableCalc(expr#0..2=[{inputs}], expr#3=[true], expr#4=[10], 
expr#5=[CAST($t0):TINYINT], expr#6=[=($t4, $t5)], expr#7=[IS NULL($t5)], 
expr#8=[OR($t6, $t7)], cs=[$t3], $condition=[$t8])
+          EnumerableCalc(expr#0..2=[{inputs}], expr#3=[true], expr#4=[10], 
expr#5=[=($t4, $t0)], cs=[$t3], $condition=[$t5])
             EnumerableTableScan(table=[[scott, DEPT]])
 !plan
 
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 2144c10..3cae87e 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
@@ -160,8 +160,8 @@ public class GeodeAllDataTypesTest extends 
AbstractGeodeTest {
         .queryContains(
             GeodeAssertions.query("SELECT stringValue AS stringValue "
                 + "FROM /allDataTypesRegion WHERE "
-                + "stringValue IN SET('abc', 'def') OR floatValue IN 
SET(1.5678, null) "
-                + "OR booleanValue IN SET(true, false, null)"));
+                + "stringValue IN SET('abc', 'def') OR floatValue = 1.5678 "
+                + "OR booleanValue IN SET(true, false)"));
   }
 
   @Test
diff --git 
a/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeZipsTest.java 
b/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeZipsTest.java
index 0737014..caea5ea 100644
--- 
a/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeZipsTest.java
+++ 
b/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeZipsTest.java
@@ -282,7 +282,7 @@ public class GeodeZipsTest extends AbstractGeodeTest {
   @Test
   public void testWhereWithOrWithEmptyResult() {
     String expectedQuery = "SELECT state AS state FROM /zips "
-        + "WHERE state IN SET('', null, true, false, 123, 13.892)";
+        + "WHERE state IN SET('', true, false, 123, 13.892)";
     calciteAssert()
         .query("SELECT state as state "
             + "FROM view WHERE state = '' OR state = null OR "

Reply via email to