Skip to content

Commit b238367

Browse files
mark-vieirajoshua-adams-1
authored andcommitted
Suppport per-project behavior in ESQL extra verifiers (elastic#131884)
1 parent f45e1ad commit b238367

File tree

11 files changed

+50
-59
lines changed

11 files changed

+50
-59
lines changed

x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionBreakerIT.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,6 @@ protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
8787
}
8888

8989
public static class EsqlTestPluginWithMockBlockFactory extends EsqlPlugin {
90-
public EsqlTestPluginWithMockBlockFactory(Settings settings) {
91-
super(settings);
92-
}
93-
9490
@Override
9591
protected BlockFactoryProvider blockFactoryProvider(
9692
CircuitBreaker breaker,

x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlPluginWithEnterpriseOrTrialLicense.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
package org.elasticsearch.xpack.esql.action;
99

10-
import org.elasticsearch.common.settings.Settings;
1110
import org.elasticsearch.license.License;
1211
import org.elasticsearch.license.XPackLicenseState;
1312
import org.elasticsearch.license.internal.XPackLicenseStatus;
@@ -20,10 +19,6 @@
2019
* that require an Enteprise (or Trial) license.
2120
*/
2221
public class EsqlPluginWithEnterpriseOrTrialLicense extends EsqlPlugin {
23-
public EsqlPluginWithEnterpriseOrTrialLicense(Settings settings) {
24-
super(settings);
25-
}
26-
2722
protected XPackLicenseState getLicenseState() {
2823
License.OperationMode operationMode = randomFrom(License.OperationMode.ENTERPRISE, License.OperationMode.TRIAL);
2924
return new XPackLicenseState(() -> System.currentTimeMillis(), new XPackLicenseStatus(operationMode, true, "Test license expired"));

x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlPluginWithNonEnterpriseOrExpiredLicense.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
package org.elasticsearch.xpack.esql.action;
99

10-
import org.elasticsearch.common.settings.Settings;
1110
import org.elasticsearch.license.License;
1211
import org.elasticsearch.license.XPackLicenseState;
1312
import org.elasticsearch.license.internal.XPackLicenseStatus;
@@ -23,10 +22,6 @@
2322
* - an expired enterprise or trial license
2423
*/
2524
public class EsqlPluginWithNonEnterpriseOrExpiredLicense extends EsqlPlugin {
26-
public EsqlPluginWithNonEnterpriseOrExpiredLicense(Settings settings) {
27-
super(settings);
28-
}
29-
3025
protected XPackLicenseState getLicenseState() {
3126
License.OperationMode operationMode;
3227
boolean active;

x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/spatial/SpatialNoLicenseTestCase.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
package org.elasticsearch.xpack.esql.spatial;
99

10-
import org.elasticsearch.common.settings.Settings;
1110
import org.elasticsearch.license.License;
1211
import org.elasticsearch.license.XPackLicenseState;
1312
import org.elasticsearch.license.internal.XPackLicenseStatus;
@@ -50,10 +49,6 @@ private static XPackLicenseState getLicenseState() {
5049
* This is used to test the behavior of spatial functions when no valid license is present.
5150
*/
5251
public static class TestEsqlPlugin extends EsqlPlugin {
53-
public TestEsqlPlugin(Settings settings) {
54-
super(settings);
55-
}
56-
5752
protected XPackLicenseState getLicenseState() {
5853
return SpatialNoLicenseTestCase.getLicenseState();
5954
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.esql.analysis;
9+
10+
import org.elasticsearch.cluster.project.ProjectResolver;
11+
import org.elasticsearch.cluster.service.ClusterService;
12+
import org.elasticsearch.xpack.esql.common.Failures;
13+
import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
14+
15+
import java.util.List;
16+
import java.util.function.BiConsumer;
17+
18+
/**
19+
* SPI provider interface for supplying additional ESQL plan checks to be performed during verification.
20+
*/
21+
public interface PlanCheckerProvider {
22+
/**
23+
* Build a list of checks to perform on the plan. Each one is called once per
24+
* {@link LogicalPlan} node in the plan.
25+
*/
26+
List<BiConsumer<LogicalPlan, Failures>> checkers(ProjectResolver projectResolver, ClusterService clusterService);
27+
}

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
package org.elasticsearch.xpack.esql.analysis;
99

10-
import org.elasticsearch.common.settings.Settings;
1110
import org.elasticsearch.license.XPackLicenseState;
1211
import org.elasticsearch.xpack.esql.LicenseAware;
1312
import org.elasticsearch.xpack.esql.capabilities.PostAnalysisPlanVerificationAware;
@@ -56,31 +55,22 @@
5655
* step does type resolution and fails queries based on invalid type expressions.
5756
*/
5857
public class Verifier {
59-
public interface ExtraCheckers {
60-
/**
61-
* Build a list of checks to perform on the plan. Each one is called once per
62-
* {@link LogicalPlan} node in the plan.
63-
*/
64-
List<BiConsumer<LogicalPlan, Failures>> extra(Settings settings);
65-
}
6658

6759
/**
6860
* Extra plan verification checks defined in plugins.
6961
*/
70-
private final List<ExtraCheckers> extraCheckers;
62+
private final List<BiConsumer<LogicalPlan, Failures>> extraCheckers;
7163
private final Metrics metrics;
7264
private final XPackLicenseState licenseState;
73-
private final Settings settings;
7465

7566
public Verifier(Metrics metrics, XPackLicenseState licenseState) {
76-
this(metrics, licenseState, Collections.emptyList(), Settings.EMPTY);
67+
this(metrics, licenseState, Collections.emptyList());
7768
}
7869

79-
public Verifier(Metrics metrics, XPackLicenseState licenseState, List<ExtraCheckers> extraCheckers, Settings settings) {
70+
public Verifier(Metrics metrics, XPackLicenseState licenseState, List<BiConsumer<LogicalPlan, Failures>> extraCheckers) {
8071
this.metrics = metrics;
8172
this.licenseState = licenseState;
8273
this.extraCheckers = extraCheckers;
83-
this.settings = settings;
8474
}
8575

8676
/**
@@ -104,9 +94,7 @@ Collection<Failure> verify(LogicalPlan plan, BitSet partialMetrics) {
10494

10595
// collect plan checkers
10696
var planCheckers = planCheckers(plan);
107-
for (ExtraCheckers e : extraCheckers) {
108-
planCheckers.addAll(e.extra(settings));
109-
}
97+
planCheckers.addAll(extraCheckers);
11098

11199
// Concrete verifications
112100
plan.forEachDown(p -> {

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/execution/PlanExecutor.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,21 +8,22 @@
88
package org.elasticsearch.xpack.esql.execution;
99

1010
import org.elasticsearch.action.ActionListener;
11-
import org.elasticsearch.common.settings.Settings;
1211
import org.elasticsearch.indices.IndicesExpressionGrouper;
1312
import org.elasticsearch.license.XPackLicenseState;
1413
import org.elasticsearch.telemetry.metric.MeterRegistry;
1514
import org.elasticsearch.xpack.esql.action.EsqlExecutionInfo;
1615
import org.elasticsearch.xpack.esql.action.EsqlQueryRequest;
1716
import org.elasticsearch.xpack.esql.analysis.PreAnalyzer;
1817
import org.elasticsearch.xpack.esql.analysis.Verifier;
18+
import org.elasticsearch.xpack.esql.common.Failures;
1919
import org.elasticsearch.xpack.esql.core.expression.FoldContext;
2020
import org.elasticsearch.xpack.esql.enrich.EnrichPolicyResolver;
2121
import org.elasticsearch.xpack.esql.expression.function.EsqlFunctionRegistry;
2222
import org.elasticsearch.xpack.esql.optimizer.LogicalOptimizerContext;
2323
import org.elasticsearch.xpack.esql.optimizer.LogicalPlanOptimizer;
2424
import org.elasticsearch.xpack.esql.optimizer.LogicalPlanPreOptimizer;
2525
import org.elasticsearch.xpack.esql.optimizer.LogicalPreOptimizerContext;
26+
import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
2627
import org.elasticsearch.xpack.esql.planner.mapper.Mapper;
2728
import org.elasticsearch.xpack.esql.plugin.TransportActionServices;
2829
import org.elasticsearch.xpack.esql.querylog.EsqlQueryLog;
@@ -36,6 +37,7 @@
3637
import org.elasticsearch.xpack.esql.telemetry.QueryMetric;
3738

3839
import java.util.List;
40+
import java.util.function.BiConsumer;
3941

4042
import static org.elasticsearch.action.ActionListener.wrap;
4143

@@ -55,15 +57,14 @@ public PlanExecutor(
5557
MeterRegistry meterRegistry,
5658
XPackLicenseState licenseState,
5759
EsqlQueryLog queryLog,
58-
List<Verifier.ExtraCheckers> extraCheckers,
59-
Settings settings
60+
List<BiConsumer<LogicalPlan, Failures>> extraCheckers
6061
) {
6162
this.indexResolver = indexResolver;
6263
this.preAnalyzer = new PreAnalyzer();
6364
this.functionRegistry = new EsqlFunctionRegistry();
6465
this.mapper = new Mapper();
6566
this.metrics = new Metrics(functionRegistry);
66-
this.verifier = new Verifier(metrics, licenseState, extraCheckers, settings);
67+
this.verifier = new Verifier(metrics, licenseState, extraCheckers);
6768
this.planTelemetryManager = new PlanTelemetryManager(meterRegistry);
6869
this.queryLog = queryLog;
6970
}

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

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,14 +67,16 @@
6767
import org.elasticsearch.xpack.esql.action.RestEsqlListQueriesAction;
6868
import org.elasticsearch.xpack.esql.action.RestEsqlQueryAction;
6969
import org.elasticsearch.xpack.esql.action.RestEsqlStopAsyncAction;
70-
import org.elasticsearch.xpack.esql.analysis.Verifier;
70+
import org.elasticsearch.xpack.esql.analysis.PlanCheckerProvider;
71+
import org.elasticsearch.xpack.esql.common.Failures;
7172
import org.elasticsearch.xpack.esql.enrich.EnrichLookupOperator;
7273
import org.elasticsearch.xpack.esql.enrich.LookupFromIndexOperator;
7374
import org.elasticsearch.xpack.esql.execution.PlanExecutor;
7475
import org.elasticsearch.xpack.esql.expression.ExpressionWritables;
7576
import org.elasticsearch.xpack.esql.io.stream.ExpressionQueryBuilder;
7677
import org.elasticsearch.xpack.esql.io.stream.PlanStreamWrapperQueryBuilder;
7778
import org.elasticsearch.xpack.esql.plan.PlanWritables;
79+
import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
7880
import org.elasticsearch.xpack.esql.planner.PhysicalSettings;
7981
import org.elasticsearch.xpack.esql.querydsl.query.SingleValueQuery;
8082
import org.elasticsearch.xpack.esql.querylog.EsqlQueryLog;
@@ -85,6 +87,7 @@
8587
import java.util.Collection;
8688
import java.util.List;
8789
import java.util.Objects;
90+
import java.util.function.BiConsumer;
8891
import java.util.function.Predicate;
8992
import java.util.function.Supplier;
9093

@@ -184,12 +187,7 @@ public class EsqlPlugin extends Plugin implements ActionPlugin, ExtensiblePlugin
184187
Setting.Property.Dynamic
185188
);
186189

187-
private final List<Verifier.ExtraCheckers> extraCheckers = new ArrayList<>();
188-
private final Settings settings;
189-
190-
public EsqlPlugin(Settings settings) {
191-
this.settings = settings;
192-
}
190+
private final List<PlanCheckerProvider> extraCheckerProviders = new ArrayList<>();
193191

194192
@Override
195193
public Collection<?> createComponents(PluginServices services) {
@@ -203,14 +201,17 @@ public Collection<?> createComponents(PluginServices services) {
203201
BigArrays bigArrays = services.indicesService().getBigArrays().withCircuitBreaking();
204202
var blockFactoryProvider = blockFactoryProvider(circuitBreaker, bigArrays, maxPrimitiveArrayBlockSize);
205203
setupSharedSecrets();
204+
List<BiConsumer<LogicalPlan, Failures>> extraCheckers = extraCheckerProviders.stream()
205+
.flatMap(p -> p.checkers(services.projectResolver(), services.clusterService()).stream())
206+
.toList();
207+
206208
return List.of(
207209
new PlanExecutor(
208210
new IndexResolver(services.client()),
209211
services.telemetryProvider().getMeterRegistry(),
210212
getLicenseState(),
211213
new EsqlQueryLog(services.clusterService().getClusterSettings(), services.slowLogFieldProvider()),
212-
extraCheckers,
213-
settings
214+
extraCheckers
214215
),
215216
new ExchangeService(
216217
services.clusterService().getSettings(),
@@ -349,6 +350,6 @@ public List<ExecutorBuilder<?>> getExecutorBuilders(Settings settings) {
349350

350351
@Override
351352
public void loadExtensions(ExtensionLoader loader) {
352-
extraCheckers.addAll(loader.loadExtensions(Verifier.ExtraCheckers.class));
353+
extraCheckerProviders.addAll(loader.loadExtensions(PlanCheckerProvider.class));
353354
}
354355
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ protected Writeable.Reader<ClusterComputeRequest> instanceReader() {
5757
protected NamedWriteableRegistry getNamedWriteableRegistry() {
5858
List<NamedWriteableRegistry.Entry> writeables = new ArrayList<>();
5959
writeables.addAll(new SearchModule(Settings.EMPTY, List.of()).getNamedWriteables());
60-
writeables.addAll(new EsqlPlugin(Settings.EMPTY).getNamedWriteables());
60+
writeables.addAll(new EsqlPlugin().getNamedWriteables());
6161
return new NamedWriteableRegistry(writeables);
6262
}
6363

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ protected Writeable.Reader<DataNodeRequest> instanceReader() {
6060
protected NamedWriteableRegistry getNamedWriteableRegistry() {
6161
List<NamedWriteableRegistry.Entry> writeables = new ArrayList<>();
6262
writeables.addAll(new SearchModule(Settings.EMPTY, List.of()).getNamedWriteables());
63-
writeables.addAll(new EsqlPlugin(Settings.EMPTY).getNamedWriteables());
63+
writeables.addAll(new EsqlPlugin().getNamedWriteables());
6464
return new NamedWriteableRegistry(writeables);
6565
}
6666

0 commit comments

Comments
 (0)