This is an automated email from the ASF dual-hosted git repository.
rubenada pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new 95101d8255 [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator
with Strong policy ANY and unsafe operands can be further simplified
95101d8255 is described below
commit 95101d8255cad4a0e92f25ec00797bb16338cfb3
Author: Ruben Quesada Lopez <[email protected]>
AuthorDate: Mon Aug 17 15:21:37 2026 +0100
[CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong
policy ANY and unsafe operands can be further simplified
---
.../java/org/apache/calcite/rex/RexSimplify.java | 74 ++++++++---
.../apache/calcite/rex/RexProgramBuilderBase.java | 78 +++++++++++
.../org/apache/calcite/rex/RexProgramTest.java | 146 +++++++++++++++++++++
3 files changed, 282 insertions(+), 16 deletions(-)
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 fe9f7b5ca0..98fbbb781b 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
@@ -1200,15 +1200,21 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs
unknownAs) {
if (hasCustomNullabilityRules(a.getKind())) {
return simplifiedResult;
}
- if (!isSafe) {
- return simplifiedResult;
- }
switch (Strong.policy(a)) {
case NOT_NULL:
+ // Drops the subtree; require full-tree safety so we don't hide runtime
errors
+ if (!isSafe) {
+ return simplifiedResult;
+ }
return rexBuilder.makeLiteral(true);
case ANY:
// "f" is a strong operator, so "f(operand0, operand1) IS NOT NULL"
- // simplifies to "operand0 IS NOT NULL AND operand1 IS NOT NULL"
+ // simplifies to "operand0 IS NOT NULL AND operand1 IS NOT NULL";
+ // this branch PRESERVES the operand subtrees, so it only
+ // needs SHALLOW safety of the outer operator
+ if (!SafeRexVisitor.INSTANCE.isShallowSafe(a)) {
+ return simplifiedResult;
+ }
final List<RexNode> operands = new ArrayList<>();
for (RexNode operand : ((RexCall) a).getOperands()) {
final RexNode simplified = simplifyIsNotNull(operand);
@@ -1223,6 +1229,9 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs
unknownAs) {
}
return RexUtil.composeConjunction(rexBuilder, operands);
case CUSTOM:
+ if (!isSafe) {
+ return simplifiedResult;
+ }
switch (a.getKind()) {
case LITERAL:
return rexBuilder.makeLiteral(!((RexLiteral) a).isNull());
@@ -1261,15 +1270,18 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs
unknownAs) {
if (hasCustomNullabilityRules(a.getKind())) {
return simplifiedResult;
}
- if (!isSafe) {
- return simplifiedResult;
- }
switch (Strong.policy(a)) {
case NOT_NULL:
+ // Drops the subtree; require full-tree safety so we don't hide runtime
errors
+ if (!isSafe) {
+ return simplifiedResult;
+ }
return rexBuilder.makeLiteral(false);
case ANY:
- // "f" is a strong operator, so "f(operand0, operand1) IS NULL"
simplifies
- // to "operand0 IS NULL OR operand1 IS NULL"
+ // See symmetric comment in simplifyIsNotNull
+ if (!SafeRexVisitor.INSTANCE.isShallowSafe(a)) {
+ return simplifiedResult;
+ }
final List<RexNode> operands = new ArrayList<>();
for (RexNode operand : ((RexCall) a).getOperands()) {
final RexNode simplified = simplifyIsNull(operand);
@@ -1595,23 +1607,34 @@ enum SafeRexVisitor implements RexVisitor<Boolean> {
}
@Override public Boolean visitCall(RexCall call) {
+ return isSafe(call, true);
+ }
+
+ private boolean isSafe(RexCall call, boolean deep) {
SqlKind sqlKind = call.getKind();
SqlOperator sqlOperator = call.getOperator();
if (SqlKind.CHECKED_ARITHMETIC.contains(sqlKind)) {
// Checked arithmetic throws on overflow, so it is only safe when the
// arithmetic is never performed, i.e. when an operand is NULL.
- return RexVisitorImpl.visitArrayAnd(this, call.operands)
- && call.operands.stream().anyMatch(o -> RexUtil.isNullLiteral(o,
true));
+ if (deep) {
+ boolean areOperandsSafe = RexVisitorImpl.visitArrayAnd(this,
call.operands);
+ if (!areOperandsSafe) {
+ return false;
+ }
+ }
+ return call.operands.stream().anyMatch(o -> RexUtil.isNullLiteral(o,
true));
}
switch (sqlKind) {
case DIVIDE:
case MOD:
List<RexNode> operands = call.getOperands();
- boolean areOperandsSafe = RexVisitorImpl.visitArrayAnd(this,
call.operands);
- if (!areOperandsSafe) {
- return false;
+ if (deep) {
+ boolean areOperandsSafe = RexVisitorImpl.visitArrayAnd(this,
call.operands);
+ if (!areOperandsSafe) {
+ return false;
+ }
}
boolean hasNullOperand = RexUtil.isNullLiteral(operands.get(0), true)
|| RexUtil.isNullLiteral(operands.get(1), true);
@@ -1621,7 +1644,7 @@ enum SafeRexVisitor implements RexVisitor<Boolean> {
if (operands.get(1) instanceof RexLiteral) {
return !checkLiteralValue(operands.get(1), BigDecimal.ZERO);
}
- // the safety of division could not be deduced, so assume it is unsafe
+ // the safety of MOD / DIVIDE could not be deduced, so assume it is
unsafe
return false;
default:
break;
@@ -1631,12 +1654,31 @@ enum SafeRexVisitor implements RexVisitor<Boolean> {
|| RexUtil.isLosslessCast(call)
|| safeOps.contains(sqlKind)
|| safeOperators.contains(sqlOperator)) {
- return RexVisitorImpl.visitArrayAnd(this, call.operands);
+ return !deep || RexVisitorImpl.visitArrayAnd(this, call.operands);
}
return false;
}
+ /**
+ * Shallow variant of the visitor: reports whether the OUTER node's
+ * operator can be evaluated on non-null operands without throwing at
+ * runtime. Unlike {@link #visitCall(RexCall)}, it does not recurse into
+ * the operands. Callers that only need to know whether the outer
+ * operator itself is safe (e.g. RexSimplify's {@code Strong.ANY}
+ * distribution branches, which preserve subtree evaluation) can use
+ * this in place of the full-tree {@link RexSimplify#isSafeExpression}.
+ *
+ * <p>Non-{@link RexCall} nodes are always shallow-safe (they cannot
+ * throw at their own level).
+ */
+ boolean isShallowSafe(RexNode node) {
+ if (!(node instanceof RexCall)) {
+ return true;
+ }
+ return isSafe((RexCall) node, false);
+ }
+
@Override public Boolean visitOver(RexOver over) {
return false;
}
diff --git
a/core/src/test/java/org/apache/calcite/rex/RexProgramBuilderBase.java
b/core/src/test/java/org/apache/calcite/rex/RexProgramBuilderBase.java
index 4b873fce5c..53b0f54268 100644
--- a/core/src/test/java/org/apache/calcite/rex/RexProgramBuilderBase.java
+++ b/core/src/test/java/org/apache/calcite/rex/RexProgramBuilderBase.java
@@ -19,14 +19,17 @@
import org.apache.calcite.DataContext;
import org.apache.calcite.DataContexts;
import org.apache.calcite.adapter.java.JavaTypeFactory;
+import org.apache.calcite.avatica.util.TimeUnit;
import org.apache.calcite.jdbc.JavaTypeFactoryImpl;
import org.apache.calcite.plan.RelOptPredicateList;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeFactory;
import org.apache.calcite.rel.type.RelDataTypeSystem;
+import org.apache.calcite.sql.SqlIntervalQualifier;
import org.apache.calcite.sql.fun.SqlInternalOperators;
import org.apache.calcite.sql.fun.SqlLibraryOperators;
import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.parser.SqlParserPos;
import org.apache.calcite.sql.type.SqlTypeName;
import com.google.common.collect.ImmutableList;
@@ -65,6 +68,7 @@ public abstract class RexProgramBuilderBase {
protected RexLiteral nullReal;
protected RexLiteral nullDouble;
protected RexLiteral nullVarbinary;
+ protected RexLiteral nullDate;
private RelDataType nullableBool;
private RelDataType nonNullableBool;
@@ -90,6 +94,9 @@ public abstract class RexProgramBuilderBase {
private RelDataType nullableVarbinary;
private RelDataType nonNullableVarbinary;
+ private RelDataType nullableDate;
+ private RelDataType nonNullableDate;
+
// Note: JUnit 4 creates new instance for each test method,
// so we initialize these structures on demand
// It maps non-nullable type to struct of (10 nullable, 10 non-nullable)
fields
@@ -142,6 +149,10 @@ public abstract class RexProgramBuilderBase {
nonNullableVarbinary = typeFactory.createSqlType(SqlTypeName.VARBINARY);
nullableVarbinary =
typeFactory.createTypeWithNullability(nonNullableVarbinary, true);
nullVarbinary = rexBuilder.makeNullLiteral(nullableVarbinary);
+
+ nonNullableDate = typeFactory.createSqlType(SqlTypeName.DATE);
+ nullableDate = typeFactory.createTypeWithNullability(nonNullableDate,
true);
+ nullDate = rexBuilder.makeNullLiteral(nullableDate);
}
private RexDynamicParam getDynamicParam(RelDataType type, String
fieldNamePrefix) {
@@ -324,6 +335,14 @@ protected RexNode mul(RexNode n1, RexNode n2) {
return rexBuilder.makeCall(SqlStdOperatorTable.MULTIPLY, n1, n2);
}
+ protected RexNode checkedPlus(RexNode n1, RexNode n2) {
+ return rexBuilder.makeCall(SqlStdOperatorTable.CHECKED_PLUS, n1, n2);
+ }
+
+ protected RexNode checkedMul(RexNode n1, RexNode n2) {
+ return rexBuilder.makeCall(SqlStdOperatorTable.CHECKED_MULTIPLY, n1, n2);
+ }
+
protected RexNode coalesce(RexNode... nodes) {
return rexBuilder.makeCall(SqlStdOperatorTable.COALESCE, nodes);
}
@@ -484,6 +503,14 @@ protected RelDataType tVarbinary(boolean nullable) {
return nullable ? nullableVarbinary : nonNullableVarbinary;
}
+ protected RelDataType tDate() {
+ return nonNullableDate;
+ }
+
+ protected RelDataType tDate(boolean nullable) {
+ return nullable ? nullableDate : nonNullableDate;
+ }
+
protected RelDataType tArray(RelDataType elemType) {
return typeFactory.createArrayType(elemType, -1);
@@ -549,6 +576,13 @@ protected RexLiteral literalVarchar(String value) {
protected RexLiteral literal(double value) {
return rexBuilder.makeApproxLiteral(value, nonNullableDouble);
}
+
+ protected RexLiteral interval(int value, TimeUnit timeUnit) {
+ return rexBuilder.makeIntervalLiteral(
+ BigDecimal.valueOf(value),
+ new SqlIntervalQualifier(timeUnit, null, SqlParserPos.ZERO));
+ }
+
// Variables
/**
@@ -792,6 +826,50 @@ protected RexNode vDecimalNotNull(int arg) {
return vParamNotNull("decimal", arg, nonNullableDecimal);
}
+ /**
+ * Creates {@code nullable date variable} with index of 0.
+ * If you need several distinct variables, use {@link #vDate(int)}.
+ * The resulting node would look like {@code ?0.date0}
+ *
+ * @return nullable date with index of 0
+ */
+ protected RexNode vDate() {
+ return vDate(0);
+ }
+
+ /**
+ * Creates {@code nullable date variable} with index of {@code arg}
(0-based).
+ * The resulting node would look like {@code ?0.date3} if {@code arg} is
{@code 3}.
+ *
+ * @param arg argument index (0-based)
+ * @return nullable date variable with given index (0-based)
+ */
+ protected RexNode vDate(int arg) {
+ return vParam("date", arg, nullableDate);
+ }
+
+ /**
+ * Creates {@code non-nullable date variable} with index of 0.
+ * If you need several distinct variables, use {@link #vDateNotNull(int)}.
+ * The resulting node would look like {@code ?0.notNullDate0}
+ *
+ * @return non-nullable date variable with index of 0
+ */
+ protected RexNode vDateNotNull() {
+ return vDateNotNull(0);
+ }
+
+ /**
+ * Creates {@code non-nullable date variable} with index of {@code arg}
(0-based).
+ * The resulting node would look like {@code ?0.notNullDate3} if {@code arg}
is {@code 3}.
+ *
+ * @param arg argument index (0-based)
+ * @return non-nullable date variable with given index (0-based)
+ */
+ protected RexNode vDateNotNull(int arg) {
+ return vParamNotNull("date", arg, nonNullableDate);
+ }
+
/**
* Creates {@code nullable variable} with given type and name of {@code arg}
(0-based).
* This enables cases when type is built dynamically.
diff --git a/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
b/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
index ca063c4855..0dbbcd9cde 100644
--- a/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
+++ b/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
@@ -17,6 +17,7 @@
package org.apache.calcite.rex;
import org.apache.calcite.avatica.util.ByteString;
+import org.apache.calcite.avatica.util.TimeUnit;
import org.apache.calcite.plan.RelOptPredicateList;
import org.apache.calcite.plan.RelOptUtil;
import org.apache.calcite.plan.Strong;
@@ -3024,6 +3025,151 @@ trueLiteral, literal(1),
checkSimplifyUnchanged(div(cast(vVarchar(), tInt(false)), nullInt));
}
+ /**
+ * Test cases for <a
href="https://issues.apache.org/jira/browse/CALCITE-7722";>[CALCITE-7722]
+ * RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and
unsafe operands
+ * can be further simplified</a>.
+ */
+ @Test void testSimplifyIsNotNullDistributesAcrossStrongOpWithLossyCast() {
+ // "(CAST(?0.varchar0):INTEGER + 1) IS NOT NULL" ==> "IS NOT
NULL(CAST(?0.varchar0):INTEGER)"
+ // The outer PLUS is strong AND shallow-safe; distribution keeps the
+ // non-lossless CAST inside the rewrapped IS NOT NULL
+ checkSimplify(
+ isNotNull(plus(cast(vVarchar(), tInt(true)), literal(1))),
+ "IS NOT NULL(CAST(?0.varchar0):INTEGER)");
+
+ // Symmetric IS NULL peel:
+ // "(CAST(?0.varchar0):INTEGER + 1) IS NULL" ==> "IS
NULL(CAST(?0.varchar0):INTEGER)"
+ checkSimplify(
+ isNull(plus(cast(vVarchar(), tInt(true)), literal(1))),
+ "IS NULL(CAST(?0.varchar0):INTEGER)");
+
+ // Confirm this is consistent with same expression without CAST
+ checkSimplify(isNotNull(plus(vInt(), literal(1))), "IS NOT NULL(?0.int0)");
+ checkSimplify(isNull(plus(vInt(), literal(1))), "IS NULL(?0.int0)");
+
+ // MULTIPLY is also strong + shallow-safe
+ checkSimplify(
+ isNotNull(mul(cast(vVarchar(), tInt(true)), literal(2))),
+ "IS NOT NULL(CAST(?0.varchar0):INTEGER)");
+ checkSimplify(
+ isNull(mul(cast(vVarchar(), tInt(true)), literal(2))),
+ "IS NULL(CAST(?0.varchar0):INTEGER)");
+ checkSimplify(isNotNull(mul(vInt(), literal(2))), "IS NOT NULL(?0.int0)");
+ checkSimplify(isNull(mul(vInt(), literal(2))), "IS NULL(?0.int0)");
+
+ // PLUS of two non-lossless CAST
+ checkSimplify(
+ isNotNull(
+ plus(
+ cast(vVarchar(0), tInt(true)),
+ cast(vVarchar(1), tInt(true)))),
+ "AND(IS NOT NULL(CAST(?0.varchar0):INTEGER), IS NOT
NULL(CAST(?0.varchar1):INTEGER))");
+ checkSimplify(
+ isNull(
+ plus(
+ cast(vVarchar(0), tInt(true)),
+ cast(vVarchar(1), tInt(true)))),
+ "OR(IS NULL(CAST(?0.varchar0):INTEGER), IS
NULL(CAST(?0.varchar1):INTEGER))");
+
+ // Nested PLUS:
+ // "((CAST(?0.varchar0):INTEGER + 1) + 2) IS NOT NULL"
+ // ==> "IS NOT NULL(CAST(?0.varchar0):INTEGER)"
+ checkSimplify(
+ isNotNull(
+ plus(plus(cast(vVarchar(), tInt(true)), literal(1)), literal(2))),
+ "IS NOT NULL(CAST(?0.varchar0):INTEGER)");
+
+ // Operators with checked arithmetic: they cannot be peeled because they
are not "safe"
+ // (they will throw at runtime in case of overflow)
+ checkSimplifyUnchanged(
+ isNotNull(checkedPlus(cast(vVarchar(), tInt(true)), literal(1))));
+ checkSimplifyUnchanged(
+ isNull(checkedPlus(cast(vVarchar(), tInt(true)), literal(1))));
+ checkSimplifyUnchanged(
+ isNotNull(checkedMul(cast(vVarchar(), tInt(true)), literal(2))));
+ checkSimplifyUnchanged(
+ isNull(checkedMul(cast(vVarchar(), tInt(true)), literal(2))));
+
+ // Arithmetic on DATE
+ checkSimplify(
+ isNotNull(sub(vDate(), cast(vVarchar(), tDate(true)))),
+ "AND(IS NOT NULL(?0.date0), IS NOT NULL(CAST(?0.varchar0):DATE))");
+
+ // The outer PLUS is shallow-safe, but the div(1, 0) is not, so no further
simplification occurs
+ checkSimplify(
+ isNotNull(plus(div(literal(1), literal(0)), vIntNotNull())),
+ "IS NOT NULL(/(1, 0))");
+ checkSimplify(
+ isNull(plus(div(literal(1), literal(0)), vIntNotNull())),
+ "IS NULL(/(1, 0))");
+
+ // The outer PLUS / MULT is shallow-safe, but the CAST is not
(non-lossless),
+ // so no further simplification occurs
+ checkSimplify(isNull(plus(cast(vVarchar(), tInt(false)), literal(2))),
+ "IS NULL(CAST(?0.varchar0):INTEGER NOT NULL)");
+ checkSimplify(isNull(mul(cast(vVarchar(), tInt(false)), literal(2))),
+ "IS NULL(CAST(?0.varchar0):INTEGER NOT NULL)");
+ checkSimplify(isNotNull(plus(cast(vVarchar(), tInt(false)), literal(2))),
+ "IS NOT NULL(CAST(?0.varchar0):INTEGER NOT NULL)");
+ checkSimplify(isNotNull(mul(cast(vVarchar(), tInt(false)), literal(2))),
+ "IS NOT NULL(CAST(?0.varchar0):INTEGER NOT NULL)");
+
+ // The outer PLUS / MULT is shallow-safe, and the CAST is safe too
(lossless CAST),
+ // so fully simplified
+ checkSimplify(isNull(plus(cast(vSmallInt(), tInt(false)), literal(2))),
+ "false");
+ checkSimplify(isNull(mul(cast(vSmallInt(), tInt(false)), literal(2))),
+ "false");
+ checkSimplify(isNotNull(plus(cast(vSmallInt(), tInt(false)), literal(2))),
+ "true");
+ checkSimplify(isNotNull(mul(cast(vSmallInt(), tInt(false)), literal(2))),
+ "true");
+
+ // IS NOT NULL(x/0) itself is not peeled, because DIVIDE with a
literal-zero divisor is not safe
+ checkSimplifyUnchanged(isNotNull(div(vIntNotNull(), literal(0))));
+ checkSimplifyUnchanged(isNull(div(vIntNotNull(), literal(0))));
+ checkSimplifyUnchanged(isNull(div(cast(vIntNotNull(), tBigInt()),
literal(0))));
+
+ // IS NULL(CAST(10/0 AS BIGINT)) stays as IS NULL(10/0) after the
lossless-CAST strip;
+ // the DIVIDE is not safe, so no further distribution occurs
+ checkSimplify(isNull(cast(div(vIntNotNull(), literal(0)), tBigInt())),
+ "IS NULL(/(?0.notNullInt0, 0))");
+
+ // A bit more complex AND expression:
+ // AND(
+ // CAST(?0.varchar0):INTEGER < 100,
+ // IS NOT NULL(CAST(?0.varchar0):INTEGER + 1))
+ // ===> CAST(?0.varchar0):INTEGER < 100
+ checkSimplifyFilter(
+ and(
+ lt(cast(vVarchar(), tInt(true)), literal(100)),
+ isNotNull(plus(cast(vVarchar(), tInt(true)), literal(1)))),
+ "<(CAST(?0.varchar0):INTEGER, 100)");
+ }
+
+ @Disabled("[CALCITE-7746] Review operation safety on arithmetic on dates and
intervals")
+ @Test void testSimplifyIsNotNullDistributesAcrossStrongOpWithIntervals() {
+ // Arithmetic on DATE and INTERVAL should be considered "unsafe" (since
+ // it can throw at runtime), so simplification should not be applied
+ checkSimplifyUnchanged(
+ isNotNull(plus(cast(vVarchar(), tDate(true)), interval(10,
TimeUnit.DAY))));
+ checkSimplifyUnchanged(
+ isNull(plus(cast(vVarchar(), tDate(true)), interval(1,
TimeUnit.MONTH))));
+ checkSimplifyUnchanged(
+ isNull(
+ plus(
+ plus(cast(vVarchar(), tDate(true)), interval(1,
TimeUnit.MONTH)),
+ interval(10, TimeUnit.DAY))));
+ checkSimplifyUnchanged(
+ and(
+ isNotNull(plus(cast(vVarchar(), tDate(true)), interval(5,
TimeUnit.MONTH))),
+ isNotNull(
+ plus(
+ plus(cast(vVarchar(), tDate(true)), interval(1,
TimeUnit.MONTH)),
+ interval(10, TimeUnit.DAY)))));
+ }
+
@Test void testPushNotIntoCase() {
checkSimplify(
not(
