From 4a3058c337ec90e388d0ca0399ea293f1b8f24cc Mon Sep 17 00:00:00 2001 From: Ruben Quesada Lopez Date: Mon, 17 Aug 2026 15:21:37 +0100 Subject: [PATCH 1/5] [CALCITE-7722] Simplify IS NULL / IS NOT NULL expressions by removing operations if they do not affect the nullability --- .../org/apache/calcite/rex/RexSimplify.java | 96 ++++++++++++++++--- .../apache/calcite/rex/RexProgramTest.java | 76 +++++++++++++++ 2 files changed, 159 insertions(+), 13 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 386964963ce..59d957dbf5d 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java +++ b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java @@ -1197,15 +1197,40 @@ 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 inside + 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 (each is either recursively + // simplified, or rewrapped verbatim as IS NOT NULL(operand)), so it only + // needs SHALLOW safety of the outer operator. Requiring full-tree safety + // here suppresses common peels like (CAST(str):DOUBLE + 1.0) IS NOT NULL + // → CAST(str):DOUBLE IS NOT NULL just because a non-lossless CAST lives + // deeper in the tree, even though that CAST would still be evaluated + // inside the rewrapped IS NOT NULL(operand). + if (!SafeRexVisitor.INSTANCE.isShallowSafe(a)) { + return simplifiedResult; + } + // The peel only preserves runtime semantics when each operand can be + // safely rewrapped as IS_NOT_NULL(operand) without losing the + // "may throw" signal. If an operand is typed non-nullable BUT is not + // fully safe (e.g. its subtree still contains a divide-by-zero), the + // trivial RexCall.isAlwaysTrue() shortcut on IS_NOT_NULL of a + // non-nullable operand would collapse the wrap to TRUE and hide the + // throw. Fall back to shallow-safety-first: allow the peel only if + // every operand is either nullable or itself fully safe. + for (RexNode operand : ((RexCall) a).getOperands()) { + if (!operand.getType().isNullable() && !isSafeExpression(operand)) { + return simplifiedResult; + } + } final List operands = new ArrayList<>(); for (RexNode operand : ((RexCall) a).getOperands()) { final RexNode simplified = simplifyIsNotNull(operand); @@ -1220,6 +1245,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()); @@ -1258,15 +1286,32 @@ 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 inside + 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" + // to "operand0 IS NULL OR operand1 IS NULL". This branch PRESERVES the + // operand subtrees (each is either recursively simplified, or rewrapped + // verbatim as IS NULL(operand)), so it only needs SHALLOW safety of the + // outer operator; see the symmetric comment in simplifyIsNotNull. + if (!SafeRexVisitor.INSTANCE.isShallowSafe(a)) { + return simplifiedResult; + } + // See symmetric comment in simplifyIsNotNull: RexCall.isAlwaysFalse() + // collapses IS_NULL(op) to FALSE whenever op's type is non-nullable, + // which would hide runtime throws inside op. Require each operand + // to be nullable OR fully safe before distributing. + for (RexNode operand : ((RexCall) a).getOperands()) { + if (!operand.getType().isNullable() && !isSafeExpression(operand)) { + return simplifiedResult; + } + } final List operands = new ArrayList<>(); for (RexNode operand : ((RexCall) a).getOperands()) { final RexNode simplified = simplifyIsNull(operand); @@ -1596,6 +1641,10 @@ enum SafeRexVisitor implements RexVisitor { } @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(); @@ -1603,9 +1652,11 @@ enum SafeRexVisitor implements RexVisitor { case DIVIDE: case MOD: List 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); @@ -1615,7 +1666,7 @@ enum SafeRexVisitor implements RexVisitor { 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; @@ -1625,12 +1676,31 @@ enum SafeRexVisitor implements RexVisitor { || 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}. + * + *

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/RexProgramTest.java b/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java index 332f1865a73..98eec0c7400 100644 --- a/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java +++ b/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java @@ -3024,6 +3024,82 @@ trueLiteral, literal(1), checkSimplifyUnchanged(div(cast(vVarchar(), tInt(false)), nullInt)); } + /** Test cases for peeling {@code IS [NOT] NULL} across a strong outer + * operator whose subtree contains a non-lossless {@code CAST}. + * + *

Distributing {@code IS [NOT] NULL} across a strong operator (e.g. + * {@code +}, {@code *}) preserves subtree evaluation: each operand is + * either recursively simplified or rewrapped verbatim as + * {@code IS [NOT] NULL(operand)}, so the presence of a non-lossless + * {@code CAST} deeper in the tree must not block the distribution. */ + @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)"); + + // Nested PLUS on both sides: distribution still peels one layer + // and stops at the inner CAST, which is not shallow-safe. + // "((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)"); + } + + /** The distribution must still be suppressed when the outer node is + * itself not shallow-safe (e.g. DIVIDE by a literal zero) or when an + * operand is typed non-nullable yet not fully safe, otherwise + * {@link org.apache.calcite.rex.RexCall#isAlwaysTrue()} would collapse + * the rewrapped {@code IS NOT NULL(operand)} to {@code TRUE} and hide + * the runtime throw. */ + @Test void testSimplifyIsNotNullDoesNotDistributeAcrossUnsafeOuter() { + // The outer PLUS is shallow-safe, but the div(1, 0) operand is typed + // non-nullable, so the peel would rewrap it as IS NOT NULL(/(1, 0)) + // which the trivial isAlwaysTrue() shortcut would collapse to TRUE + // and lose the throw. The peel is therefore suppressed. + checkSimplifyUnchanged(isNotNull(plus(div(literal(1), literal(0)), vIntNotNull()))); + checkSimplifyUnchanged(isNull(plus(div(literal(1), literal(0)), vIntNotNull()))); + + // IS NOT NULL(x/0) itself is not peeled, because DIVIDE with a + // literal-zero divisor is not shallow-safe (this branch would + // otherwise drop the throwing subexpression). + 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 + // shallow-safe, so no further distribution occurs. + checkSimplify(isNull(cast(div(vIntNotNull(), literal(0)), tBigInt())), + "IS NULL(/(?0.notNullInt0, 0))"); + } + @Test void testPushNotIntoCase() { checkSimplify( not( From 2953c94ad93e187ff32ab3e4268726cb180c1f17 Mon Sep 17 00:00:00 2001 From: Ruben Quesada Lopez Date: Tue, 18 Aug 2026 10:09:14 +0100 Subject: [PATCH 2/5] Improve tests, simplify comments --- .../org/apache/calcite/rex/RexSimplify.java | 36 ++------ .../calcite/rex/RexProgramBuilderBase.java | 34 ++++++++ .../apache/calcite/rex/RexProgramTest.java | 85 +++++++++++-------- 3 files changed, 94 insertions(+), 61 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 59d957dbf5d..1ebaa5e388c 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java +++ b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java @@ -1199,33 +1199,23 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) { } switch (Strong.policy(a)) { case NOT_NULL: - // Drops the subtree; require full-tree safety so we don't hide runtime - // errors inside + // 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". - // This branch PRESERVES the operand subtrees (each is either recursively - // simplified, or rewrapped verbatim as IS NOT NULL(operand)), so it only - // needs SHALLOW safety of the outer operator. Requiring full-tree safety - // here suppresses common peels like (CAST(str):DOUBLE + 1.0) IS NOT NULL - // → CAST(str):DOUBLE IS NOT NULL just because a non-lossless CAST lives - // deeper in the tree, even though that CAST would still be evaluated - // inside the rewrapped IS NOT NULL(operand). + // 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; } // The peel only preserves runtime semantics when each operand can be // safely rewrapped as IS_NOT_NULL(operand) without losing the - // "may throw" signal. If an operand is typed non-nullable BUT is not - // fully safe (e.g. its subtree still contains a divide-by-zero), the - // trivial RexCall.isAlwaysTrue() shortcut on IS_NOT_NULL of a - // non-nullable operand would collapse the wrap to TRUE and hide the - // throw. Fall back to shallow-safety-first: allow the peel only if - // every operand is either nullable or itself fully safe. + // "may throw" signal: allow the peel only if every operand is either + // nullable or itself fully safe. for (RexNode operand : ((RexCall) a).getOperands()) { if (!operand.getType().isNullable() && !isSafeExpression(operand)) { return simplifiedResult; @@ -1288,25 +1278,17 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) { } switch (Strong.policy(a)) { case NOT_NULL: - // Drops the subtree; require full-tree safety so we don't hide runtime - // errors inside + // 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". This branch PRESERVES the - // operand subtrees (each is either recursively simplified, or rewrapped - // verbatim as IS NULL(operand)), so it only needs SHALLOW safety of the - // outer operator; see the symmetric comment in simplifyIsNotNull. + // See symmetric comment in simplifyIsNotNull if (!SafeRexVisitor.INSTANCE.isShallowSafe(a)) { return simplifiedResult; } - // See symmetric comment in simplifyIsNotNull: RexCall.isAlwaysFalse() - // collapses IS_NULL(op) to FALSE whenever op's type is non-nullable, - // which would hide runtime throws inside op. Require each operand - // to be nullable OR fully safe before distributing. + // See symmetric comment in simplifyIsNotNull for (RexNode operand : ((RexCall) a).getOperands()) { if (!operand.getType().isNullable() && !isSafeExpression(operand)) { return simplifiedResult; 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 4b873fce5c8..cc48e387fbb 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 /** 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 98eec0c7400..d54058b9159 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,26 +3025,21 @@ trueLiteral, literal(1), checkSimplifyUnchanged(div(cast(vVarchar(), tInt(false)), nullInt)); } - /** Test cases for peeling {@code IS [NOT] NULL} across a strong outer - * operator whose subtree contains a non-lossless {@code CAST}. - * - *

Distributing {@code IS [NOT] NULL} across a strong operator (e.g. - * {@code +}, {@code *}) preserves subtree evaluation: each operand is - * either recursively simplified or rewrapped verbatim as - * {@code IS [NOT] NULL(operand)}, so the presence of a non-lossless - * {@code CAST} deeper in the tree must not block the distribution. */ + /** + * Test cases for [CALCITE-7722] + * RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands + * can be further simplified. + */ @Test void testSimplifyIsNotNullDistributesAcrossStrongOpWithLossyCast() { - // "(CAST(?0.varchar0):INTEGER + 1) IS NOT NULL" - // ==> "IS NOT NULL(CAST(?0.varchar0):INTEGER)" + // "(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. + // 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)" + // "(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)"); @@ -3052,7 +3048,7 @@ trueLiteral, literal(1), 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. + // MULTIPLY is also strong + shallow-safe checkSimplify( isNotNull(mul(cast(vVarchar(), tInt(true)), literal(2))), "IS NOT NULL(CAST(?0.varchar0):INTEGER)"); @@ -3062,40 +3058,61 @@ trueLiteral, literal(1), checkSimplify(isNotNull(mul(vInt(), literal(2))), "IS NOT NULL(?0.int0)"); checkSimplify(isNull(mul(vInt(), literal(2))), "IS NULL(?0.int0)"); - // Nested PLUS on both sides: distribution still peels one layer - // and stops at the inner CAST, which is not shallow-safe. + // 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)"); - } - /** The distribution must still be suppressed when the outer node is - * itself not shallow-safe (e.g. DIVIDE by a literal zero) or when an - * operand is typed non-nullable yet not fully safe, otherwise - * {@link org.apache.calcite.rex.RexCall#isAlwaysTrue()} would collapse - * the rewrapped {@code IS NOT NULL(operand)} to {@code TRUE} and hide - * the runtime throw. */ - @Test void testSimplifyIsNotNullDoesNotDistributeAcrossUnsafeOuter() { - // The outer PLUS is shallow-safe, but the div(1, 0) operand is typed - // non-nullable, so the peel would rewrap it as IS NOT NULL(/(1, 0)) - // which the trivial isAlwaysTrue() shortcut would collapse to TRUE - // and lose the throw. The peel is therefore suppressed. + // Operators with checked arithmetic + checkSimplify( + isNotNull(checkedPlus(cast(vVarchar(), tInt(true)), literal(1))), + "IS NOT NULL(CAST(?0.varchar0):INTEGER)"); + checkSimplify( + isNull(checkedPlus(cast(vVarchar(), tInt(true)), literal(1))), + "IS NULL(CAST(?0.varchar0):INTEGER)"); + checkSimplify( + isNotNull(checkedMul(cast(vVarchar(), tInt(true)), literal(2))), + "IS NOT NULL(CAST(?0.varchar0):INTEGER)"); + checkSimplify( + isNull(checkedMul(cast(vVarchar(), tInt(true)), literal(2))), + "IS NULL(CAST(?0.varchar0):INTEGER)"); + + // Arithmetic on INTERVAL + checkSimplify( + isNotNull(plus(cast(vVarchar(), tDate(true)), interval(10, TimeUnit.DAY))), + "IS NOT NULL(CAST(?0.varchar0):DATE)"); + checkSimplify( + isNull(plus(cast(vVarchar(), tDate(true)), interval(1, TimeUnit.MONTH))), + "IS NULL(CAST(?0.varchar0):DATE)"); + + // The outer PLUS is shallow-safe, but the div(1, 0) is not, so the peel is therefore suppressed checkSimplifyUnchanged(isNotNull(plus(div(literal(1), literal(0)), vIntNotNull()))); checkSimplifyUnchanged(isNull(plus(div(literal(1), literal(0)), vIntNotNull()))); - // IS NOT NULL(x/0) itself is not peeled, because DIVIDE with a - // literal-zero divisor is not shallow-safe (this branch would - // otherwise drop the throwing subexpression). + // 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 - // shallow-safe, so no further distribution occurs. + // 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))"); } From 52811f01d8e1a2c6e1b016d211ad06eb09eda1e5 Mon Sep 17 00:00:00 2001 From: Ruben Quesada Lopez Date: Tue, 18 Aug 2026 11:04:35 +0100 Subject: [PATCH 3/5] Consider isSafe on RexCall isAlwaysTrue / isAlwaysFalse simplification for IS [NOT] NULL --- .../java/org/apache/calcite/rex/RexCall.java | 4 +-- .../org/apache/calcite/rex/RexSimplify.java | 15 --------- .../apache/calcite/rex/RexProgramTest.java | 32 +++++++++++++++++-- 3 files changed, 31 insertions(+), 20 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/rex/RexCall.java b/core/src/main/java/org/apache/calcite/rex/RexCall.java index deac203607d..b2653f2fe67 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexCall.java +++ b/core/src/main/java/org/apache/calcite/rex/RexCall.java @@ -227,7 +227,7 @@ private boolean digestWithType() { } switch (getKind()) { case IS_NOT_NULL: - return !operands.get(0).getType().isNullable(); + return RexSimplify.isSafeExpression(this) && !operands.get(0).getType().isNullable(); case IS_NOT_TRUE: case IS_FALSE: case NOT: @@ -253,7 +253,7 @@ private boolean digestWithType() { } switch (getKind()) { case IS_NULL: - return !operands.get(0).getType().isNullable(); + return RexSimplify.isSafeExpression(this) && !operands.get(0).getType().isNullable(); case IS_NOT_TRUE: case IS_FALSE: case NOT: 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 1ebaa5e388c..ac31ceca939 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java +++ b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java @@ -1212,15 +1212,6 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) { if (!SafeRexVisitor.INSTANCE.isShallowSafe(a)) { return simplifiedResult; } - // The peel only preserves runtime semantics when each operand can be - // safely rewrapped as IS_NOT_NULL(operand) without losing the - // "may throw" signal: allow the peel only if every operand is either - // nullable or itself fully safe. - for (RexNode operand : ((RexCall) a).getOperands()) { - if (!operand.getType().isNullable() && !isSafeExpression(operand)) { - return simplifiedResult; - } - } final List operands = new ArrayList<>(); for (RexNode operand : ((RexCall) a).getOperands()) { final RexNode simplified = simplifyIsNotNull(operand); @@ -1288,12 +1279,6 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) { if (!SafeRexVisitor.INSTANCE.isShallowSafe(a)) { return simplifiedResult; } - // See symmetric comment in simplifyIsNotNull - for (RexNode operand : ((RexCall) a).getOperands()) { - if (!operand.getType().isNullable() && !isSafeExpression(operand)) { - return simplifiedResult; - } - } final List operands = new ArrayList<>(); for (RexNode operand : ((RexCall) a).getOperands()) { final RexNode simplified = simplifyIsNull(operand); 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 d54058b9159..a3a98a55b43 100644 --- a/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java +++ b/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java @@ -3102,9 +3102,35 @@ trueLiteral, literal(1), isNull(plus(cast(vVarchar(), tDate(true)), interval(1, TimeUnit.MONTH))), "IS NULL(CAST(?0.varchar0):DATE)"); - // The outer PLUS is shallow-safe, but the div(1, 0) is not, so the peel is therefore suppressed - checkSimplifyUnchanged(isNotNull(plus(div(literal(1), literal(0)), vIntNotNull()))); - checkSimplifyUnchanged(isNull(plus(div(literal(1), literal(0)), vIntNotNull()))); + // 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)))); From 73e650fa70afa8bb803dcfebf75526664c6ab08f Mon Sep 17 00:00:00 2001 From: Ruben Quesada Lopez Date: Tue, 18 Aug 2026 13:29:12 +0100 Subject: [PATCH 4/5] Add clarification comment on test --- core/src/test/java/org/apache/calcite/rex/RexProgramTest.java | 4 ++++ 1 file changed, 4 insertions(+) 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 a3a98a55b43..8e3e76f23f2 100644 --- a/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java +++ b/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java @@ -3081,6 +3081,10 @@ trueLiteral, literal(1), "IS NOT NULL(CAST(?0.varchar0):INTEGER)"); // Operators with checked arithmetic + // WARNING: these simplifications are a bit "controversial" since checked operators + // can throw at runtime (in case of overflow); however, at the moment they are considered + // "safe" by RexSimplify#SafeRexVisitor, so these simplifications are applied; if this + // gets reviewed in the future (see CALCITE-7725) these tests might need to get adjusted checkSimplify( isNotNull(checkedPlus(cast(vVarchar(), tInt(true)), literal(1))), "IS NOT NULL(CAST(?0.varchar0):INTEGER)"); From 315d9182ccaa8144f9d3c6b64ee34117e2ed9803 Mon Sep 17 00:00:00 2001 From: Ruben Quesada Lopez Date: Wed, 19 Aug 2026 18:07:36 +0100 Subject: [PATCH 5/5] Add more tests --- .../calcite/rex/RexProgramBuilderBase.java | 44 +++++++++++++++++++ .../apache/calcite/rex/RexProgramTest.java | 30 ++++++++++++- 2 files changed, 73 insertions(+), 1 deletion(-) 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 cc48e387fbb..53b0f542688 100644 --- a/core/src/test/java/org/apache/calcite/rex/RexProgramBuilderBase.java +++ b/core/src/test/java/org/apache/calcite/rex/RexProgramBuilderBase.java @@ -826,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 8e3e76f23f2..967e5c8a681 100644 --- a/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java +++ b/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java @@ -3098,13 +3098,30 @@ trueLiteral, literal(1), isNull(checkedMul(cast(vVarchar(), tInt(true)), literal(2))), "IS NULL(CAST(?0.varchar0):INTEGER)"); - // Arithmetic on INTERVAL + // Arithmetic on DATE and INTERVAL + checkSimplify( + isNotNull(sub(vDate(), cast(vVarchar(), tDate(true)))), + "AND(IS NOT NULL(?0.date0), IS NOT NULL(CAST(?0.varchar0):DATE))"); checkSimplify( isNotNull(plus(cast(vVarchar(), tDate(true)), interval(10, TimeUnit.DAY))), "IS NOT NULL(CAST(?0.varchar0):DATE)"); checkSimplify( isNull(plus(cast(vVarchar(), tDate(true)), interval(1, TimeUnit.MONTH))), "IS NULL(CAST(?0.varchar0):DATE)"); + checkSimplify( + isNull( + plus( + plus(cast(vVarchar(), tDate(true)), interval(1, TimeUnit.MONTH)), + interval(10, TimeUnit.DAY))), + "IS NULL(CAST(?0.varchar0):DATE)"); + checkSimplify( + 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)))), + "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( @@ -3145,6 +3162,17 @@ trueLiteral, literal(1), // 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)"); } @Test void testPushNotIntoCase() {