This is an automated email from the ASF dual-hosted git repository.
mbudiu 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 fd172dbc51 [CALCITE-7070] FILTER_REDUCE_EXPRESSIONS crashes on
expression BETWEEN ( NULL) AND X
fd172dbc51 is described below
commit fd172dbc51043e1d56cdc79fc39f82495b74dc7d
Author: Mihai Budiu <[email protected]>
AuthorDate: Mon Jul 14 15:09:02 2025 -0700
[CALCITE-7070] FILTER_REDUCE_EXPRESSIONS crashes on expression BETWEEN (
NULL) AND X
Signed-off-by: Mihai Budiu <[email protected]>
---
.../apache/calcite/plan/RelOptPredicateList.java | 37 ++++++++++++++++++-
.../calcite/rel/metadata/RelMdPredicates.java | 7 ++--
.../java/org/apache/calcite/rex/RexSimplify.java | 43 +++++++++++++++++++++-
.../main/java/org/apache/calcite/rex/RexUtil.java | 6 +--
.../apache/calcite/sql2rel/SqlToRelConverter.java | 15 +++++++-
.../calcite/rel/rel2sql/RelToSqlConverterTest.java | 2 +-
.../org/apache/calcite/test/RelOptRulesTest.java | 2 +-
.../org/apache/calcite/test/RelOptRulesTest.xml | 35 ++++--------------
8 files changed, 107 insertions(+), 40 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/plan/RelOptPredicateList.java
b/core/src/main/java/org/apache/calcite/plan/RelOptPredicateList.java
index fa1236a31e..30caf49e59 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelOptPredicateList.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelOptPredicateList.java
@@ -18,9 +18,11 @@
import org.apache.calcite.rex.RexBuilder;
import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexLiteral;
import org.apache.calcite.rex.RexNode;
import org.apache.calcite.rex.RexUtil;
import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.util.Litmus;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -45,7 +47,7 @@
* expression that has a predicate {@code y < 10} then the pulled up predicates
* for the Filter are {@code [y < 10, x > 1]}.
*
- * <p><b>Inferred predicates</b> only apply to joins. If there there is a
+ * <p><b>Inferred predicates</b> only apply to joins. If there is a
* predicate on the left input to a join, and that predicate is over columns
* used in the join condition, then a predicate can be inferred on the right
* input to the join. (And vice versa.)
@@ -93,6 +95,8 @@ public class RelOptPredicateList {
* {@link #pulledUpPredicates}. */
public final ImmutableMap<RexNode, RexNode> constantMap;
+ // Please keep this constructor private; if you add additional constructors,
+ // please check the invariants similar to this constructor.
private RelOptPredicateList(ImmutableList<RexNode> pulledUpPredicates,
ImmutableList<RexNode> leftInferredPredicates,
ImmutableList<RexNode> rightInferredPredicates,
@@ -104,6 +108,37 @@ private RelOptPredicateList(ImmutableList<RexNode>
pulledUpPredicates,
this.rightInferredPredicates =
requireNonNull(rightInferredPredicates, "rightInferredPredicates");
this.constantMap = requireNonNull(constantMap, "constantMap");
+
+ // Validate invariants required
+ // (unfortunately the style rules don't allow us to move this to a
separate function).
+ // Do not allow comparisons with null literals in pulledUpPredicates
+ for (RexNode predicate : this.pulledUpPredicates) {
+ switch (predicate.getKind()) {
+ // note that IS_DISTINCT_FROM is not in this list
+ case EQUALS:
+ case NOT_EQUALS:
+ case LESS_THAN:
+ case LESS_THAN_OR_EQUAL:
+ case GREATER_THAN:
+ case GREATER_THAN_OR_EQUAL:
+ final RexCall call = (RexCall) predicate;
+ final RexNode left = call.getOperands().get(0);
+ if (left.getKind() == SqlKind.LITERAL) {
+ RexLiteral literal = (RexLiteral) left;
+ Litmus.THROW.check(literal.getValue() != null,
+ "Comparison with NULL in pulledUpPredicates");
+ }
+ final RexNode right = call.getOperands().get(1);
+ if (right.getKind() == SqlKind.LITERAL) {
+ RexLiteral literal = (RexLiteral) right;
+ Litmus.THROW.check(literal.getValue() != null,
+ "Comparison with NULL in pulledUpPredicates");
+ }
+ break;
+ default:
+ break;
+ }
+ }
}
/** Creates a RelOptPredicateList with only pulled-up predicates, no inferred
diff --git
a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java
b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java
index 9dcfb8e92e..44b3f1c1d5 100644
--- a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java
+++ b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java
@@ -318,12 +318,11 @@ public RelOptPredicateList getPredicates(Filter filter,
RelMetadataQuery mq) {
final RelNode input = filter.getInput();
final RexBuilder rexBuilder = filter.getCluster().getRexBuilder();
final RelOptPredicateList inputInfo = mq.getPulledUpPredicates(input);
-
+ List<RexNode> predicates =
+
RexUtil.retainDeterministic(RelOptUtil.conjunctions(filter.getCondition()));
return Util.first(inputInfo, RelOptPredicateList.EMPTY)
.union(rexBuilder,
- RelOptPredicateList.of(rexBuilder,
- RexUtil.retainDeterministic(
- RelOptUtil.conjunctions(filter.getCondition()))));
+ RelOptPredicateList.of(rexBuilder, predicates));
}
/**
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 86345533e3..1283a0f2e8 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
@@ -757,6 +757,11 @@ private <C extends Comparable<C>> RexNode
simplifyComparison(RexCall e,
}
}
+ RexNode node = simplifyComparisonWithNull(e, unknownAs);
+ if (node instanceof RexLiteral) {
+ return node;
+ }
+
// If none of the arguments were simplified, return the call unchanged.
final RexNode e2;
if (operands.equals(e.operands)) {
@@ -767,6 +772,41 @@ private <C extends Comparable<C>> RexNode
simplifyComparison(RexCall e,
return simplifyUsingPredicates(e2, clazz);
}
+
+ /**
+ * If this RexNode is a comparison against NULL, return FALSE, otherwise
return it unchanged.
+ */
+ static RexNode simplifyComparisonWithNull(
+ RexNode e, RexBuilder rexBuilder, RexUnknownAs unknownAs) {
+ final RexSimplify.Comparison comparison = RexSimplify.Comparison.of(e);
+ if (comparison != null) {
+ boolean againstNull = comparison.literal.isNull();
+ // There is another possibility to check: in a comparison like 1 = null,
+ // the "non-literal" side of the Comparison can be null
+ if (comparison.ref instanceof RexLiteral) {
+ againstNull = againstNull || ((RexLiteral) comparison.ref).isNull();
+ }
+ if (againstNull) {
+ return unknownAs == FALSE
+ ? rexBuilder.makeLiteral(false)
+ : rexBuilder.makeNullLiteral(e.getType());
+ }
+ }
+ return e;
+ }
+
+ public static RexNode simplifyComparisonWithNull(RexNode e, RexBuilder
rexBuilder) {
+ return RexSimplify.simplifyComparisonWithNull(e, rexBuilder, FALSE);
+ }
+
+ /**
+ * If this RexNode is a comparison against NULL, return a simplified form,
+ * otherwise return it unchanged.
+ */
+ public RexNode simplifyComparisonWithNull(RexNode e, RexUnknownAs unknownAs)
{
+ return simplifyComparisonWithNull(e, this.rexBuilder, unknownAs);
+ }
+
/**
* Simplifies a conjunction of boolean expressions.
*/
@@ -2081,7 +2121,8 @@ private static <C extends Comparable<C>> RangeSet<C>
residue(RexNode ref,
if (comparison != null && comparison.ref.equals(ref)) {
final C c1 = comparison.literal.getValueAs(clazz);
if (c1 == null) {
- continue;
+ throw new AssertionError("value must not be null in "
+ + comparison.literal);
}
switch (predicate.getKind()) {
case NOT_EQUALS:
diff --git a/core/src/main/java/org/apache/calcite/rex/RexUtil.java
b/core/src/main/java/org/apache/calcite/rex/RexUtil.java
index d4cc04c6eb..4b134a1439 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexUtil.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexUtil.java
@@ -819,13 +819,13 @@ public static boolean isDeterministic(RexNode e) {
}
public static List<RexNode> retainDeterministic(List<RexNode> list) {
- List<RexNode> conjuctions = new ArrayList<>();
+ List<RexNode> conjunctions = new ArrayList<>();
for (RexNode x : list) {
if (isDeterministic(x)) {
- conjuctions.add(x);
+ conjunctions.add(x);
}
}
- return conjuctions;
+ return conjunctions;
}
/**
diff --git
a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
index 87d2b47201..b943a909e3 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
@@ -88,6 +88,7 @@
import org.apache.calcite.rex.RexPatternFieldRef;
import org.apache.calcite.rex.RexRangeRef;
import org.apache.calcite.rex.RexShuttle;
+import org.apache.calcite.rex.RexSimplify;
import org.apache.calcite.rex.RexSubQuery;
import org.apache.calcite.rex.RexUtil;
import org.apache.calcite.rex.RexWindowBound;
@@ -217,6 +218,7 @@
import java.util.function.BiFunction;
import java.util.function.Supplier;
import java.util.function.UnaryOperator;
+import java.util.stream.Collectors;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.ImmutableList.toImmutableList;
@@ -1112,6 +1114,16 @@ private static SqlNode reg(SqlValidatorScope scope,
SqlNode e) {
return e;
}
+ private RexNode simplifyPredicate(RexNode predicate) {
+ final RexNode convertedWhere2 =
+ RexUtil.removeNullabilityCast(typeFactory, predicate);
+ List<RexNode> conjuncts = RelOptUtil.conjunctions(convertedWhere2);
+ List<RexNode> simplified = conjuncts.stream()
+ .map(e -> RexSimplify.simplifyComparisonWithNull(e, rexBuilder))
+ .collect(Collectors.toList());
+ return RexUtil.composeConjunction(rexBuilder, simplified);
+ }
+
/**
* Converts a WHERE clause.
*
@@ -1127,8 +1139,7 @@ private void convertWhere(
SqlNode newWhere = pushDownNotForIn(bb.scope, where);
replaceSubQueries(bb, newWhere, RelOptUtil.Logic.UNKNOWN_AS_FALSE);
final RexNode convertedWhere = bb.convertExpression(newWhere);
- final RexNode convertedWhere2 =
- RexUtil.removeNullabilityCast(typeFactory, convertedWhere);
+ final RexNode convertedWhere2 = simplifyPredicate(convertedWhere);
// only allocate filter if the condition is not TRUE
if (convertedWhere2.isAlwaysTrue()) {
diff --git
a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
index 306fc26f39..ebd8835ca3 100644
---
a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
+++
b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
@@ -10576,7 +10576,7 @@ private void checkLiteral2(String expression, String
expected) {
+ "FROM (SELECT 1 AS \"additional_column\",
\"product\".\"product_id\", \"product\".\"product_name\"\n"
+ "FROM \"foodmart\".\"product\"\n"
+ "LEFT JOIN \"foodmart\".\"product\" AS \"product0\" ON
\"product\".\"product_id\" = \"product0\".\"product_id\") AS \"t\"\n"
- + "WHERE \"product_name\" IS NOT NULL AND NOT EXISTS (SELECT *\n"
+ + "WHERE NOT EXISTS (SELECT *\n"
+ "FROM \"foodmart\".\"employee\"\n"
+ "WHERE \"employee_id\" = \"t\".\"additional_column\")";
sql(sql).ok(expected);
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 3a4fb184c5..08d7f52c29 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -4832,7 +4832,7 @@ RelOptFixture checkDynamicFunctions(boolean
treatDynamicCallsAsConstant) {
+ "where empno=10 and empno is not null";
sql(sql)
.withRule(CoreRules.FILTER_REDUCE_EXPRESSIONS)
- .check();
+ .checkUnchanged();
}
@Test void testReduceConstantsNegated() {
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 ac3a626d81..f2f8e873bc 100644
--- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -1994,13 +1994,7 @@ MultiJoin(joinFilter=[true], isFullOuterJoin=[false],
joinTypes=[[RIGHT, INNER]]
<Resource name="planBefore">
<![CDATA[
LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4],
SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8])
- LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4],
SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8])
- LogicalCorrelate(correlation=[$cor0], joinType=[inner],
requiredColumns=[{0}])
- LogicalTableScan(table=[[CATALOG, SALES, EMP]])
- LogicalAggregate(group=[{0}])
- LogicalProject(i=[true])
- LogicalFilter(condition=[=($cor0.EMPNO, null)])
- LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
+ LogicalValues(tuples=[[]])
]]>
</Resource>
<Resource name="planAfter">
@@ -2016,13 +2010,7 @@ LogicalValues(tuples=[[]])
<Resource name="planBefore">
<![CDATA[
LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4],
SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8])
- LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4],
SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8])
- LogicalCorrelate(correlation=[$cor0], joinType=[inner],
requiredColumns=[{0}])
- LogicalTableScan(table=[[CATALOG, SALES, EMP]])
- LogicalAggregate(group=[{0}])
- LogicalProject(i=[true])
- LogicalFilter(condition=[=($cor0.EMPNO, null)])
- LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
+ LogicalValues(tuples=[[]])
]]>
</Resource>
<Resource name="planAfter">
@@ -11982,7 +11970,7 @@ and empno = 10 and mgr is null and empno = 10]]>
<Resource name="planBefore">
<![CDATA[
LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4],
SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8])
- LogicalFilter(condition=[AND(=($7, 7), =($0, 10), IS NULL($3), =($0, 10))])
+ LogicalFilter(condition=[AND(=($7, 7), =($0, 10), IS NULL($3))])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
</Resource>
@@ -15364,7 +15352,7 @@ LogicalProject(EMPNO=[$0], DEPTNO=[$1])
<Resource name="planBefore">
<![CDATA[
LogicalProject(EXPR$0=[+(1, 2)], EXPR$1=[+($0, +(3, 4))], EXPR$2=[+(+(5, 6),
$0)], EXPR$3=[null:INTEGER], EXPR$4=[CASE(IS NOT NULL(2), 2, null:INTEGER)],
EXPR$5=[ROW(+(7, 8))])
- LogicalFilter(condition=[AND(=($0, +(7, 8)), =($0, +(8, 7)), =($0, CASE(IS
NOT NULL(2), 2, null:INTEGER)))])
+ LogicalFilter(condition=[AND(=($0, +(7, 8)), =($0, CASE(IS NOT NULL(2), 2,
null:INTEGER)))])
LogicalProject(DEPTNO=[$0], NAME=[$1], EMPNO=[$2], ENAME=[$3], JOB=[$4],
MGR=[$5], HIREDATE=[$6], SAL=[$7], COMM=[$8], DEPTNO0=[$9], SLACKER=[$10])
LogicalJoin(condition=[=($0, $11)], joinType=[inner])
LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
@@ -15555,7 +15543,7 @@ and empno = 10 and mgr is null and empno = 10]]>
<Resource name="planBefore">
<![CDATA[
LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4],
SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8])
- LogicalFilter(condition=[AND(=($7, 7), =($7, 8), =($0, 10), IS NULL($3),
=($0, 10))])
+ LogicalFilter(condition=[AND(=($7, 7), =($7, 8), =($0, 10), IS NULL($3))])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
</Resource>
@@ -15717,13 +15705,6 @@ where empno=10 and empno is not null]]>
</Resource>
<Resource name="planBefore">
<![CDATA[
-LogicalProject(EMPNO=[$0])
- LogicalFilter(condition=[AND(=($0, 10), IS NOT NULL($0))])
- LogicalTableScan(table=[[CATALOG, SALES, EMP]])
-]]>
- </Resource>
- <Resource name="planAfter">
- <![CDATA[
LogicalProject(EMPNO=[$0])
LogicalFilter(condition=[=($0, 10)])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
@@ -15737,7 +15718,7 @@ LogicalProject(EMPNO=[$0])
<Resource name="planBefore">
<![CDATA[
LogicalProject(EMPNO=[$0])
- LogicalFilter(condition=[AND(=($0, 10), IS NULL($0))])
+ LogicalFilter(condition=[false])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
</Resource>
@@ -15814,7 +15795,7 @@ where n is null]]>
LogicalProject(N=[$0])
LogicalFilter(condition=[IS NULL($0)])
LogicalProject(N=[$0])
- LogicalFilter(condition=[AND(IS NULL($0), IS NULL($0))])
+ LogicalFilter(condition=[IS NULL($0)])
LogicalProject(N=[null:INTEGER])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
@@ -15836,7 +15817,7 @@ LogicalProject(N=[$0])
<![CDATA[
LogicalAggregate(group=[{}], EXPR$0=[COUNT()])
LogicalProject($f0=[1])
- LogicalFilter(condition=[=(null, 1)])
+ LogicalFilter(condition=[false])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
</Resource>
