Make sure verify is being called when we call LocalPhysicalPlanOptimizer.localOptimize()

Author: julian-elastic

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

@@ -24,7 +24,7 @@ public Failures verify(LogicalPlan plan, boolean skipRemoteEnrichVerification) {
         Failures failures = new Failures();
         Failures dependencyFailures = new Failures();
 
-        if(skipRemoteEnrichVerification) {
+        if (skipRemoteEnrichVerification) {
             // AwaitsFix https://github.com/elastic/elasticsearch/issues/118531
             var enriches = plan.collectFirstChildren(Enrich.class::isInstance);
             if (enriches.isEmpty() == false && ((Enrich) enriches.get(0)).mode() == Enrich.Mode.REMOTE) {

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/PhysicalVerifier.java

@@ -31,7 +31,7 @@ public Failures verify(PhysicalPlan plan, boolean skipRemoteEnrichVerification)
         Failures failures = new Failures();
         Failures depFailures = new Failures();
 
-        if (skipRemoteEnrichVerification){
+        if (skipRemoteEnrichVerification) {
             // AwaitsFix https://github.com/elastic/elasticsearch/issues/118531
             var enriches = plan.collectFirstChildren(EnrichExec.class::isInstance);
             if (enriches.isEmpty() == false && ((EnrichExec) enriches.get(0)).mode() == Enrich.Mode.REMOTE) {

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

@@ -43,6 +43,7 @@
 import org.elasticsearch.xpack.esql.core.expression.Expression;
 import org.elasticsearch.xpack.esql.core.expression.Expressions;
 import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
+import org.elasticsearch.xpack.esql.core.expression.FoldContext;
 import org.elasticsearch.xpack.esql.core.expression.Literal;
 import org.elasticsearch.xpack.esql.core.expression.NamedExpression;
 import org.elasticsearch.xpack.esql.core.expression.ReferenceAttribute;
@@ -2062,7 +2063,7 @@ public void testToDateNanosPushDown() {
         assertThat(expected.toString(), is(esQuery.query().toString()));
     }
 
-    public void testVerifierOnMissingReferencesWithBinaryPlans() throws Exception {
+    public void testVerifierOnMissingReferences() throws Exception {
 
         PhysicalPlan plan = plannerOptimizer.plan("""
             from test
@@ -2079,7 +2080,17 @@ public void testVerifierOnMissingReferencesWithBinaryPlans() throws Exception {
         List<Order> orders = List.of(new Order(plan.source(), missingAttr, Order.OrderDirection.ASC, Order.NullsPosition.FIRST));
         TopNExec topNExec = new TopNExec(plan.source(), plan, orders, new Literal(Source.EMPTY, limit, INTEGER), randomEstimatedRowSize());
 
-        Exception e = expectThrows(IllegalStateException.class, () -> plannerOptimizer.localPhysicalVerify(topNExec));
+        // We want to verify that the localOptimize detects the missing attribute.
+        // However, it also throws an error in one of the rules before we get to the verifier.
+        // So we use an implementation of LocalPhysicalPlanOptimizer that does not have any rules.
+        LocalPhysicalOptimizerContext context = new LocalPhysicalOptimizerContext(config, FoldContext.small(), SearchStats.EMPTY);
+        LocalPhysicalPlanOptimizer optimizerWithNoopExecute = new LocalPhysicalPlanOptimizer(context) {
+            @Override
+            protected List<Batch<PhysicalPlan>> batches() {
+                return List.of();
+            }
+        };
+        Exception e = expectThrows(IllegalStateException.class, () -> optimizerWithNoopExecute.localOptimize(topNExec));
         assertThat(e.getMessage(), containsString(" optimized incorrectly due to missing references [missing attr"));
     }
 

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

@@ -23,7 +23,6 @@ public class TestPlannerOptimizer {
     private final Analyzer analyzer;
     private final LogicalPlanOptimizer logicalOptimizer;
     private final PhysicalPlanOptimizer physicalPlanOptimizer;
-    private final LocalPhysicalPlanOptimizer localPhysicalPlanOptimizer;
 
     private final Mapper mapper;
     private final Configuration config;
@@ -40,9 +39,6 @@ public TestPlannerOptimizer(Configuration config, Analyzer analyzer, LogicalPlan
         parser = new EsqlParser();
         physicalPlanOptimizer = new PhysicalPlanOptimizer(new PhysicalOptimizerContext(config));
         mapper = new Mapper();
-        localPhysicalPlanOptimizer = new LocalPhysicalPlanOptimizer(
-            new LocalPhysicalOptimizerContext(config, FoldContext.small(), SearchStats.EMPTY)
-        );
 
     }
 
@@ -89,12 +85,4 @@ private PhysicalPlan physicalPlan(String query, Analyzer analyzer) {
         var physical = mapper.map(logical);
         return physical;
     }
-
-    public PhysicalPlan localPhysicalOptimize(PhysicalPlan plan) {
-        return localPhysicalPlanOptimizer.localOptimize(plan);
-    }
-
-    public PhysicalPlan localPhysicalVerify(PhysicalPlan plan) {
-        return localPhysicalPlanOptimizer.verify(plan);
-    }
 }