Skip to content

[CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified #5184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rubenada wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified #5184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4a3058c
[CALCITE-7722] Simplify IS NULL / IS NOT NULL expressions by removing…
rubenada Aug 17, 2026
2953c94
Improve tests, simplify comments
rubenada Aug 18, 2026
52811f0
Consider isSafe on RexCall isAlwaysTrue / isAlwaysFalse simplificatio…
rubenada Aug 18, 2026
73e650f
Add clarification comment on test
rubenada Aug 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions core/src/main/java/org/apache/calcite/rex/RexCall.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down
65 changes: 51 additions & 14 deletions core/src/main/java/org/apache/calcite/rex/RexSimplify.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1197,15 +1197,21 @@
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);
Expand All @@ -1220,6 +1226,9 @@
}
return RexUtil.composeConjunction(rexBuilder, operands);
case CUSTOM:
if (!isSafe) {
return simplifiedResult;
}
switch (a.getKind()) {
case LITERAL:
return rexBuilder.makeLiteral(!((RexLiteral) a).isNull());
Expand All @@ -1233,7 +1242,7 @@
}
}

private @Nullable RexNode simplifyIsNull(RexNode a) {

Check failure on line 1245 in core/src/main/java/org/apache/calcite/rex/RexSimplify.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 16 to the 15 allowed. 


See more on https://sonarcloud.io/project/issues?id=apache_calcite&issues=AaAQRCbKPUuIH4yO-G6x&open=AaAQRCbKPUuIH4yO-G6x&pullRequest=5184
// Simplify the argument first,
// call ourselves recursively to see whether we can make more progress.
// For example, given
Expand All @@ -1258,15 +1267,18 @@
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);
Expand Down Expand Up @@ -1596,16 +1608,22 @@
}

@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();

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);
Expand All @@ -1615,7 +1633,7 @@
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;
Expand All @@ -1625,12 +1643,31 @@
|| 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;
}
Expand Down
34 changes: 34 additions & 0 deletions core/src/test/java/org/apache/calcite/rex/RexProgramBuilderBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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

/**
Expand Down
123 changes: 123 additions & 0 deletions core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -3024,6 +3025,128 @@ 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
// 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)");
Comment thread
thomasrebele marked this conversation as resolved.
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 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())),

@mihaibudiu mihaibudiu Aug 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we have some tests using checked arithmetic or arithmetic on intervals?

@rubenada rubenada Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point; added both cases

"IS NULL(/(?0.notNullInt0, 0))");
}

@Test void testPushNotIntoCase() {
checkSimplify(
not(
Expand Down
Loading