You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jc...@apache.org on 2018/10/25 18:02:41 UTC
calcite git commit: [CALCITE-2631] General improvements in case
simplification [CALCITE-2639] FilterReduceExpressionsRule causes
ArithmeticException at execution time [Forced Update!]
Repository: calcite
Updated Branches:
refs/heads/master 7a0dc6ec8 -> 7a079229a (forced update)
[CALCITE-2631] General improvements in case simplification
[CALCITE-2639] FilterReduceExpressionsRule causes ArithmeticException at execution time
Close apache/calcite#893
Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/7a079229
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/7a079229
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/7a079229
Branch: refs/heads/master
Commit: 7a079229a9d4d42c9249ba95544e235db5b6eb1f
Parents: 2904693
Author: Jesus Camacho Rodriguez <jc...@apache.org>
Authored: Wed Oct 24 12:42:02 2018 -0700
Committer: Jesus Camacho Rodriguez <jc...@apache.org>
Committed: Thu Oct 25 11:02:11 2018 -0700
----------------------------------------------------------------------
.../org/apache/calcite/rex/RexSimplify.java | 124 +++++++++++--------
.../apache/calcite/test/RelOptRulesTest.java | 11 ++
.../org/apache/calcite/test/RexProgramTest.java | 22 ++++
.../org/apache/calcite/test/RelOptRulesTest.xml | 16 +++
4 files changed, 123 insertions(+), 50 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/calcite/blob/7a079229/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
----------------------------------------------------------------------
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 8905794..4ac6ed5 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
@@ -676,52 +676,84 @@ public class RexSimplify {
private RexNode simplifyCase(RexCall call, RexUnknownAs unknownAs) {
List<CaseBranch> inputBranches =
- CaseBranch.fromCaseOperands(rexBuilder, new ArrayList(call.getOperands()));
+ CaseBranch.fromCaseOperands(rexBuilder, new ArrayList<>(call.getOperands()));
// run simplification on all operands
- RexSimplify branchSimplifier = this;
- RelDataType branchType = call.getType();
+ RexSimplify condSimplifier = this.withPredicates(RelOptPredicateList.EMPTY);
+ RexSimplify valueSimplifier = this;
+ RelDataType caseType = call.getType();
+ boolean conditionNeedsSimplify = false;
+ CaseBranch lastBranch = null;
List<CaseBranch> branches = new ArrayList<>();
- for (CaseBranch branch : inputBranches) {
+ for (CaseBranch inputBranch : inputBranches) {
// simplify the condition
- RexNode newCond = branchSimplifier.simplify(branch.cond, RexUnknownAs.FALSE);
-
- // use the condition to simplify the branch
- RexNode value = branch.value;
- RexNode newValue = branchSimplifier.simplify(value, unknownAs);
+ RexNode newCond = condSimplifier.simplify(inputBranch.cond, RexUnknownAs.FALSE);
+ if (newCond.isAlwaysFalse()) {
+ // If the condition is false, we do not need to add it
+ continue;
+ }
- branches.add(new CaseBranch(newCond, newValue));
- }
+ // simplify the value
+ RexNode newValue = valueSimplifier.simplify(inputBranch.value, unknownAs);
- // remove branches with invalid conditions
- branches.removeIf(branch -> branch.cond.isAlwaysFalse());
+ // create new branch
+ if (lastBranch != null) {
+ if (lastBranch.value.toString().equals(newValue.toString())
+ && isSafeExpression(newCond)) {
+ // in this case, last branch and new branch have the same conclusion,
+ // hence we create a new composite condition and we do not add it to
+ // the final branches for the time being
+ newCond = rexBuilder.makeCall(SqlStdOperatorTable.OR, lastBranch.cond, newCond);
+ conditionNeedsSimplify = true;
+ } else {
+ // if we reach here, the new branch is not mergeable with the last one,
+ // hence we are going to add the last branch to the final branches.
+ // if the last branch was merged, then we will simplify it first.
+ // otherwise, we just add it
+ CaseBranch branch = generateBranch(conditionNeedsSimplify, condSimplifier, lastBranch);
+ if (!branch.cond.isAlwaysFalse()) {
+ // If the condition is not false, we add it to the final result
+ branches.add(branch);
+ if (branch.cond.isAlwaysTrue()) {
+ // If the condition is always true, we are done
+ lastBranch = null;
+ break;
+ }
+ }
+ conditionNeedsSimplify = false;
+ }
+ }
+ lastBranch = new CaseBranch(newCond, newValue);
- // delete all branches after the first AlwaysTrue
- for (int i = 0; i < branches.size(); i++) {
- CaseBranch branch = branches.get(i);
- if (branch.cond.isAlwaysTrue()) {
- branches.subList(i + 1, branches.size()).clear();
+ if (newCond.isAlwaysTrue()) {
+ // If the condition is always true, we are done (useful in first loop iteration)
break;
}
}
-
- branches = compactBranchesWithTheSameConclusion(branches);
+ if (lastBranch != null) {
+ // we need to add the last pending branch once we have finished
+ // with the for loop
+ CaseBranch branch = generateBranch(conditionNeedsSimplify, condSimplifier, lastBranch);
+ if (!branch.cond.isAlwaysFalse()) {
+ branches.add(branch);
+ }
+ }
if (branches.size() == 1) {
- final RexNode firstValue = branches.get(0).value;
-
- if (sameTypeOrNarrowsNullability(branchType, firstValue.getType())) {
- return firstValue;
+ // we can just return the value in this case (matching the case type)
+ final RexNode value = branches.get(0).value;
+ if (sameTypeOrNarrowsNullability(caseType, value.getType())) {
+ return value;
} else {
- return rexBuilder.makeAbstractCast(branchType, firstValue);
+ return rexBuilder.makeAbstractCast(caseType, value);
}
}
if (call.getType().getSqlTypeName() == SqlTypeName.BOOLEAN) {
- final RexNode result = simplifyBooleanCase(rexBuilder, branches, unknownAs, branchType);
+ final RexNode result = simplifyBooleanCase(rexBuilder, branches, unknownAs, caseType);
if (result != null) {
- if (sameTypeOrNarrowsNullability(branchType, result.getType())) {
+ if (sameTypeOrNarrowsNullability(caseType, result.getType())) {
return simplify(result, unknownAs);
} else {
// If the simplification would widen the nullability
@@ -738,30 +770,22 @@ public class RexSimplify {
if (newOperands.equals(call.getOperands())) {
return call;
}
- RexNode retNode = rexBuilder.makeCall(SqlStdOperatorTable.CASE, newOperands);
- assert sameTypeOrNarrowsNullability(branchType, retNode.getType()) : "Unexpected type change.";
- return retNode;
+ return rexBuilder.makeCall(SqlStdOperatorTable.CASE, newOperands);
}
- private List<CaseBranch> compactBranchesWithTheSameConclusion(List<CaseBranch> branches) {
- ArrayList newBranches = new ArrayList<>();
- CaseBranch last = null;
- for (CaseBranch b : branches) {
- if (last == null) {
- last = b;
- } else {
- if (last.value.toString().equals(b.value.toString()) && isSafeExpression(b.cond)) {
- RexNode newCond = rexBuilder.makeCall(SqlStdOperatorTable.OR, last.cond, b.cond);
- // last and b are 2 consequent branches with the same conclusion
- last = new CaseBranch(simplify(newCond, UNKNOWN.FALSE), last.value);
- } else {
- newBranches.add(last);
- last = b;
- }
- }
- }
- newBranches.add(last);
- return newBranches;
+ /**
+ * If boolean is true, simplify cond in input branch and return new branch.
+ * Otherwise, simply return input branch.
+ */
+ private CaseBranch generateBranch(boolean simplifyCond, RexSimplify simplifier,
+ CaseBranch branch) {
+ if (simplifyCond) {
+ // the previous branch was merged, time to simplify it and
+ // add it to the final result
+ return new CaseBranch(
+ simplifier.simplify(branch.cond, RexUnknownAs.FALSE), branch.value);
+ }
+ return branch;
}
/**
@@ -907,7 +931,7 @@ public class RexSimplify {
*
* CaseSafe means that it only contains a combination of known good operators.
*
- * The divison is an unsafe operator, consider:
+ * The division is an unsafe operator, consider:
* <pre>case when a > 0 then 1/a else null end</pre>
*/
static boolean isSafeExpression(RexNode r) {
http://git-wip-us.apache.org/repos/asf/calcite/blob/7a079229/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
index 1f54233..720b5a8 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -4056,6 +4056,17 @@ public class RelOptRulesTest extends RelOptTestBase {
final String planAfter = NL + RelOptUtil.toString(relAfter);
diffRepos.assertEquals("planAfter", "${planAfter}", planAfter);
}
+
+ @Test public void testOversimplifiedCaseStatement() {
+ HepProgram program = new HepProgramBuilder()
+ .addRuleInstance(ReduceExpressionsRule.FILTER_INSTANCE)
+ .build();
+
+ String sql = "select * from emp "
+ + "where MGR > 0 and "
+ + "case when MGR > 0 then deptno / MGR else null end > 1";
+ checkPlanning(program, sql);
+ }
}
// End RelOptRulesTest.java
http://git-wip-us.apache.org/repos/asf/calcite/blob/7a079229/core/src/test/java/org/apache/calcite/test/RexProgramTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/RexProgramTest.java b/core/src/test/java/org/apache/calcite/test/RexProgramTest.java
index 9ccccc6..5d6b15a 100644
--- a/core/src/test/java/org/apache/calcite/test/RexProgramTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RexProgramTest.java
@@ -1754,6 +1754,28 @@ public class RexProgramTest extends RexProgramBuilderBase {
assertThat(result, is(condition));
}
+ @Test public void testSimplifyCaseBranchesCollapse() {
+ // case when x is true then 1 when x is not true then 1 else 2 end
+ // => case when x is true or x is not true then 1 else 2 end
+ checkSimplify(
+ case_(
+ isTrue(vBool()), literal(1),
+ isNotTrue(vBool()), literal(1),
+ literal(2)),
+ "CASE(OR(IS TRUE(?0.bool0), IS NOT TRUE(?0.bool0)), 1, 2)");
+ }
+
+ @Test public void testSimplifyCaseBranchesCollapse2() {
+ // case when x is true then 1 when true then 1 else 2 end
+ // => 1
+ checkSimplify(
+ case_(
+ isTrue(vBool()), literal(1),
+ trueLiteral, literal(1),
+ literal(2)),
+ "1");
+ }
+
@Test public void testSimplifyCaseNullableVarChar() {
RexNode condition = eq(input(tVarchar(), 0), literal("S"));
RexNode caseNode = case_(condition, literal("A"), literal("B"));
http://git-wip-us.apache.org/repos/asf/calcite/blob/7a079229/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
----------------------------------------------------------------------
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 b7af98c..e344576 100644
--- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -8548,4 +8548,20 @@ LogicalCorrelate(correlation=[$cor0], joinType=[anti], requiredColumns=[{0}])
]]>
</Resource>
</TestCase>
+ <TestCase name="testOversimplifiedCaseStatement">
+ <Resource name="planBefore">
+ <![CDATA[
+LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8])
+ LogicalFilter(condition=[AND(>($3, 0), >(CASE(>($3, 0), /($7, $3), null), 1))])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ <Resource name="planAfter">
+ <![CDATA[
+LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8])
+ LogicalFilter(condition=[AND(>($3, 0), CASE(>($3, 0), >(/($7, $3), 1), false))])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ </TestCase>
</Root>