This is an automated email from the ASF dual-hosted git repository.
kgyrtkirk 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 564caac [CALCITE-2338] Make Simplification API more conservative
564caac is described below
commit 564caac262e43d7c27397d7bd17ab0f1689fdf4d
Author: Zoltan Haindrich <[email protected]>
AuthorDate: Tue Feb 26 15:20:08 2019 +0100
[CALCITE-2338] Make Simplification API more conservative
Deprecated simplifyOr/simplitfyAnd methods as alternate entry points may
not get that much attention during development.
Before this, some simplification opportunities were missed during predicate
processing.
From now on in RelBuilder all Rex simplifications are postponed until Rel
creation time - which makes it more consistent and it also obeys the
simplification setting.
---
.../calcite/rel/metadata/RelMdPredicates.java | 4 ++-
.../java/org/apache/calcite/rex/RexSimplify.java | 40 +++++++++++++---------
.../main/java/org/apache/calcite/rex/RexUtil.java | 5 +--
.../java/org/apache/calcite/tools/RelBuilder.java | 5 ++-
.../org/apache/calcite/test/RelOptRulesTest.xml | 4 +--
5 files changed, 35 insertions(+), 23 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java
b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java
index a7993cb..a044898 100644
--- a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java
+++ b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java
@@ -43,6 +43,7 @@ import org.apache.calcite.rex.RexLiteral;
import org.apache.calcite.rex.RexNode;
import org.apache.calcite.rex.RexPermuteInputsShuttle;
import org.apache.calcite.rex.RexSimplify;
+import org.apache.calcite.rex.RexUnknownAs;
import org.apache.calcite.rex.RexUtil;
import org.apache.calcite.rex.RexVisitorImpl;
import org.apache.calcite.sql.SqlKind;
@@ -402,7 +403,8 @@ public class RelMdPredicates
Util.first(cluster.getPlanner().getExecutor(), RexUtil.EXECUTOR);
RexNode disjunctivePredicate =
new RexSimplify(rexBuilder, RelOptPredicateList.EMPTY, executor)
- .simplifyOrs(finalResidualPredicates);
+ .simplifyUnknownAs(rexBuilder.makeCall(SqlStdOperatorTable.OR,
finalResidualPredicates),
+ RexUnknownAs.FALSE);
if (!disjunctivePredicate.isAlwaysTrue()) {
predicates.add(disjunctivePredicate);
}
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 65694fa..5528467 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
@@ -52,7 +52,6 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
-import java.util.function.Function;
import static org.apache.calcite.rex.RexUnknownAs.FALSE;
import static org.apache.calcite.rex.RexUnknownAs.UNKNOWN;
@@ -228,15 +227,18 @@ public class RexSimplify {
* yield UNKNOWN. (If the simplified expression has type BOOLEAN NOT NULL,
* then of course it can only return FALSE.) */
public RexNode simplifyUnknownAs(RexNode e, RexUnknownAs unknownAs) {
- return verify(e, unknownAs,
- simplifier -> simplifier.simplify(e, unknownAs));
+ final RexNode simplified = withParanoid(false).simplify(e, unknownAs);
+ if (paranoid) {
+ verify(e, simplified, unknownAs);
+ }
+ return simplified;
}
/** Internal method to simplify an expression.
*
* <p>Unlike the public {@link #simplify(RexNode)}
* and {@link #simplifyUnknownAsFalse(RexNode)} methods,
- * never calls {@link #verify(RexNode, RexUnknownAs, Function)}.
+ * never calls {@link #verify(RexNode, RexNode, RexUnknownAs)}.
* Verify adds an overhead that is only acceptable for a top-level call.
*/
RexNode simplify(RexNode e, RexUnknownAs unknownAs) {
@@ -314,6 +316,7 @@ public class RexSimplify {
private <C extends Comparable<C>> RexNode simplifyComparison(RexCall e,
RexUnknownAs unknownAs, Class<C> clazz) {
final List<RexNode> operands = new ArrayList<>(e.operands);
+ // UNKNOWN mode is warranted: false = null
simplifyList(operands, UNKNOWN);
// Simplify "x <op> x"
@@ -435,7 +438,9 @@ public class RexSimplify {
/**
* Simplifies a conjunction of boolean expressions.
*/
+ @Deprecated // to be removed before 2.0
public RexNode simplifyAnds(Iterable<? extends RexNode> nodes) {
+ ensureParanoidOff();
return simplifyAnds(nodes, defaultUnknownAs);
}
@@ -1134,6 +1139,7 @@ public class RexSimplify {
@Deprecated // to be removed before 2.0
public RexNode simplifyAnd(RexCall e) {
+ ensureParanoidOff();
return simplifyAnd(e, defaultUnknownAs);
}
@@ -1538,7 +1544,9 @@ public class RexSimplify {
/** Simplifies OR(x, x) into x, and similar.
* The simplified expression returns UNKNOWN values as is (not as FALSE). */
+ @Deprecated // to be removed before 2.0
public RexNode simplifyOr(RexCall call) {
+ ensureParanoidOff();
return simplifyOr(call, UNKNOWN);
}
@@ -1554,18 +1562,21 @@ public class RexSimplify {
/** Simplifies a list of terms and combines them into an OR.
* Modifies the list in place.
* The simplified expression returns UNKNOWN values as is (not as FALSE). */
+ @Deprecated // to be removed before 2.0
public RexNode simplifyOrs(List<RexNode> terms) {
+ ensureParanoidOff();
return simplifyOrs(terms, UNKNOWN);
}
+ private void ensureParanoidOff() {
+ if (paranoid) {
+ throw new UnsupportedOperationException("Paranoid is not supported for
this method");
+ }
+ }
+
/** Simplifies a list of terms and combines them into an OR.
* Modifies the list in place. */
private RexNode simplifyOrs(List<RexNode> terms, RexUnknownAs unknownAs) {
- if (paranoid) {
- final RexNode before = RexUtil.composeDisjunction(rexBuilder, terms);
- return verify(before, unknownAs,
- simplifier -> simplifier.simplifyOrs(terms, unknownAs));
- }
for (int i = 0; i < terms.size(); i++) {
final RexNode term = simplify(terms.get(i), unknownAs);
switch (term.getKind()) {
@@ -1591,12 +1602,7 @@ public class RexSimplify {
return RexUtil.composeDisjunction(rexBuilder, terms);
}
- private RexNode verify(RexNode before, RexUnknownAs unknownAs,
- Function<RexSimplify, RexNode> simplifier) {
- final RexNode simplified = simplifier.apply(withParanoid(false));
- if (!paranoid) {
- return simplified;
- }
+ private void verify(RexNode before, RexNode simplified, RexUnknownAs
unknownAs) {
if (simplified.isAlwaysFalse()
&& before.isAlwaysTrue()) {
throw new AssertionError("always true [" + before
@@ -1611,7 +1617,7 @@ public class RexSimplify {
final RexAnalyzer foo1 = new RexAnalyzer(simplified, predicates);
if (foo0.unsupportedCount > 0 || foo1.unsupportedCount > 0) {
// Analyzer cannot handle this expression currently
- return simplified;
+ return;
}
if (!foo0.variables.containsAll(foo1.variables)) {
throw new AssertionError("variable mismatch: "
@@ -1652,7 +1658,6 @@ public class RexSimplify {
+ ", and " + simplified + " yielded " + v1);
}
}
- return simplified;
}
private RexNode simplifyCast(RexCall e) {
@@ -2175,6 +2180,7 @@ public class RexSimplify {
list.set(index, newVal);
return true;
}
+
}
// End RexSimplify.java
diff --git a/core/src/main/java/org/apache/calcite/rex/RexUtil.java
b/core/src/main/java/org/apache/calcite/rex/RexUtil.java
index 9308577..322120d 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexUtil.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexUtil.java
@@ -1843,14 +1843,15 @@ public class RexUtil {
@Deprecated // to be removed before 2.0
public static RexNode simplifyOr(RexBuilder rexBuilder, RexCall call) {
return new RexSimplify(rexBuilder, RelOptPredicateList.EMPTY, EXECUTOR)
- .simplifyOr(call);
+ .simplifyUnknownAs(call, RexUnknownAs.UNKNOWN);
}
@Deprecated // to be removed before 2.0
public static RexNode simplifyOrs(RexBuilder rexBuilder,
List<RexNode> terms) {
return new RexSimplify(rexBuilder, RelOptPredicateList.EMPTY, EXECUTOR)
- .simplifyOrs(terms);
+ .simplifyUnknownAs(rexBuilder.makeCall(SqlStdOperatorTable.OR, terms),
+ RexUnknownAs.UNKNOWN);
}
/**
diff --git a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
index 0465286..9dfba8b 100644
--- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
+++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
@@ -576,7 +576,7 @@ public class RelBuilder {
* {@code e AND TRUE} becomes {@code e};
* {@code e AND e2 AND NOT e} becomes {@code e2}. */
public RexNode and(Iterable<? extends RexNode> operands) {
- return simplifier.simplifyAnds(operands);
+ return RexUtil.composeConjunction(getRexBuilder(), operands);
}
/** Creates an OR. */
@@ -1643,6 +1643,9 @@ public class RelBuilder {
final RelNode join;
final boolean correlate = variablesSet.size() == 1;
RexNode postCondition = literal(true);
+ if (simplify) {
+ condition = simplifier.simplifyUnknownAsFalse(condition);
+ }
if (correlate) {
final CorrelationId id = Iterables.getOnlyElement(variablesSet);
final ImmutableBitSet requiredColumns =
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 df1b18a..c94842c 100644
--- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -4664,7 +4664,7 @@ LogicalProject(EXPR$0=[1])
LogicalProject(DEPTNO=[$7])
LogicalFilter(condition=[>($7, 10)])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
- LogicalFilter(condition=[OR(>($7, 7), >($7, 10))])
+ LogicalFilter(condition=[>($7, 7)])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
</Resource>
@@ -8038,7 +8038,7 @@ LogicalProject(DEPTNO=[$1])
<Resource name="planAfter">
<![CDATA[
LogicalProject(EMPNO=[$0], D=[CASE(=($9, 0), false, IS NULL(CASE(true,
CAST($7):INTEGER, null:INTEGER)), null:NULL, IS NOT NULL($12), true, <($10,
$9), null:NULL, false)])
- LogicalJoin(condition=[=($7, $11)], joinType=[left])
+ LogicalJoin(condition=[=(CASE(true, CAST($7):INTEGER, null:INTEGER), $11)],
joinType=[left])
LogicalJoin(condition=[true], joinType=[inner])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
LogicalAggregate(group=[{}], c=[COUNT()], ck=[COUNT($0)])
