Browse Source

ESQL: Introduce expression validation phase (#105477)

Introduce dedicated phase through Validate interface that allows
 interested expressions to perform post logical optimization
 verification for things like literals/foldables which might be hidden
 through references after analysis.

Fix #105425
Costin Leau 1 year ago
parent
commit
e443a7b6ba

+ 6 - 0
docs/changelog/105477.yaml

@@ -0,0 +1,6 @@
+pr: 105477
+summary: "ESQL: Introduce expression validation phase"
+area: ES|QL
+type: enhancement
+issues:
+ - 105425

+ 5 - 0
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/VerificationException.java

@@ -8,6 +8,7 @@
 package org.elasticsearch.xpack.esql;
 
 import org.elasticsearch.xpack.ql.common.Failure;
+import org.elasticsearch.xpack.ql.common.Failures;
 
 import java.util.Collection;
 
@@ -20,4 +21,8 @@ public class VerificationException extends EsqlClientException {
         super(Failure.failMessage(sources));
     }
 
+    public VerificationException(Failures failures) {
+        super(failures.toString());
+    }
+
 }

+ 23 - 0
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/capabilities/Validatable.java

@@ -0,0 +1,23 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.xpack.esql.capabilities;
+
+import org.elasticsearch.xpack.ql.common.Failures;
+
+/**
+ * Interface implemented by expressions that require validation post logical optimization,
+ * when the plan and references have been not just resolved but also replaced.
+ */
+public interface Validatable {
+
+    /**
+     * Validates the implementing expression - discovered failures are reported to the given
+     * {@link Failures} class.
+     */
+    default void validate(Failures failures) {}
+}

+ 26 - 0
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/Validations.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.xpack.esql.expression;
+
+import org.elasticsearch.xpack.ql.common.Failure;
+import org.elasticsearch.xpack.ql.expression.Expression;
+import org.elasticsearch.xpack.ql.expression.Expression.TypeResolution;
+import org.elasticsearch.xpack.ql.expression.TypeResolutions;
+
+public final class Validations {
+
+    private Validations() {}
+
+    /**
+     * Validates if the given expression is foldable - if not returns a Failure.
+     */
+    public static Failure isFoldable(Expression e, String operationName, TypeResolutions.ParamOrdinal paramOrd) {
+        TypeResolution resolution = TypeResolutions.isFoldable(e, operationName, paramOrd);
+        return resolution.unresolved() ? Failure.fail(e, resolution.message()) : null;
+    }
+}

+ 11 - 6
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/AutoBucket.java

@@ -13,12 +13,14 @@ import org.elasticsearch.compute.operator.EvalOperator.ExpressionEvaluator;
 import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.index.mapper.DateFieldMapper;
 import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException;
+import org.elasticsearch.xpack.esql.capabilities.Validatable;
 import org.elasticsearch.xpack.esql.expression.function.FunctionInfo;
 import org.elasticsearch.xpack.esql.expression.function.Param;
 import org.elasticsearch.xpack.esql.expression.function.scalar.EsqlScalarFunction;
 import org.elasticsearch.xpack.esql.expression.function.scalar.date.DateTrunc;
 import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.Div;
 import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.Mul;
+import org.elasticsearch.xpack.ql.common.Failures;
 import org.elasticsearch.xpack.ql.expression.Expression;
 import org.elasticsearch.xpack.ql.expression.Foldables;
 import org.elasticsearch.xpack.ql.expression.Literal;
@@ -32,11 +34,11 @@ import java.util.List;
 import java.util.function.BiFunction;
 import java.util.function.Function;
 
+import static org.elasticsearch.xpack.esql.expression.Validations.isFoldable;
 import static org.elasticsearch.xpack.ql.expression.TypeResolutions.ParamOrdinal.FIRST;
 import static org.elasticsearch.xpack.ql.expression.TypeResolutions.ParamOrdinal.FOURTH;
 import static org.elasticsearch.xpack.ql.expression.TypeResolutions.ParamOrdinal.SECOND;
 import static org.elasticsearch.xpack.ql.expression.TypeResolutions.ParamOrdinal.THIRD;
-import static org.elasticsearch.xpack.ql.expression.TypeResolutions.isFoldable;
 import static org.elasticsearch.xpack.ql.expression.TypeResolutions.isInteger;
 import static org.elasticsearch.xpack.ql.expression.TypeResolutions.isNumeric;
 import static org.elasticsearch.xpack.ql.expression.TypeResolutions.isType;
@@ -52,7 +54,7 @@ import static org.elasticsearch.xpack.ql.expression.TypeResolutions.isType;
  *     in the above case we'll pick month long buckets, yielding 12 buckets.
  * </p>
  */
-public class AutoBucket extends EsqlScalarFunction {
+public class AutoBucket extends EsqlScalarFunction implements Validatable {
     // TODO maybe we should just cover the whole of representable dates here - like ten years, 100 years, 1000 years, all the way up.
     // That way you never end up with more than the target number of buckets.
     private static final Rounding LARGEST_HUMAN_DATE_ROUNDING = Rounding.builder(Rounding.DateTimeUnit.YEAR_OF_CENTURY).build();
@@ -188,10 +190,6 @@ public class AutoBucket extends EsqlScalarFunction {
         if (resolution.unresolved()) {
             return resolution;
         }
-        resolution = isFoldable(buckets, sourceText(), SECOND);
-        if (resolution.unresolved()) {
-            return resolution;
-        }
         return checkThirdAndForth.apply(from, THIRD).and(checkThirdAndForth.apply(to, FOURTH));
     }
 
@@ -206,6 +204,13 @@ public class AutoBucket extends EsqlScalarFunction {
         );
     }
 
+    @Override
+    public void validate(Failures failures) {
+        String operation = sourceText();
+
+        failures.add(isFoldable(buckets, operation, SECOND)).add(isFoldable(from, operation, THIRD)).add(isFoldable(to, operation, FOURTH));
+    }
+
     private long foldToLong(Expression e) {
         Object value = Foldables.valueOf(e);
         return DataTypes.isDateTime(e.dataType())

+ 3 - 4
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizer.java

@@ -27,7 +27,7 @@ import org.elasticsearch.xpack.esql.plan.logical.local.LocalRelation;
 import org.elasticsearch.xpack.esql.plan.logical.local.LocalSupplier;
 import org.elasticsearch.xpack.esql.planner.PlannerUtils;
 import org.elasticsearch.xpack.esql.type.EsqlDataTypes;
-import org.elasticsearch.xpack.ql.common.Failure;
+import org.elasticsearch.xpack.ql.common.Failures;
 import org.elasticsearch.xpack.ql.expression.Alias;
 import org.elasticsearch.xpack.ql.expression.Attribute;
 import org.elasticsearch.xpack.ql.expression.AttributeMap;
@@ -70,7 +70,6 @@ import org.elasticsearch.xpack.ql.util.StringUtils;
 
 import java.time.ZoneId;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -98,8 +97,8 @@ public class LogicalPlanOptimizer extends ParameterizedRuleExecutor<LogicalPlan,
     public LogicalPlan optimize(LogicalPlan verified) {
         var optimized = execute(verified);
 
-        Collection<Failure> failures = verifier.verify(optimized);
-        if (failures.isEmpty() == false) {
+        Failures failures = verifier.verify(optimized);
+        if (failures.hasFailures()) {
             throw new VerificationException(failures);
         }
         return optimized;

+ 11 - 25
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/LogicalVerifier.java

@@ -7,21 +7,11 @@
 
 package org.elasticsearch.xpack.esql.optimizer;
 
-import org.elasticsearch.xpack.esql.expression.function.scalar.math.AutoBucket;
+import org.elasticsearch.xpack.esql.capabilities.Validatable;
 import org.elasticsearch.xpack.esql.optimizer.OptimizerRules.LogicalPlanDependencyCheck;
-import org.elasticsearch.xpack.ql.common.Failure;
-import org.elasticsearch.xpack.ql.expression.Expression;
+import org.elasticsearch.xpack.ql.common.Failures;
 import org.elasticsearch.xpack.ql.plan.logical.LogicalPlan;
 
-import java.util.Collection;
-import java.util.LinkedHashSet;
-import java.util.Set;
-
-import static org.elasticsearch.xpack.ql.common.Failure.fail;
-import static org.elasticsearch.xpack.ql.expression.TypeResolutions.ParamOrdinal.FOURTH;
-import static org.elasticsearch.xpack.ql.expression.TypeResolutions.ParamOrdinal.THIRD;
-import static org.elasticsearch.xpack.ql.expression.TypeResolutions.isFoldable;
-
 public final class LogicalVerifier {
 
     private static final LogicalPlanDependencyCheck DEPENDENCY_CHECK = new LogicalPlanDependencyCheck();
@@ -30,25 +20,21 @@ public final class LogicalVerifier {
     private LogicalVerifier() {}
 
     /** Verifies the optimized logical plan. */
-    public Collection<Failure> verify(LogicalPlan plan) {
-        Set<Failure> failures = new LinkedHashSet<>();
+    public Failures verify(LogicalPlan plan) {
+        Failures failures = new Failures();
 
         plan.forEachUp(p -> {
             // dependency check
             // FIXME: re-enable
             // DEPENDENCY_CHECK.checkPlan(p, failures);
 
-            // post optimization folding check
-            p.forEachExpression(AutoBucket.class, e -> {
-                Expression.TypeResolution resolution = isFoldable(e.from(), e.sourceText(), THIRD);
-                if (resolution.unresolved()) {
-                    failures.add(fail(e, resolution.message()));
-                }
-                resolution = isFoldable(e.to(), e.sourceText(), FOURTH);
-                if (resolution.unresolved()) {
-                    failures.add(fail(e, resolution.message()));
-                }
-            });
+            if (failures.hasFailures() == false) {
+                p.forEachExpression(ex -> {
+                    if (ex instanceof Validatable v) {
+                        v.validate(failures);
+                    }
+                });
+            }
         });
 
         return failures;

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

@@ -2942,7 +2942,7 @@ public class LogicalPlanOptimizerTests extends ESTestCase {
         assertTrue(e.getMessage().startsWith("Found "));
         final String header = "Found 1 problem\nline ";
         assertEquals(
-            "3:8: third argument of [auto_bucket(salary, 10, emp_no, bucket_end)] must be a constant, received [emp_no]",
+            "3:32: third argument of [auto_bucket(salary, 10, emp_no, bucket_end)] must be a constant, received [emp_no]",
             e.getMessage().substring(header.length())
         );
     }

+ 57 - 0
x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/common/Failures.java

@@ -0,0 +1,57 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.xpack.ql.common;
+
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.Objects;
+
+/**
+ * Glorified list for managing {@link Failure}s.
+ */
+public class Failures {
+
+    private final Collection<Failure> failures;
+
+    public Failures() {
+        this.failures = new LinkedHashSet<>();
+    }
+
+    public Failures add(Failure failure) {
+        if (failure != null) {
+            failures.add(failure);
+        }
+        return this;
+    }
+
+    public boolean hasFailures() {
+        return failures.size() > 0;
+    }
+
+    public Collection<Failure> failures() {
+        return failures;
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(failures);
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        Failures failures1 = (Failures) o;
+        return Objects.equals(failures, failures1.failures);
+    }
+
+    @Override
+    public String toString() {
+        return failures.isEmpty() ? "[]" : Failure.failMessage(failures);
+    }
+}