Browse Source

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*.
Nik Everett 10 tháng trước cách đây
mục cha
commit
e750a707d1

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

@@ -132,8 +132,16 @@ public final class Expressions {
         return e instanceof NamedExpression ne ? ne.name() : e.sourceText();
     }
 
-    public static boolean isNull(Expression e) {
-        return e.dataType() == DataType.NULL || (e.foldable() && e.fold() == null);
+    /**
+     * Is this {@linkplain Expression} <strong>guaranteed</strong> to have
+     * only the {@code null} value. {@linkplain Expression}s that
+     * {@link Expression#fold()} to {@code null} <strong>may</strong>
+     * return {@code false} here, but should <strong>eventually</strong> be folded
+     * into a {@link Literal} containing {@code null} which will return
+     * {@code true} from here.
+     */
+    public static boolean isGuaranteedNull(Expression e) {
+        return e.dataType() == DataType.NULL || (e instanceof Literal lit && lit.value() == null);
     }
 
     public static List<String> names(Collection<? extends Expression> e) {

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

@@ -151,14 +151,14 @@ public class In extends EsqlScalarFunction {
     public boolean foldable() {
         // QL's In fold()s to null, if value() is null, but isn't foldable() unless all children are
         // TODO: update this null check in QL too?
-        return Expressions.isNull(value)
+        return Expressions.isGuaranteedNull(value)
             || Expressions.foldable(children())
-            || (Expressions.foldable(list) && list.stream().allMatch(Expressions::isNull));
+            || (Expressions.foldable(list) && list.stream().allMatch(Expressions::isGuaranteedNull));
     }
 
     @Override
     public Object fold() {
-        if (Expressions.isNull(value) || list.stream().allMatch(Expressions::isNull)) {
+        if (Expressions.isGuaranteedNull(value) || list.stream().allMatch(Expressions::isGuaranteedNull)) {
             return null;
         }
         return super.fold();

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

@@ -30,7 +30,7 @@ public class FoldNull extends OptimizerRules.OptimizerExpressionRule<Expression>
         // perform this early to prevent the rule from converting the null filter into nullifying the whole expression
         // P.S. this could be done inside the Aggregate but this place better centralizes the logic
         if (e instanceof AggregateFunction agg) {
-            if (Expressions.isNull(agg.filter())) {
+            if (Expressions.isGuaranteedNull(agg.filter())) {
                 return agg.withFilter(Literal.of(agg.filter(), false));
             }
         }
@@ -38,13 +38,13 @@ public class FoldNull extends OptimizerRules.OptimizerExpressionRule<Expression>
         if (result != e) {
             return result;
         } else if (e instanceof In in) {
-            if (Expressions.isNull(in.value())) {
+            if (Expressions.isGuaranteedNull(in.value())) {
                 return Literal.of(in, null);
             }
         } else if (e instanceof Alias == false
             && e.nullable() == Nullability.TRUE
             && e instanceof Categorize == false
-            && Expressions.anyMatch(e.children(), Expressions::isNull)) {
+            && Expressions.anyMatch(e.children(), Expressions::isGuaranteedNull)) {
                 return Literal.of(e, null);
             }
         return e;

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

@@ -29,7 +29,7 @@ public final class PruneFilters extends OptimizerRules.OptimizerRule<Filter> {
             if (TRUE.equals(condition)) {
                 return filter.child();
             }
-            if (FALSE.equals(condition) || Expressions.isNull(condition)) {
+            if (FALSE.equals(condition) || Expressions.isGuaranteedNull(condition)) {
                 return PruneEmptyPlans.skipPlan(filter);
             }
         }
@@ -42,8 +42,8 @@ public final class PruneFilters extends OptimizerRules.OptimizerRule<Filter> {
 
     private static Expression foldBinaryLogic(BinaryLogic binaryLogic) {
         if (binaryLogic instanceof Or or) {
-            boolean nullLeft = Expressions.isNull(or.left());
-            boolean nullRight = Expressions.isNull(or.right());
+            boolean nullLeft = Expressions.isGuaranteedNull(or.left());
+            boolean nullRight = Expressions.isGuaranteedNull(or.right());
             if (nullLeft && nullRight) {
                 return new Literal(binaryLogic.source(), null, DataType.NULL);
             }
@@ -55,7 +55,7 @@ public final class PruneFilters extends OptimizerRules.OptimizerRule<Filter> {
             }
         }
         if (binaryLogic instanceof And and) {
-            if (Expressions.isNull(and.left()) || Expressions.isNull(and.right())) {
+            if (Expressions.isGuaranteedNull(and.left()) || Expressions.isGuaranteedNull(and.right())) {
                 return new Literal(binaryLogic.source(), null, DataType.NULL);
             }
         }

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

@@ -30,7 +30,7 @@ public final class SplitInWithFoldableValue extends OptimizerRules.OptimizerExpr
             List<Expression> foldables = new ArrayList<>(in.list().size());
             List<Expression> nonFoldables = new ArrayList<>(in.list().size());
             in.list().forEach(e -> {
-                if (e.foldable() && Expressions.isNull(e) == false) { // keep `null`s, needed for the 3VL
+                if (e.foldable() && Expressions.isGuaranteedNull(e) == false) { // keep `null`s, needed for the 3VL
                     foldables.add(e);
                 } else {
                     nonFoldables.add(e);

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

@@ -4820,7 +4820,7 @@ public class LogicalPlanOptimizerTests extends ESTestCase {
 
         e.forEachUp(node -> {
             if (node.children().size() == 0) {
-                result.set(result.get() || Expressions.isNull(node));
+                result.set(result.get() || Expressions.isGuaranteedNull(node));
             }
         });