[CALCITE-2731] RexProgramBuilder makes unsafe simplifications to CASE expressions (Zoltan Haindrich)
Simplification treated LocalRefs as safe expressions; which was wrong, since they might be used to reference expressions such as x / 0. Close apache/calcite#960 Project: http://git-wip-us.apache.org/repos/asf/calcite/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/b51dbdb9 Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/b51dbdb9 Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/b51dbdb9 Branch: refs/heads/branch-1.18 Commit: b51dbdb99907f92d9e9f3f92dbe27fe4160ae0b2 Parents: 8c7dc78 Author: Zoltan Haindrich <k...@rxd.hu> Authored: Fri Dec 7 17:28:10 2018 +0100 Committer: Julian Hyde <jh...@apache.org> Committed: Wed Dec 12 22:43:26 2018 -0800 ---------------------------------------------------------------------- .../java/org/apache/calcite/rex/RexSimplify.java | 17 ++++++++++------- core/src/test/resources/sql/conditions.iq | 12 ++++++++++++ core/src/test/resources/sql/some.iq | 2 +- core/src/test/resources/sql/sub-query.iq | 14 +++++++------- 4 files changed, 30 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite/blob/b51dbdb9/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 52e2ea1..8fa8a8b 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java +++ b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java @@ -39,10 +39,12 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.common.collect.Multimap; import com.google.common.collect.Range; +import com.google.common.collect.Sets; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.EnumSet; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashSet; @@ -843,12 +845,13 @@ public class RexSimplify { /** * Decides whether it is safe to flatten the given case part into AND/ORs */ - static class SafeRexVisitor implements RexVisitor<Boolean> { + enum SafeRexVisitor implements RexVisitor<Boolean> { + INSTANCE; - private Set<SqlKind> safeOps; + private final Set<SqlKind> safeOps; SafeRexVisitor() { - safeOps = new HashSet<>(); + Set<SqlKind> safeOps = EnumSet.noneOf(SqlKind.class); safeOps.addAll(SqlKind.COMPARISON); safeOps.add(SqlKind.PLUS); @@ -867,6 +870,7 @@ public class RexSimplify { safeOps.add(SqlKind.NOT); safeOps.add(SqlKind.CASE); safeOps.add(SqlKind.LIKE); + this.safeOps = Sets.immutableEnumSet(safeOps); } @Override public Boolean visitInputRef(RexInputRef inputRef) { @@ -874,7 +878,7 @@ public class RexSimplify { } @Override public Boolean visitLocalRef(RexLocalRef localRef) { - return true; + return false; } @Override public Boolean visitLiteral(RexLiteral literal) { @@ -882,7 +886,6 @@ public class RexSimplify { } @Override public Boolean visitCall(RexCall call) { - if (!safeOps.contains(call.getKind())) { return false; } @@ -937,12 +940,12 @@ public class RexSimplify { * <pre>case when a > 0 then 1 / a else null end</pre> */ static boolean isSafeExpression(RexNode r) { - return r.accept(new SafeRexVisitor()); + return r.accept(SafeRexVisitor.INSTANCE); } private static RexNode simplifyBooleanCase(RexBuilder rexBuilder, List<CaseBranch> inputBranches, RexUnknownAs unknownAs, RelDataType branchType) { - RexNode result = null; + RexNode result; // prepare all condition/branches for boolean interpretation // It's done here make these interpretation changes available to case2or simplifications http://git-wip-us.apache.org/repos/asf/calcite/blob/b51dbdb9/core/src/test/resources/sql/conditions.iq ---------------------------------------------------------------------- diff --git a/core/src/test/resources/sql/conditions.iq b/core/src/test/resources/sql/conditions.iq index 70f7b76..aede5e0 100644 --- a/core/src/test/resources/sql/conditions.iq +++ b/core/src/test/resources/sql/conditions.iq @@ -271,4 +271,16 @@ from ax where ((s,t) in (('a','a'),(null, 'bb'))) is null; !ok +with ax(s) as (values (1),(0)) +select case when s=0 then false else 100/s > 0 end from ax; + ++--------+ +| EXPR$0 | ++--------+ +| false | +| true | ++--------+ +(2 rows) + +!ok # End conditions.iq http://git-wip-us.apache.org/repos/asf/calcite/blob/b51dbdb9/core/src/test/resources/sql/some.iq ---------------------------------------------------------------------- diff --git a/core/src/test/resources/sql/some.iq b/core/src/test/resources/sql/some.iq index 6eb7e92..bde7a74 100644 --- a/core/src/test/resources/sql/some.iq +++ b/core/src/test/resources/sql/some.iq @@ -108,7 +108,7 @@ from "scott".emp; (14 rows) !ok -EnumerableCalc(expr#0..10=[{inputs}], expr#11=[0], expr#12=[=($t1, $t11)], expr#13=[false], expr#14=[AND($t12, $t13)], expr#15=[<=($t8, $t0)], expr#16=[IS TRUE($t15)], expr#17=[true], expr#18=[NOT($t12)], expr#19=[AND($t16, $t17, $t18)], expr#20=[>($t1, $t2)], expr#21=[null], expr#22=[CAST($t21):BOOLEAN], expr#23=[NOT($t16)], expr#24=[AND($t20, $t22, $t18, $t23)], expr#25=[NOT($t20)], expr#26=[AND($t15, $t18, $t23, $t25)], expr#27=[OR($t14, $t19, $t24, $t26)], expr#28=[NOT($t27)], EMPNO=[$t3], ENAME=[$t4], JOB=[$t5], MGR=[$t6], HIREDATE=[$t7], SAL=[$t8], COMM=[$t9], DEPTNO=[$t10], X=[$t28]) +EnumerableCalc(expr#0..10=[{inputs}], expr#11=[0], expr#12=[=($t1, $t11)], expr#13=[false], expr#14=[<=($t8, $t0)], expr#15=[IS TRUE($t14)], expr#16=[true], expr#17=[>($t1, $t2)], expr#18=[null], expr#19=[CASE($t12, $t13, $t15, $t16, $t17, $t18, $t14)], expr#20=[NOT($t19)], EMPNO=[$t3], ENAME=[$t4], JOB=[$t5], MGR=[$t6], HIREDATE=[$t7], SAL=[$t8], COMM=[$t9], DEPTNO=[$t10], X=[$t20]) EnumerableJoin(condition=[true], joinType=[inner]) EnumerableAggregate(group=[{}], m=[MAX($6)], c=[COUNT()], d=[COUNT($6)]) EnumerableTableScan(table=[[scott, EMP]]) http://git-wip-us.apache.org/repos/asf/calcite/blob/b51dbdb9/core/src/test/resources/sql/sub-query.iq ---------------------------------------------------------------------- diff --git a/core/src/test/resources/sql/sub-query.iq b/core/src/test/resources/sql/sub-query.iq index e29948f..26c0795 100644 --- a/core/src/test/resources/sql/sub-query.iq +++ b/core/src/test/resources/sql/sub-query.iq @@ -1038,7 +1038,7 @@ from "scott".emp; (14 rows) !ok -EnumerableCalc(expr#0..3=[{inputs}], expr#4=[IS NULL($t3)], expr#5=[false], expr#6=[AND($t4, $t5)], expr#7=[=($t2, $t5)], expr#8=[null], expr#9=[IS NULL($t8)], expr#10=[OR($t7, $t9)], expr#11=[IS TRUE($t10)], expr#12=[null], expr#13=[CAST($t12):BOOLEAN], expr#14=[NOT($t4)], expr#15=[AND($t11, $t13, $t14)], expr#16=[IS NOT NULL($t2)], expr#17=[true], expr#18=[IS NOT TRUE($t10)], expr#19=[AND($t16, $t17, $t18, $t14)], expr#20=[NOT($t16)], expr#21=[AND($t5, $t18, $t14, $t20)], expr#22=[OR($t6, $t15, $t19, $t21)], expr#23=[NOT($t22)], SAL=[$t1], EXPR$1=[$t23]) +EnumerableCalc(expr#0..3=[{inputs}], expr#4=[IS NULL($t3)], expr#5=[false], expr#6=[=($t2, $t5)], expr#7=[null], expr#8=[null], expr#9=[IS NULL($t8)], expr#10=[IS NOT NULL($t2)], expr#11=[true], expr#12=[CASE($t4, $t5, $t6, $t7, $t9, $t7, $t10, $t11, $t5)], expr#13=[NOT($t12)], SAL=[$t1], EXPR$1=[$t13]) EnumerableJoin(condition=[true], joinType=[left]) EnumerableCalc(expr#0..7=[{inputs}], EMPNO=[$t0], SAL=[$t5]) EnumerableTableScan(table=[[scott, EMP]]) @@ -1074,7 +1074,7 @@ from "scott".emp; (14 rows) !ok -EnumerableCalc(expr#0..3=[{inputs}], expr#4=[IS NULL($t3)], expr#5=[false], expr#6=[AND($t4, $t5)], expr#7=[=($t2, $t5)], expr#8=[IS TRUE($t7)], expr#9=[null], expr#10=[CAST($t9):BOOLEAN], expr#11=[NOT($t4)], expr#12=[AND($t8, $t10, $t11)], expr#13=[IS NOT NULL($t2)], expr#14=[true], expr#15=[IS NOT TRUE($t7)], expr#16=[AND($t13, $t14, $t15, $t11)], expr#17=[NOT($t13)], expr#18=[AND($t5, $t15, $t11, $t17)], expr#19=[OR($t6, $t12, $t16, $t18)], expr#20=[NOT($t19)], SAL=[$t1], EXPR$1=[$t20]) +EnumerableCalc(expr#0..3=[{inputs}], expr#4=[IS NULL($t3)], expr#5=[false], expr#6=[=($t2, $t5)], expr#7=[null], expr#8=[IS NOT NULL($t2)], expr#9=[true], expr#10=[CASE($t4, $t5, $t6, $t7, $t8, $t9, $t5)], expr#11=[NOT($t10)], SAL=[$t1], EXPR$1=[$t11]) EnumerableJoin(condition=[true], joinType=[left]) EnumerableCalc(expr#0..7=[{inputs}], EMPNO=[$t0], SAL=[$t5]) EnumerableTableScan(table=[[scott, EMP]]) @@ -1110,7 +1110,7 @@ from "scott".emp; (14 rows) !ok -EnumerableCalc(expr#0..3=[{inputs}], expr#4=[IS NULL($t3)], expr#5=[false], expr#6=[AND($t4, $t5)], expr#7=[=($t2, $t5)], expr#8=[null], expr#9=[IS NULL($t8)], expr#10=[OR($t7, $t9)], expr#11=[IS TRUE($t10)], expr#12=[null], expr#13=[CAST($t12):BOOLEAN], expr#14=[NOT($t4)], expr#15=[AND($t11, $t13, $t14)], expr#16=[IS NOT NULL($t2)], expr#17=[true], expr#18=[IS NOT TRUE($t10)], expr#19=[AND($t16, $t17, $t18, $t14)], expr#20=[NOT($t16)], expr#21=[AND($t5, $t18, $t14, $t20)], expr#22=[OR($t6, $t15, $t19, $t21)], expr#23=[NOT($t22)], SAL=[$t1], EXPR$1=[$t23]) +EnumerableCalc(expr#0..3=[{inputs}], expr#4=[IS NULL($t3)], expr#5=[false], expr#6=[=($t2, $t5)], expr#7=[null], expr#8=[null], expr#9=[IS NULL($t8)], expr#10=[IS NOT NULL($t2)], expr#11=[true], expr#12=[CASE($t4, $t5, $t6, $t7, $t9, $t7, $t10, $t11, $t5)], expr#13=[NOT($t12)], SAL=[$t1], EXPR$1=[$t13]) EnumerableJoin(condition=[true], joinType=[left]) EnumerableCalc(expr#0..7=[{inputs}], EMPNO=[$t0], SAL=[$t5]) EnumerableTableScan(table=[[scott, EMP]]) @@ -1146,7 +1146,7 @@ from "scott".emp; (14 rows) !ok -EnumerableCalc(expr#0..3=[{inputs}], expr#4=[IS NULL($t3)], expr#5=[false], expr#6=[AND($t4, $t5)], expr#7=[=($t2, $t5)], expr#8=[null], expr#9=[IS NULL($t8)], expr#10=[OR($t7, $t9)], expr#11=[IS TRUE($t10)], expr#12=[null], expr#13=[CAST($t12):BOOLEAN], expr#14=[NOT($t4)], expr#15=[AND($t11, $t13, $t14)], expr#16=[IS NOT NULL($t2)], expr#17=[true], expr#18=[IS NOT TRUE($t10)], expr#19=[AND($t16, $t17, $t18, $t14)], expr#20=[NOT($t16)], expr#21=[AND($t5, $t18, $t14, $t20)], expr#22=[OR($t6, $t15, $t19, $t21)], expr#23=[NOT($t22)], SAL=[$t1], EXPR$1=[$t23]) +EnumerableCalc(expr#0..3=[{inputs}], expr#4=[IS NULL($t3)], expr#5=[false], expr#6=[=($t2, $t5)], expr#7=[null], expr#8=[null], expr#9=[IS NULL($t8)], expr#10=[IS NOT NULL($t2)], expr#11=[true], expr#12=[CASE($t4, $t5, $t6, $t7, $t9, $t7, $t10, $t11, $t5)], expr#13=[NOT($t12)], SAL=[$t1], EXPR$1=[$t13]) EnumerableJoin(condition=[true], joinType=[left]) EnumerableCalc(expr#0..7=[{inputs}], EMPNO=[$t0], SAL=[$t5]) EnumerableTableScan(table=[[scott, EMP]]) @@ -1182,7 +1182,7 @@ from "scott".emp; (14 rows) !ok -EnumerableCalc(expr#0..3=[{inputs}], expr#4=[IS NULL($t3)], expr#5=[false], expr#6=[AND($t4, $t5)], expr#7=[=($t2, $t5)], expr#8=[null], expr#9=[IS NULL($t8)], expr#10=[OR($t7, $t9)], expr#11=[IS TRUE($t10)], expr#12=[null], expr#13=[CAST($t12):BOOLEAN], expr#14=[NOT($t4)], expr#15=[AND($t11, $t13, $t14)], expr#16=[IS NOT NULL($t2)], expr#17=[true], expr#18=[IS NOT TRUE($t10)], expr#19=[AND($t16, $t17, $t18, $t14)], expr#20=[NOT($t16)], expr#21=[AND($t5, $t18, $t14, $t20)], expr#22=[OR($t6, $t15, $t19, $t21)], expr#23=[NOT($t22)], SAL=[$t1], EXPR$1=[$t23]) +EnumerableCalc(expr#0..3=[{inputs}], expr#4=[IS NULL($t3)], expr#5=[false], expr#6=[=($t2, $t5)], expr#7=[null], expr#8=[null], expr#9=[IS NULL($t8)], expr#10=[IS NOT NULL($t2)], expr#11=[true], expr#12=[CASE($t4, $t5, $t6, $t7, $t9, $t7, $t10, $t11, $t5)], expr#13=[NOT($t12)], SAL=[$t1], EXPR$1=[$t13]) EnumerableJoin(condition=[true], joinType=[left]) EnumerableCalc(expr#0..7=[{inputs}], EMPNO=[$t0], SAL=[$t5]) EnumerableTableScan(table=[[scott, EMP]]) @@ -1218,7 +1218,7 @@ from "scott".emp; (14 rows) !ok -EnumerableCalc(expr#0..2=[{inputs}], expr#3=[IS NOT NULL($t2)], expr#4=[true], expr#5=[AND($t3, $t4)], expr#6=[false], expr#7=[NOT($t3)], expr#8=[AND($t6, $t7)], expr#9=[OR($t5, $t8)], expr#10=[NOT($t9)], SAL=[$t1], EXPR$1=[$t10]) +EnumerableCalc(expr#0..2=[{inputs}], expr#3=[IS NOT NULL($t2)], expr#4=[true], expr#5=[false], expr#6=[CASE($t3, $t4, $t5)], expr#7=[NOT($t6)], SAL=[$t1], EXPR$1=[$t7]) EnumerableJoin(condition=[true], joinType=[left]) EnumerableCalc(expr#0..7=[{inputs}], EMPNO=[$t0], SAL=[$t5]) EnumerableTableScan(table=[[scott, EMP]]) @@ -1252,7 +1252,7 @@ from "scott".emp; (14 rows) !ok -EnumerableCalc(expr#0..3=[{inputs}], expr#4=[IS NULL($t3)], expr#5=[false], expr#6=[AND($t4, $t5)], expr#7=[=($t2, $t5)], expr#8=[IS TRUE($t7)], expr#9=[null], expr#10=[CAST($t9):BOOLEAN], expr#11=[NOT($t4)], expr#12=[AND($t8, $t10, $t11)], expr#13=[IS NOT NULL($t2)], expr#14=[true], expr#15=[IS NOT TRUE($t7)], expr#16=[AND($t13, $t14, $t15, $t11)], expr#17=[NOT($t13)], expr#18=[AND($t5, $t15, $t11, $t17)], expr#19=[OR($t6, $t12, $t16, $t18)], expr#20=[NOT($t19)], SAL=[$t1], EXPR$1=[$t20]) +EnumerableCalc(expr#0..3=[{inputs}], expr#4=[IS NULL($t3)], expr#5=[false], expr#6=[=($t2, $t5)], expr#7=[null], expr#8=[IS NOT NULL($t2)], expr#9=[true], expr#10=[CASE($t4, $t5, $t6, $t7, $t8, $t9, $t5)], expr#11=[NOT($t10)], SAL=[$t1], EXPR$1=[$t11]) EnumerableJoin(condition=[true], joinType=[left]) EnumerableCalc(expr#0..7=[{inputs}], EMPNO=[$t0], SAL=[$t5]) EnumerableTableScan(table=[[scott, EMP]])