ESQL: Do not fold in Range.foldable (elastic#119766) (elastic#119964)

The upper and lower bounds can be incompatible, in which case the range
is foldable to FALSE independently of the foldability of the field.

But we shouldn't try and fold the bounds to find out. Calling
`foldable` really shouldn't already perform folding.

Instead, only perform the check for the invalid bounds if the bounds are
already literals. This means that folding will still take place, but it
requires the range's child expressions to be folded first.

Author: alex-spies

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

@@ -8,6 +8,7 @@
 
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.xpack.esql.core.expression.Expression;
+import org.elasticsearch.xpack.esql.core.expression.Literal;
 import org.elasticsearch.xpack.esql.core.expression.function.scalar.ScalarFunction;
 import org.elasticsearch.xpack.esql.core.expression.predicate.operator.comparison.BinaryComparison;
 import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
@@ -85,10 +86,22 @@ public ZoneId zoneId() {
         return zoneId;
     }
 
+    /**
+     * In case that the range is empty due to foldable, invalid bounds, but the bounds themselves are not yet folded, the optimizer will
+     * need two passes to fold this.
+     * That's because we shouldn't perform folding when trying to determine foldability.
+     */
     @Override
     public boolean foldable() {
         if (lower.foldable() && upper.foldable()) {
-            return areBoundariesInvalid() || value.foldable();
+            if (value().foldable()) {
+                return true;
+            }
+
+            // We cannot fold the bounds here; but if they're already literals, we can check if the range is always empty.
+            if (lower() instanceof Literal && upper() instanceof Literal) {
+                return areBoundariesInvalid();
+            }
         }
 
         return false;

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/ConstantFoldingTests.java

@@ -14,6 +14,7 @@
 import org.elasticsearch.xpack.esql.core.expression.Literal;
 import org.elasticsearch.xpack.esql.core.expression.Nullability;
 import org.elasticsearch.xpack.esql.core.expression.predicate.BinaryOperator;
+import org.elasticsearch.xpack.esql.core.expression.predicate.Range;
 import org.elasticsearch.xpack.esql.core.expression.predicate.logical.And;
 import org.elasticsearch.xpack.esql.core.expression.predicate.logical.Not;
 import org.elasticsearch.xpack.esql.core.expression.predicate.logical.Or;
@@ -27,11 +28,16 @@
 import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.Mod;
 import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.Mul;
 import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.Sub;
+import org.elasticsearch.xpack.esql.plan.logical.Filter;
+import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
 
 import static org.elasticsearch.xpack.esql.EsqlTestUtils.FIVE;
 import static org.elasticsearch.xpack.esql.EsqlTestUtils.THREE;
 import static org.elasticsearch.xpack.esql.EsqlTestUtils.TWO;
+import static org.elasticsearch.xpack.esql.EsqlTestUtils.as;
+import static org.elasticsearch.xpack.esql.EsqlTestUtils.emptySource;
 import static org.elasticsearch.xpack.esql.EsqlTestUtils.equalsOf;
+import static org.elasticsearch.xpack.esql.EsqlTestUtils.fieldAttribute;
 import static org.elasticsearch.xpack.esql.EsqlTestUtils.greaterThanOf;
 import static org.elasticsearch.xpack.esql.EsqlTestUtils.greaterThanOrEqualOf;
 import static org.elasticsearch.xpack.esql.EsqlTestUtils.lessThanOf;
@@ -111,6 +117,53 @@ public void testArithmeticFolding() {
         assertEquals(1, foldOperator(new Mod(EMPTY, new Literal(EMPTY, 7, DataType.INTEGER), THREE)));
     }
 
+    public void testFoldRange() {
+        // 1 + 9 < value AND value < 20-1
+        // with value = 12 and randomly replacing the `<` by `<=`
+        Expression lowerBound = new Add(EMPTY, new Literal(EMPTY, 1, DataType.INTEGER), new Literal(EMPTY, 9, DataType.INTEGER));
+        Expression upperBound = new Sub(EMPTY, new Literal(EMPTY, 20, DataType.INTEGER), new Literal(EMPTY, 1, DataType.INTEGER));
+        Expression value = new Literal(EMPTY, 12, DataType.INTEGER);
+        Range range = new Range(EMPTY, value, lowerBound, randomBoolean(), upperBound, randomBoolean(), randomZone());
+
+        Expression folded = new ConstantFolding().rule(range);
+        assertTrue((Boolean) as(folded, Literal.class).value());
+    }
+
+    public void testFoldRangeWithInvalidBoundaries() {
+        // 1 + 9 < value AND value <= 11 - 1
+        // This is always false. We also randomly test versions with `<=`.
+        Expression lowerBound;
+        boolean includeLowerBound = randomBoolean();
+        if (includeLowerBound) {
+            // 1 + 10 <= value
+            lowerBound = new Add(EMPTY, new Literal(EMPTY, 1, DataType.INTEGER), new Literal(EMPTY, 10, DataType.INTEGER));
+        } else {
+            // 1 + 9 < value
+            lowerBound = new Add(EMPTY, new Literal(EMPTY, 1, DataType.INTEGER), new Literal(EMPTY, 9, DataType.INTEGER));
+        }
+
+        boolean includeUpperBound = randomBoolean();
+        // value < 11 - 1
+        // or
+        // value <= 11 - 1
+        Expression upperBound = new Sub(EMPTY, new Literal(EMPTY, 11, DataType.INTEGER), new Literal(EMPTY, 1, DataType.INTEGER));
+
+        Expression value = fieldAttribute();
+
+        Range range = new Range(EMPTY, value, lowerBound, includeLowerBound, upperBound, includeUpperBound, randomZone());
+
+        // We need to test this as part of a logical plan, to correctly simulate how we traverse down the expression tree.
+        // Just applying this to the range directly won't perform a transformDown.
+        LogicalPlan filter = new Filter(EMPTY, emptySource(), range);
+
+        Filter foldedOnce = as(new ConstantFolding().apply(filter), Filter.class);
+        // We need to run the rule twice, because during the first run only the boundaries can be folded - the range doesn't know it's
+        // foldable, yet.
+        Filter foldedTwice = as(new ConstantFolding().apply(foldedOnce), Filter.class);
+
+        assertFalse((Boolean) as(foldedTwice.condition(), Literal.class).value());
+    }
+
     private static Object foldOperator(BinaryOperator<?, ?, ?, ?> b) {
         return ((Literal) new ConstantFolding().rule(b)).value();
     }