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 "