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 <[email protected]>
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 "
