Skip to content

Commit e750a70

Browse files
authored
ESQL: Rework isNull (#118101) (#118178)
This reworks `Expressions#isNull` so it only matches the `null` literal. This doesn't super change the way ESQL works because we already rewrite things that `fold` into `null` into the `null` literal. It's just that, now, `isNull` won't return `true` for things that *fold* to null - only things that have *already* folded to null. This is important because `fold` can be quite expensive so we're better off keeping the results of it when possible. Which is what the constant folding rules *do*.
1 parent f8258db commit e750a70

File tree

6 files changed

+22
-14
lines changed

6 files changed

+22
-14
lines changed

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Expressions.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,16 @@ public static String name(Expression e) {
132132
return e instanceof NamedExpression ne ? ne.name() : e.sourceText();
133133
}
134134

135-
public static boolean isNull(Expression e) {
136-
return e.dataType() == DataType.NULL || (e.foldable() && e.fold() == null);
135+
/**
136+
* Is this {@linkplain Expression} <strong>guaranteed</strong> to have
137+
* only the {@code null} value. {@linkplain Expression}s that
138+
* {@link Expression#fold()} to {@code null} <strong>may</strong>
139+
* return {@code false} here, but should <strong>eventually</strong> be folded
140+
* into a {@link Literal} containing {@code null} which will return
141+
* {@code true} from here.
142+
*/
143+
public static boolean isGuaranteedNull(Expression e) {
144+
return e.dataType() == DataType.NULL || (e instanceof Literal lit && lit.value() == null);
137145
}
138146

139147
public static List<String> names(Collection<? extends Expression> e) {

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/In.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,14 +151,14 @@ public Expression replaceChildren(List<Expression> newChildren) {
151151
public boolean foldable() {
152152
// QL's In fold()s to null, if value() is null, but isn't foldable() unless all children are
153153
// TODO: update this null check in QL too?
154-
return Expressions.isNull(value)
154+
return Expressions.isGuaranteedNull(value)
155155
|| Expressions.foldable(children())
156-
|| (Expressions.foldable(list) && list.stream().allMatch(Expressions::isNull));
156+
|| (Expressions.foldable(list) && list.stream().allMatch(Expressions::isGuaranteedNull));
157157
}
158158

159159
@Override
160160
public Object fold() {
161-
if (Expressions.isNull(value) || list.stream().allMatch(Expressions::isNull)) {
161+
if (Expressions.isGuaranteedNull(value) || list.stream().allMatch(Expressions::isGuaranteedNull)) {
162162
return null;
163163
}
164164
return super.fold();

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/FoldNull.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,21 +30,21 @@ public Expression rule(Expression e) {
3030
// perform this early to prevent the rule from converting the null filter into nullifying the whole expression
3131
// P.S. this could be done inside the Aggregate but this place better centralizes the logic
3232
if (e instanceof AggregateFunction agg) {
33-
if (Expressions.isNull(agg.filter())) {
33+
if (Expressions.isGuaranteedNull(agg.filter())) {
3434
return agg.withFilter(Literal.of(agg.filter(), false));
3535
}
3636
}
3737

3838
if (result != e) {
3939
return result;
4040
} else if (e instanceof In in) {
41-
if (Expressions.isNull(in.value())) {
41+
if (Expressions.isGuaranteedNull(in.value())) {
4242
return Literal.of(in, null);
4343
}
4444
} else if (e instanceof Alias == false
4545
&& e.nullable() == Nullability.TRUE
4646
&& e instanceof Categorize == false
47-
&& Expressions.anyMatch(e.children(), Expressions::isNull)) {
47+
&& Expressions.anyMatch(e.children(), Expressions::isGuaranteedNull)) {
4848
return Literal.of(e, null);
4949
}
5050
return e;

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PruneFilters.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ protected LogicalPlan rule(Filter filter) {
2929
if (TRUE.equals(condition)) {
3030
return filter.child();
3131
}
32-
if (FALSE.equals(condition) || Expressions.isNull(condition)) {
32+
if (FALSE.equals(condition) || Expressions.isGuaranteedNull(condition)) {
3333
return PruneEmptyPlans.skipPlan(filter);
3434
}
3535
}
@@ -42,8 +42,8 @@ protected LogicalPlan rule(Filter filter) {
4242

4343
private static Expression foldBinaryLogic(BinaryLogic binaryLogic) {
4444
if (binaryLogic instanceof Or or) {
45-
boolean nullLeft = Expressions.isNull(or.left());
46-
boolean nullRight = Expressions.isNull(or.right());
45+
boolean nullLeft = Expressions.isGuaranteedNull(or.left());
46+
boolean nullRight = Expressions.isGuaranteedNull(or.right());
4747
if (nullLeft && nullRight) {
4848
return new Literal(binaryLogic.source(), null, DataType.NULL);
4949
}
@@ -55,7 +55,7 @@ private static Expression foldBinaryLogic(BinaryLogic binaryLogic) {
5555
}
5656
}
5757
if (binaryLogic instanceof And and) {
58-
if (Expressions.isNull(and.left()) || Expressions.isNull(and.right())) {
58+
if (Expressions.isGuaranteedNull(and.left()) || Expressions.isGuaranteedNull(and.right())) {
5959
return new Literal(binaryLogic.source(), null, DataType.NULL);
6060
}
6161
}

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/SplitInWithFoldableValue.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public Expression rule(In in) {
3030
List<Expression> foldables = new ArrayList<>(in.list().size());
3131
List<Expression> nonFoldables = new ArrayList<>(in.list().size());
3232
in.list().forEach(e -> {
33-
if (e.foldable() && Expressions.isNull(e) == false) { // keep `null`s, needed for the 3VL
33+
if (e.foldable() && Expressions.isGuaranteedNull(e) == false) { // keep `null`s, needed for the 3VL
3434
foldables.add(e);
3535
} else {
3636
nonFoldables.add(e);

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4820,7 +4820,7 @@ private static boolean oneLeaveIsNull(Expression e) {
48204820

48214821
e.forEachUp(node -> {
48224822
if (node.children().size() == 0) {
4823-
result.set(result.get() || Expressions.isNull(node));
4823+
result.set(result.get() || Expressions.isGuaranteedNull(node));
48244824
}
48254825
});
48264826

0 commit comments

Comments
 (0)