[CALCITE-3979] Simplification might have removed CAST expression(s) incorrectly
ReduceExpressionsRule might have removed cast expressions and inadvertly changed the type of some subexpressions.
In case the rewriting have at some point arrived at: CAST(CAST(X,T),T) AND <(CAST(X,T),N)
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java b/core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java
index ca5c0be..a4c019b 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java
@@ -37,7 +37,6 @@
import org.apache.calcite.rel.logical.LogicalProject;
import org.apache.calcite.rel.logical.LogicalWindow;
import org.apache.calcite.rel.metadata.RelMetadataQuery;
-import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeFactory;
import org.apache.calcite.rex.RexBuilder;
import org.apache.calcite.rex.RexCall;
@@ -63,7 +62,6 @@
import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.SqlOperator;
import org.apache.calcite.sql.fun.SqlRowOperator;
-import org.apache.calcite.sql.fun.SqlStdOperatorTable;
import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.calcite.tools.RelBuilder;
import org.apache.calcite.tools.RelBuilderFactory;
@@ -650,31 +648,10 @@
// Find reducible expressions.
final List<RexNode> constExps = new ArrayList<>();
List<Boolean> addCasts = new ArrayList<>();
- final List<RexNode> removableCasts = new ArrayList<>();
findReducibleExps(rel.getCluster().getTypeFactory(), expList,
- predicates.constantMap, constExps, addCasts, removableCasts);
- if (constExps.isEmpty() && removableCasts.isEmpty()) {
- return changed;
- }
-
- // Remove redundant casts before reducing constant expressions.
- // If the argument to the redundant cast is a reducible constant,
- // reducing that argument to a constant first will result in not being
- // able to locate the original cast expression.
- if (!removableCasts.isEmpty()) {
- final List<RexNode> reducedExprs = new ArrayList<>();
- for (RexNode exp : removableCasts) {
- RexCall call = (RexCall) exp;
- reducedExprs.add(call.getOperands().get(0));
- }
- RexReplacer replacer =
- new RexReplacer(simplify, unknownAs, removableCasts, reducedExprs,
- Collections.nCopies(removableCasts.size(), false));
- replacer.mutate(expList);
- }
-
+ predicates.constantMap, constExps, addCasts);
if (constExps.isEmpty()) {
- return true;
+ return changed;
}
final List<RexNode> constExps2 = Lists.newArrayList(constExps);
@@ -739,15 +716,13 @@
* @param addCasts indicator for each expression that can be constant
* reduced, whether a cast of the resulting reduced
* expression is potentially necessary
- * @param removableCasts returns the list of cast expressions where the cast
*/
protected static void findReducibleExps(RelDataTypeFactory typeFactory,
List<RexNode> exps, ImmutableMap<RexNode, RexNode> constants,
- List<RexNode> constExps, List<Boolean> addCasts,
- List<RexNode> removableCasts) {
+ List<RexNode> constExps, List<Boolean> addCasts) {
ReducibleExprLocator gardener =
new ReducibleExprLocator(typeFactory, constants, constExps,
- addCasts, removableCasts);
+ addCasts);
for (RexNode exp : exps) {
gardener.analyze(exp);
}
@@ -920,20 +895,17 @@
private final List<Boolean> addCasts;
- private final List<RexNode> removableCasts;
-
private final Deque<SqlOperator> parentCallTypeStack = new ArrayDeque<>();
ReducibleExprLocator(RelDataTypeFactory typeFactory,
ImmutableMap<RexNode, RexNode> constants, List<RexNode> constExprs,
- List<Boolean> addCasts, List<RexNode> removableCasts) {
+ List<Boolean> addCasts) {
// go deep
super(true);
this.typeFactory = typeFactory;
this.constants = constants;
this.constExprs = constExprs;
this.addCasts = addCasts;
- this.removableCasts = removableCasts;
}
public void analyze(RexNode exp) {
@@ -1071,12 +1043,6 @@
addResult(call.getOperands().get(iOperand));
}
}
-
- // if this cast expression can't be reduced to a literal,
- // then see if we can remove the cast
- if (call.getOperator() == SqlStdOperatorTable.CAST) {
- reduceCasts(call);
- }
}
// pop operands off of the stack
@@ -1089,45 +1055,6 @@
stack.add(callConstancy);
}
- private void reduceCasts(RexCall outerCast) {
- List<RexNode> operands = outerCast.getOperands();
- if (operands.size() != 1) {
- return;
- }
- RelDataType outerCastType = outerCast.getType();
- RelDataType operandType = operands.get(0).getType();
- if (operandType.equals(outerCastType)) {
- removableCasts.add(outerCast);
- return;
- }
-
- // See if the reduction
- // CAST((CAST x AS type) AS type NOT NULL)
- // -> CAST(x AS type NOT NULL)
- // applies. TODO jvs 15-Dec-2008: consider
- // similar cases for precision changes.
- if (!(operands.get(0) instanceof RexCall)) {
- return;
- }
- RexCall innerCast = (RexCall) operands.get(0);
- if (innerCast.getOperator() != SqlStdOperatorTable.CAST) {
- return;
- }
- if (innerCast.getOperands().size() != 1) {
- return;
- }
- RelDataType outerTypeNullable =
- typeFactory.createTypeWithNullability(outerCastType, true);
- RelDataType innerTypeNullable =
- typeFactory.createTypeWithNullability(operandType, true);
- if (outerTypeNullable != innerTypeNullable) {
- return;
- }
- if (operandType.isNullable()) {
- removableCasts.add(innerCast);
- }
- }
-
@Override public Void visitDynamicParam(RexDynamicParam dynamicParam) {
return pushVariable();
}
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 4ff8902..37eaae0 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -558,6 +558,23 @@
}
/** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-3979">[CALCITE-3979]
+ * ReduceExpressionsRule might have removed CAST expression(s) incorrectly</a>. */
+ @Test public void testCastRemove() throws Exception {
+ final String sql = "select\n"
+ + "case when cast(ename as double) < 5 then 0.0\n"
+ + " else coalesce(cast(ename as double), 1.0)\n"
+ + " end as t\n"
+ + " from (\n"
+ + " select\n"
+ + " case when ename > 'abc' then ename\n"
+ + " else null\n"
+ + " end as ename from emp\n"
+ + " )";
+ sql(sql).withRule(ReduceExpressionsRule.PROJECT_INSTANCE).checkUnchanged();
+ }
+
+ /** Test case for
* <a href="https://issues.apache.org/jira/browse/CALCITE-3887">[CALCITE-3887]
* Filter and Join conditions may not need to retain nullability during simplifications</a>. */
@Test void testPushSemiJoinConditions() {
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 2685ee7..07f8b80 100644
--- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -684,6 +684,32 @@
]]>
</Resource>
</TestCase>
+ <TestCase name="testCastRemove">
+ <Resource name="sql">
+ <![CDATA[select
+case when cast(ename as double) < 5 then 0.0
+ else coalesce(cast(ename as double), 1.0)
+ end as t
+ from (
+ select
+ case when ename > 'abc' then ename
+ else null
+ end as ename from emp
+ )]]>
+ </Resource>
+ <Resource name="planBefore">
+ <![CDATA[
+LogicalProject(T=[CASE(<(CAST(CASE(>($1, 'abc'), $1, null:VARCHAR(20))):DOUBLE, 5), 0.0:DOUBLE, CASE(IS NOT NULL(CAST(CASE(>($1, 'abc'), $1, null:VARCHAR(20))):DOUBLE), CAST(CAST(CASE(>($1, 'abc'), $1, null:VARCHAR(20))):DOUBLE):DOUBLE NOT NULL, 1.0:DOUBLE))])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ <Resource name="planAfter">
+ <![CDATA[
+LogicalProject(T=[CASE(<(CAST(CASE(>($1, 'abc'), $1, null:VARCHAR(20))):DOUBLE, 5), 0.0:DOUBLE, CASE(IS NOT NULL(CAST(CASE(>($1, 'abc'), $1, null:VARCHAR(20))):DOUBLE), CAST(CAST(CASE(>($1, 'abc'), $1, null:VARCHAR(20))):DOUBLE):DOUBLE NOT NULL, 1.0:DOUBLE))])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ </TestCase>
<TestCase name="testCorrelationScalarAggAndFilter">
<Resource name="sql">
<![CDATA[SELECT e1.empno
@@ -12485,7 +12511,9 @@
</TestCase>
<TestCase name="testSwapOuterJoinFieldAccess">
<Resource name="sql">
- <![CDATA[select t1.name, e.ename from DEPT_NESTED as t1 left outer join sales.emp e on t1.skill.type = e.job]]>
+ <![CDATA[select t1.name, e.ename
+from DEPT_NESTED as t1 left outer join sales.emp e
+ on t1.skill.type = e.job]]>
</Resource>
<Resource name="planBefore">
<