From 487eed3094e4148e85d53c61f324c3430e73fd27 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 17 Mar 2026 16:20:20 +0100 Subject: [PATCH 01/16] Add parsed statement support to EsqlQueryRequest Allow internal callers to supply a pre-built EsqlStatement that bypasses ES|QL string construction and parsing. EsqlSession now checks for a parsed statement before invoking the parser. --- .../xpack/esql/action/EsqlActionIT.java | 21 +++++++++++++++ .../xpack/esql/action/EsqlQueryRequest.java | 27 +++++++++++++++++++ .../xpack/esql/session/EsqlSession.java | 2 +- .../esql/action/EsqlQueryRequestTests.java | 17 ++++++++++++ 4 files changed, 66 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionIT.java index 683a782a12230..59c8daa99430a 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionIT.java @@ -57,8 +57,13 @@ import org.elasticsearch.xpack.core.esql.action.ColumnInfo; import org.elasticsearch.xpack.esql.VerificationException; import org.elasticsearch.xpack.esql.analysis.AnalyzerSettings; +import org.elasticsearch.xpack.esql.core.expression.Alias; +import org.elasticsearch.xpack.esql.core.expression.Literal; +import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; import org.elasticsearch.xpack.esql.parser.ParsingException; +import org.elasticsearch.xpack.esql.plan.EsqlStatement; +import org.elasticsearch.xpack.esql.plan.logical.Row; import org.elasticsearch.xpack.esql.planner.PlannerSettings; import org.elasticsearch.xpack.esql.plugin.QueryPragmas; import org.junit.Before; @@ -150,6 +155,22 @@ public void testRow() { } } + /** + * Verifies that a pre-built {@link EsqlStatement} supplied via + * {@link EsqlQueryRequest#syncEsqlQueryRequestWithPlan} is executed + * without going through ES|QL string parsing. + */ + public void testRowWithParsedStatement() { + var plan = new Row(Source.EMPTY, List.of(new Alias(Source.EMPTY, "x", new Literal(Source.EMPTY, 1, DataType.INTEGER)))); + var statement = new EsqlStatement(plan, List.of()); + EsqlQueryRequest request = EsqlQueryRequest.syncEsqlQueryRequestWithPlan(statement); + request.pragmas(getPragmas()); + try (EsqlQueryResponse response = run(request)) { + assertThat(response.columns(), equalTo(List.of(new ColumnInfoImpl("x", "integer", null)))); + assertEquals(List.of(List.of(1)), getValuesList(response)); + } + } + public void testRowWithFilter() { long value = randomLongBetween(0, Long.MAX_VALUE); try (EsqlQueryResponse response = run(syncEsqlQueryRequest("ROW " + value).filter(randomQueryFilter()))) { diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java index eaee8bee2fa9a..f579b7a6fd3a5 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java @@ -24,6 +24,7 @@ import org.elasticsearch.xpack.core.async.AsyncExecutionId; import org.elasticsearch.xpack.esql.Column; import org.elasticsearch.xpack.esql.parser.QueryParams; +import org.elasticsearch.xpack.esql.plan.EsqlStatement; import org.elasticsearch.xpack.esql.plugin.EsqlQueryStatus; import org.elasticsearch.xpack.esql.plugin.QueryPragmas; @@ -66,6 +67,14 @@ public class EsqlQueryRequest extends org.elasticsearch.xpack.core.esql.action.E */ private final Map> tables = new TreeMap<>(); + /** + * An optional pre-built statement that bypasses ES|QL string parsing. + * This is transient and never serialized over the wire. It's used by internal callers + * (such as the Prometheus REST endpoints) that construct a {@link EsqlStatement} directly + * instead of going through ES|QL string construction and parsing. + */ + private EsqlStatement parsedStatement; + public static EsqlQueryRequest syncEsqlQueryRequest(String query) { return new EsqlQueryRequest(false, query); } @@ -74,6 +83,17 @@ public static EsqlQueryRequest asyncEsqlQueryRequest(String query) { return new EsqlQueryRequest(true, query); } + /** + * Creates a synchronous request with a pre-built statement, bypassing ES|QL string parsing. + * The query string is only used for logging/display since the plan is already built. + */ + public static EsqlQueryRequest syncEsqlQueryRequestWithPlan(EsqlStatement statement) { + String queryText = statement.plan().sourceText(); + EsqlQueryRequest request = new EsqlQueryRequest(false, queryText.isEmpty() ? "[pre-built plan]" : queryText); + request.parsedStatement = statement; + return request; + } + private EsqlQueryRequest(boolean async, String query) { this.async = async; this.query = query; @@ -119,6 +139,13 @@ public String query() { return query; } + /** + * Returns the pre-built statement, or {@code null} if the query string should be parsed. + */ + public EsqlStatement parsedStatement() { + return parsedStatement; + } + public boolean async() { return async; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java index 39fe018fe7e2a..40679d8424937 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java @@ -245,7 +245,7 @@ public void execute( LOGGER.debug("ESQL query:\n{}", request.query()); TimeSpanMarker parsingProfile = executionInfo.queryProfile().parsing(); parsingProfile.start(); - EsqlStatement statement = parse(request); + EsqlStatement statement = request.parsedStatement() != null ? request.parsedStatement() : parse(request); gatherSettingsMetrics(statement); parsingProfile.stop(); viewResolver.replaceViews( diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java index fe8d91fa08997..d05c27ea71c4a 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java @@ -39,12 +39,16 @@ import org.elasticsearch.xpack.core.async.AsyncExecutionId; import org.elasticsearch.xpack.core.async.AsyncTask; import org.elasticsearch.xpack.esql.Column; +import org.elasticsearch.xpack.esql.core.expression.Alias; +import org.elasticsearch.xpack.esql.core.expression.Literal; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; import org.elasticsearch.xpack.esql.parser.ParserUtils; import org.elasticsearch.xpack.esql.parser.ParsingException; import org.elasticsearch.xpack.esql.parser.QueryParam; import org.elasticsearch.xpack.esql.parser.QueryParams; +import org.elasticsearch.xpack.esql.plan.EsqlStatement; +import org.elasticsearch.xpack.esql.plan.logical.Row; import org.elasticsearch.xpack.esql.plugin.EsqlQueryStatus; import java.io.IOException; @@ -74,6 +78,19 @@ public class EsqlQueryRequestTests extends ESTestCase { private final NamedWriteableRegistry namedWriteableRegistry = new NamedWriteableRegistry(List.of(EsqlQueryStatus.ENTRY)); + public void testSyncRequestWithPlanCarriesParsedStatement() { + var plan = new Row(Source.EMPTY, List.of(new Alias(Source.EMPTY, "x", new Literal(Source.EMPTY, 1, DataType.INTEGER)))); + var statement = new EsqlStatement(plan, List.of()); + EsqlQueryRequest request = EsqlQueryRequest.syncEsqlQueryRequestWithPlan(statement); + assertFalse(request.async()); + assertSame(statement, request.parsedStatement()); + } + + public void testSyncRequestFromStringHasNullParsedStatement() { + EsqlQueryRequest request = EsqlQueryRequest.syncEsqlQueryRequest("ROW x = 1"); + assertNull(request.parsedStatement()); + } + public void testParseFields() throws IOException { String query = randomAlphaOfLengthBetween(1, 100); boolean columnar = randomBoolean(); From 6ee6ef311281e40a1e2e8a90a39c5155cc80e26e Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 17 Mar 2026 19:00:43 +0100 Subject: [PATCH 02/16] Fix testRowWithParsedStatement failing in EsqlAsyncActionIT EsqlAsyncActionIT.run() was re-creating the request from only the query string, discarding the parsedStatement. For a pre-built plan, the query string is "[pre-built plan]" which fails ES|QL parsing with a token recognition error on '['. Add asyncEsqlQueryRequestWithPlan() factory method and propagate the parsedStatement when wrapping a sync request into an async one. --- .../xpack/esql/action/EsqlAsyncActionIT.java | 7 ++++++- .../xpack/esql/action/EsqlQueryRequest.java | 11 +++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlAsyncActionIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlAsyncActionIT.java index e476a07adb8ee..facb4aaef7ec3 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlAsyncActionIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlAsyncActionIT.java @@ -50,7 +50,12 @@ protected Collection> nodePlugins() { @Override public EsqlQueryResponse run(EsqlQueryRequest original) { - EsqlQueryRequest request = EsqlQueryRequest.asyncEsqlQueryRequest(original.query()); + EsqlQueryRequest request; + if (original.parsedStatement() != null) { + request = EsqlQueryRequest.asyncEsqlQueryRequestWithPlan(original.parsedStatement()); + } else { + request = EsqlQueryRequest.asyncEsqlQueryRequest(original.query()); + } request.pragmas(original.pragmas()); request.profile(original.profile()); request.acceptedPragmaRisks(true); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java index f579b7a6fd3a5..6f75733ed3db0 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java @@ -94,6 +94,17 @@ public static EsqlQueryRequest syncEsqlQueryRequestWithPlan(EsqlStatement statem return request; } + /** + * Creates an asynchronous request with a pre-built statement, bypassing ES|QL string parsing. + * The query string is only used for logging/display since the plan is already built. + */ + public static EsqlQueryRequest asyncEsqlQueryRequestWithPlan(EsqlStatement statement) { + String queryText = statement.plan().sourceText(); + EsqlQueryRequest request = new EsqlQueryRequest(true, queryText.isEmpty() ? "[pre-built plan]" : queryText); + request.parsedStatement = statement; + return request; + } + private EsqlQueryRequest(boolean async, String query) { this.async = async; this.query = query; From 140efdeb7d2a2fd0f50c633d998373ed09b43d7e Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Fri, 20 Mar 2026 11:47:25 +0100 Subject: [PATCH 03/16] Collect PlanTelemetry from pre-built logical plans When EsqlQueryRequest carries a pre-built EsqlStatement (bypassing the parser), TelemetryAware command labels and function names were not recorded because telemetry collection normally happens inside the parser (LogicalPlanBuilder.telemetryAccounting and ExpressionBuilder.visitFunctionName/castToType). Add EsqlSession.gatherPlanTelemetry, which does a single forEachDown pass over the plan tree to fill in PlanTelemetry post-hoc. UnresolvedFunction nodes (named calls as produced by the parser) are recorded by name; concrete Function instances (inline casts from the parser, or functions instantiated directly in programmatic plan builders such as the Prometheus plugin's PromqlQueryPlanBuilder) are resolved via the function registry. --- .../xpack/esql/session/EsqlSession.java | 46 +++++++++- .../session/EsqlSessionTelemetryTests.java | 90 +++++++++++++++++++ 2 files changed, 133 insertions(+), 3 deletions(-) create mode 100644 x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionTelemetryTests.java diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java index 40679d8424937..b17e1ebff3f8c 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java @@ -53,9 +53,11 @@ import org.elasticsearch.xpack.esql.analysis.UnmappedResolution; import org.elasticsearch.xpack.esql.analysis.Verifier; import org.elasticsearch.xpack.esql.approximation.Approximation; +import org.elasticsearch.xpack.esql.capabilities.TelemetryAware; import org.elasticsearch.xpack.esql.core.expression.Attribute; import org.elasticsearch.xpack.esql.core.expression.Expression; import org.elasticsearch.xpack.esql.core.expression.FoldContext; +import org.elasticsearch.xpack.esql.core.expression.function.Function; import org.elasticsearch.xpack.esql.core.querydsl.QueryDslTimestampBoundsExtractor; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.datasources.ExternalSourceResolution; @@ -63,6 +65,7 @@ import org.elasticsearch.xpack.esql.datasources.PartitionFilterHintExtractor; import org.elasticsearch.xpack.esql.enrich.EnrichPolicyResolver; import org.elasticsearch.xpack.esql.expression.function.EsqlFunctionRegistry; +import org.elasticsearch.xpack.esql.expression.function.UnresolvedFunction; import org.elasticsearch.xpack.esql.index.EsIndex; import org.elasticsearch.xpack.esql.index.IndexResolution; import org.elasticsearch.xpack.esql.inference.InferenceResolution; @@ -112,7 +115,6 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; -import java.util.function.Function; import static java.util.stream.Collectors.toSet; import static org.elasticsearch.xpack.esql.plan.QuerySettings.UNMAPPED_FIELDS; @@ -245,7 +247,13 @@ public void execute( LOGGER.debug("ESQL query:\n{}", request.query()); TimeSpanMarker parsingProfile = executionInfo.queryProfile().parsing(); parsingProfile.start(); - EsqlStatement statement = request.parsedStatement() != null ? request.parsedStatement() : parse(request); + EsqlStatement statement; + if (request.parsedStatement() != null) { + statement = request.parsedStatement(); + gatherPlanTelemetry(statement.plan(), planTelemetry); + } else { + statement = parse(request); + } gatherSettingsMetrics(statement); parsingProfile.stop(); viewResolver.replaceViews( @@ -571,7 +579,11 @@ private void executeSubPlans( * @param newMainPlan callback to build the new main plan based on the subplan results * @param cleanup callback to release any resources hold by the subplan results */ - private record SubPlanAndCallback(LogicalPlan subPlan, Function newMainPlan, Runnable cleanup) {}; + private record SubPlanAndCallback( + LogicalPlan subPlan, + java.util.function.Function newMainPlan, + Runnable cleanup + ) {}; private SubPlanAndCallback firstSubPlan(LogicalPlan optimizedPlan, Approximation approximation, Set subPlansResults) { if (approximation != null) { @@ -707,6 +719,34 @@ private EsqlStatement parse(EsqlQueryRequest request) { ); } + /** + * Populates {@code planTelemetry} from a pre-built logical plan tree, mirroring what the parser + * does via {@code LogicalPlanBuilder.telemetryAccounting} and {@code ExpressionBuilder.visitFunctionName}. + * A single {@code forEachDown} pass collects both {@link TelemetryAware} command labels and + * {@link Function} names from each node's expressions, avoiding a second traversal of the plan tree. + *

+ * The parser produces {@link UnresolvedFunction} nodes for named function calls (e.g. + * {@code TO_LONG(x)}), so the {@code instanceof UnresolvedFunction} branch mirrors that path. + * The {@code else} branch handles concrete {@link Function} instances, which arise in two cases: + * inline cast expressions parsed by {@code ExpressionBuilder.castToType} (e.g. {@code x::long}), + * and functions instantiated directly in programmatically-built plans (e.g. Prometheus plan + * builders). + */ + static void gatherPlanTelemetry(LogicalPlan plan, PlanTelemetry planTelemetry) { + plan.forEachDown(node -> { + if (node instanceof TelemetryAware ta) { + planTelemetry.command(ta); + } + node.forEachExpression(Function.class, f -> { + if (f instanceof UnresolvedFunction uf) { + planTelemetry.function(uf.name()); + } else { + planTelemetry.function(f.getClass()); + } + }); + }); + } + private void gatherSettingsMetrics(EsqlStatement statement) { if (metrics == null || statement.settings() == null) { return; diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionTelemetryTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionTelemetryTests.java new file mode 100644 index 0000000000000..5c7fb96edce38 --- /dev/null +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionTelemetryTests.java @@ -0,0 +1,90 @@ +/* + * 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.session; + +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.esql.expression.function.EsqlFunctionRegistry; +import org.elasticsearch.xpack.esql.inference.InferenceSettings; +import org.elasticsearch.xpack.esql.parser.EsqlConfig; +import org.elasticsearch.xpack.esql.parser.EsqlParser; +import org.elasticsearch.xpack.esql.parser.QueryParams; +import org.elasticsearch.xpack.esql.telemetry.PlanTelemetry; + +import static org.hamcrest.Matchers.equalTo; + +/** + * Tests for {@link EsqlSession#gatherPlanTelemetry}, which populates {@link PlanTelemetry} from + * a pre-built logical plan tree (bypassing the parser). This matters for internal callers such as + * the Prometheus plugin that construct plans programmatically via plan builders rather than parsing + * an ES|QL string. + *

+ * Each test parses a query string to obtain both the reference {@link PlanTelemetry} and the + * {@link org.elasticsearch.xpack.esql.plan.logical.LogicalPlan}, then asserts that running + * {@link EsqlSession#gatherPlanTelemetry} post-hoc on that same plan produces identical telemetry. + */ +public class EsqlSessionTelemetryTests extends ESTestCase { + + private static final EsqlFunctionRegistry FUNCTION_REGISTRY = new EsqlFunctionRegistry(); + private static final EsqlParser PARSER = new EsqlParser(new EsqlConfig(FUNCTION_REGISTRY)); + + /** + * A single {@link org.elasticsearch.xpack.esql.capabilities.TelemetryAware} node with no + * function expressions: only the command label should be recorded. + */ + public void testTelemetryAwareNodeWithNoFunctions() { + assertEquivalentTelemetry("ROW x = 1"); + } + + /** + * A chain of {@link org.elasticsearch.xpack.esql.capabilities.TelemetryAware} nodes with no + * function expressions: all command labels across the tree should be recorded. + */ + public void testTelemetryAwareNodesAcrossTreeWithNoFunctions() { + assertEquivalentTelemetry("ROW x = 1 | WHERE true | LIMIT 100"); + } + + /** + * An {@link org.elasticsearch.xpack.esql.expression.function.UnresolvedFunction} in an + * expression (named function call, as produced by the parser): the function name should be + * recorded directly — exercises the {@code instanceof UnresolvedFunction} branch in + * {@link EsqlSession#gatherPlanTelemetry}. + */ + public void testUnresolvedFunction() { + assertEquivalentTelemetry("ROW y = 1 | EVAL x = TO_LONG(y)"); + } + + /** + * A concrete {@link org.elasticsearch.xpack.esql.core.expression.function.Function} instance + * (not an {@link org.elasticsearch.xpack.esql.expression.function.UnresolvedFunction}) in an + * expression. The parser produces concrete function instances for inline cast expressions (e.g. + * {@code y::long}), so this exercises the {@code else} branch in + * {@link EsqlSession#gatherPlanTelemetry} via a parsed query. The same branch also covers + * functions instantiated directly in programmatically-built plans (e.g. by + * {@code PromqlQueryPlanBuilder}). + */ + public void testConcreteFunction() { + assertEquivalentTelemetry("ROW y = 1 | EVAL x = y::long"); + } + + /** + * Parses {@code query} to obtain both the reference {@link PlanTelemetry} and the logical plan, + * then asserts that {@link EsqlSession#gatherPlanTelemetry} produces identical telemetry when + * run post-hoc on that same plan. + */ + private static void assertEquivalentTelemetry(String query) { + PlanTelemetry fromParsing = new PlanTelemetry(FUNCTION_REGISTRY); + var plan = PARSER.parseQuery(query, new QueryParams(), fromParsing, new InferenceSettings(Settings.EMPTY)); + + PlanTelemetry fromPostHoc = new PlanTelemetry(FUNCTION_REGISTRY); + EsqlSession.gatherPlanTelemetry(plan, fromPostHoc); + + assertThat("commands", fromPostHoc.commands(), equalTo(fromParsing.commands())); + assertThat("functions", fromPostHoc.functions(), equalTo(fromParsing.functions())); + } +} From 08a1ae84470baeb45e3a17d0dcbb3eb0e43f510a Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Fri, 20 Mar 2026 12:04:08 +0100 Subject: [PATCH 04/16] Validate QuerySettings for pre-built parsed statements When a pre-built parsedStatement is provided in EsqlQueryRequest, the normal parsing path is bypassed, which also skipped QuerySettings.validate(). This meant snapshot-only settings (e.g. SET approximation=...) could pass through on non-snapshot builds. Apply the same validation as the regular parse path. --- .../java/org/elasticsearch/xpack/esql/session/EsqlSession.java | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java index b17e1ebff3f8c..d6425a92667f6 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java @@ -251,6 +251,7 @@ public void execute( if (request.parsedStatement() != null) { statement = request.parsedStatement(); gatherPlanTelemetry(statement.plan(), planTelemetry); + QuerySettings.validate(statement, SettingsValidationContext.from(remoteClusterService)); } else { statement = parse(request); } From 0c63022d60667305ddc3daf9180731dd6bb71a4a Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Sat, 21 Mar 2026 09:21:07 +0100 Subject: [PATCH 05/16] Move plan telemetry collection out of the parser and into a single post-hoc pass Previously, telemetry (commands, functions, settings) was collected inline during parsing via context.telemetry() hooks in LogicalPlanBuilder and ExpressionBuilder. This moves all collection to a single post-hoc gatherPlanTelemetry pass over the resolved plan tree. - Remove PlanTelemetry from ParsingContext and all context.telemetry() call sites in ExpressionBuilder and LogicalPlanBuilder; delete telemetryAccounting - Remove the PlanTelemetry parameter from EsqlParser.createStatement/parse/etc. - Call gatherPlanTelemetry on viewResolution.plan() in analyseAndExecute() so view-expanded nodes are also included - Guard gatherPlanTelemetry against TelemetryAware nodes with a null label (e.g. LOOKUP JOIN relation refs) and Function subclasses not registered in the function registry (binary operators, predicates, etc.) - Add EsqlFunctionRegistry.functionExists(Class) and expose PlanTelemetry.functionRegistry() to support those guards - Remove ViewService's dead PlanTelemetry field - Delete EsqlSessionTelemetryTests, which existed solely to verify equivalence between the old parser-inline and post-hoc paths --- .../function/EsqlFunctionRegistry.java | 4 + .../xpack/esql/parser/EsqlParser.java | 46 +++------- .../xpack/esql/parser/ExpressionBuilder.java | 8 +- .../xpack/esql/parser/LogicalPlanBuilder.java | 20 +---- .../xpack/esql/plugin/EsqlPlugin.java | 2 +- .../xpack/esql/session/EsqlSession.java | 41 +++++---- .../xpack/esql/telemetry/PlanTelemetry.java | 4 + .../xpack/esql/view/ViewService.java | 8 +- .../elasticsearch/xpack/esql/CsvTests.java | 1 - .../parser/AbstractStatementParserTests.java | 8 +- .../session/EsqlSessionTelemetryTests.java | 90 ------------------- .../xpack/esql/view/InMemoryViewService.java | 3 +- .../esql/view/InMemoryViewServiceTests.java | 3 - 13 files changed, 50 insertions(+), 188 deletions(-) delete mode 100644 x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionTelemetryTests.java diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java index e3890e24196de..374d349f8e919 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java @@ -351,6 +351,10 @@ public boolean functionExists(String functionName) { return defs.containsKey(functionName); } + public boolean functionExists(Class clazz) { + return names.containsKey(clazz); + } + public String functionName(Class clazz) { String name = names.get(clazz); Check.notNull(name, "Cannot find function by class {}", clazz); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlParser.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlParser.java index d32bb57fc0659..e0e79bbe07ebd 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlParser.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlParser.java @@ -26,7 +26,6 @@ import org.elasticsearch.xpack.esql.plan.QuerySettings; import org.elasticsearch.xpack.esql.plan.SettingsValidationContext; import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan; -import org.elasticsearch.xpack.esql.telemetry.PlanTelemetry; import java.util.BitSet; import java.util.EmptyStackException; @@ -106,15 +105,15 @@ public LogicalPlan parseQuery(String query) { // testing utility public LogicalPlan parseQuery(String query, QueryParams params) { - return parseQuery(query, params, new PlanTelemetry(config.functionRegistry()), new InferenceSettings(Settings.EMPTY)); + return parseQuery(query, params, new InferenceSettings(Settings.EMPTY)); } // testing utility - public LogicalPlan parseQuery(String query, QueryParams params, PlanTelemetry metrics, InferenceSettings inferenceSettings) { + public LogicalPlan parseQuery(String query, QueryParams params, InferenceSettings inferenceSettings) { if (log.isDebugEnabled()) { log.debug("Parsing as statement: {}", query); } - return invokeParser(query, params, metrics, inferenceSettings, null, EsqlBaseParser::singleStatement, AstBuilder::plan); + return invokeParser(query, params, inferenceSettings, null, EsqlBaseParser::singleStatement, AstBuilder::plan); } // testing utility @@ -124,33 +123,22 @@ public EsqlStatement createStatement(String query) { // testing utility public EsqlStatement unvalidatedStatement(String query, QueryParams params) { - return createStatement(query, params, new PlanTelemetry(config.functionRegistry()), new InferenceSettings(Settings.EMPTY), null); + return createStatement(query, params, new InferenceSettings(Settings.EMPTY), null); } // testing utility public EsqlStatement createStatement(String query, QueryParams params) { - return parse( - query, - params, - new SettingsValidationContext(false, config.isDevVersion()), // TODO: wire CPS in - new PlanTelemetry(config.functionRegistry()), - new InferenceSettings(Settings.EMPTY) - ); + var parsed = parse(query, params, new InferenceSettings(Settings.EMPTY)); + QuerySettings.validate(parsed, new SettingsValidationContext(false, config.isDevVersion())); // TODO: wire CPS in + return parsed; } - public EsqlStatement parse( - String query, - QueryParams params, - SettingsValidationContext settingsValidationCtx, - PlanTelemetry metrics, - InferenceSettings inferenceSettings - ) { - var parsed = createStatement(query, params, metrics, inferenceSettings, null); + public EsqlStatement parse(String query, QueryParams params, InferenceSettings inferenceSettings) { + var parsed = createStatement(query, params, inferenceSettings, null); if (log.isDebugEnabled()) { log.debug("Parsed logical plan:\n{}", parsed.plan()); log.debug("Parsed settings:\n[{}]", parsed.settings().stream().map(QuerySetting::toString).collect(joining("; "))); } - QuerySettings.validate(parsed, settingsValidationCtx); return parsed; } @@ -162,11 +150,10 @@ public EsqlStatement parseView( String query, QueryParams params, SettingsValidationContext settingsValidationCtx, - PlanTelemetry metrics, InferenceSettings inferenceSettings, String viewName ) { - var parsed = createStatement(query, params, metrics, inferenceSettings, viewName); + var parsed = createStatement(query, params, inferenceSettings, viewName); if (log.isDebugEnabled()) { log.debug("Parsed view '{}' logical plan:\n{}", viewName, parsed.plan()); log.debug("Parsed settings:\n[{}]", parsed.settings().stream().map(QuerySetting::toString).collect(joining("; "))); @@ -175,23 +162,16 @@ public EsqlStatement parseView( return parsed; } - private EsqlStatement createStatement( - String query, - QueryParams params, - PlanTelemetry metrics, - InferenceSettings inferenceSettings, - String viewName - ) { + private EsqlStatement createStatement(String query, QueryParams params, InferenceSettings inferenceSettings, String viewName) { if (log.isDebugEnabled()) { log.debug("Parsing as statement: {}", query); } - return invokeParser(query, params, metrics, inferenceSettings, viewName, EsqlBaseParser::statements, AstBuilder::statement); + return invokeParser(query, params, inferenceSettings, viewName, EsqlBaseParser::statements, AstBuilder::statement); } private T invokeParser( String query, QueryParams params, - PlanTelemetry metrics, InferenceSettings inferenceSettings, String viewName, Function parseFunction, @@ -227,7 +207,7 @@ private T invokeParser( log.trace("Parse tree: {}", tree.toStringTree()); } - return result.apply(new AstBuilder(new ExpressionBuilder.ParsingContext(params, metrics, inferenceSettings, viewName)), tree); + return result.apply(new AstBuilder(new ExpressionBuilder.ParsingContext(params, inferenceSettings, viewName)), tree); } catch (StackOverflowError e) { throw new ParsingException("ESQL statement is too large, causing stack overflow when generating the parsing tree: [{}]", query); // likely thrown by an invalid popMode (such as extra closing parenthesis) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/ExpressionBuilder.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/ExpressionBuilder.java index a8db7ae474b2a..4e03bf5d124ab 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/ExpressionBuilder.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/ExpressionBuilder.java @@ -69,7 +69,6 @@ import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.LessThan; import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.LessThanOrEqual; import org.elasticsearch.xpack.esql.inference.InferenceSettings; -import org.elasticsearch.xpack.esql.telemetry.PlanTelemetry; import org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter; import java.math.BigInteger; @@ -125,7 +124,7 @@ public abstract class ExpressionBuilder extends IdentifierBuilder { protected final ParsingContext context; - public record ParsingContext(QueryParams params, PlanTelemetry telemetry, InferenceSettings inferenceSettings, String viewName) {} + public record ParsingContext(QueryParams params, InferenceSettings inferenceSettings, String viewName) {} ExpressionBuilder(ParsingContext context) { this.context = context; @@ -721,9 +720,7 @@ public Expression visitFunctionExpression(EsqlBaseParser.FunctionExpressionConte @Override public String visitFunctionName(EsqlBaseParser.FunctionNameContext ctx) { - String name = functionName(ctx); - context.telemetry().function(name); - return name; + return functionName(ctx); } private String functionName(EsqlBaseParser.FunctionNameContext ctx) { @@ -801,7 +798,6 @@ private Expression castToType(Source source, ParseTree parseTree, EsqlBaseParser } Expression expr = expression(parseTree); var convertFunction = converterToFactory.apply(source, expr, ConfigurationAware.CONFIGURATION_MARKER); - context.telemetry().function(convertFunction.getClass()); return convertFunction; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/LogicalPlanBuilder.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/LogicalPlanBuilder.java index 676d866a4fa27..9d1084a9465dd 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/LogicalPlanBuilder.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/LogicalPlanBuilder.java @@ -23,7 +23,6 @@ import org.elasticsearch.transport.RemoteClusterAware; import org.elasticsearch.xpack.esql.VerificationException; import org.elasticsearch.xpack.esql.action.EsqlCapabilities; -import org.elasticsearch.xpack.esql.capabilities.TelemetryAware; import org.elasticsearch.xpack.esql.common.Failure; import org.elasticsearch.xpack.esql.core.expression.Alias; import org.elasticsearch.xpack.esql.core.expression.Attribute; @@ -180,16 +179,12 @@ protected List plans(List ctxs) { @Override public LogicalPlan visitSingleStatement(EsqlBaseParser.SingleStatementContext ctx) { - var plan = plan(ctx.query()); - telemetryAccounting(plan); - return plan; + return plan(ctx.query()); } @Override public QuerySetting visitSetCommand(EsqlBaseParser.SetCommandContext ctx) { - var field = visitSetField(ctx.setField()); - context.telemetry().setting(field.name()); - return new QuerySetting(source(ctx), field); + return new QuerySetting(source(ctx), visitSetField(ctx.setField())); } @Override @@ -216,7 +211,6 @@ public LogicalPlan visitCompositeQuery(EsqlBaseParser.CompositeQueryContext ctx) } try { LogicalPlan input = plan(ctx.query()); - telemetryAccounting(input); PlanFactory makePlan = typedParsing(this, ctx.processingCommand(), PlanFactory.class); return makePlan.apply(input); } finally { @@ -224,13 +218,6 @@ public LogicalPlan visitCompositeQuery(EsqlBaseParser.CompositeQueryContext ctx) } } - private LogicalPlan telemetryAccounting(LogicalPlan node) { - if (node instanceof TelemetryAware ma) { - this.context.telemetry().command(ma); - } - return node; - } - @Override public PlanFactory visitEvalCommand(EsqlBaseParser.EvalCommandContext ctx) { return p -> new Eval(source(ctx), p, visitFields(ctx.fields())); @@ -402,7 +389,6 @@ private LogicalPlan visitRelation(Source source, SourceCommand command, EsqlBase List mainQueryAndSubqueries = new ArrayList<>(subqueries.size() + 1); if (table.indexPattern().isEmpty() == false) { mainQueryAndSubqueries.add(unresolvedRelation); - telemetryAccounting(unresolvedRelation); } mainQueryAndSubqueries.addAll(subqueries); @@ -438,10 +424,8 @@ public LogicalPlan visitSubquery(EsqlBaseParser.SubqueryContext ctx) { LogicalPlan plan = visitFromCommand(fromCtx); List processingCommands = visitList(this, ctx.processingCommand(), PlanFactory.class); for (PlanFactory processingCommand : processingCommands) { - telemetryAccounting(plan); plan = processingCommand.apply(plan); } - telemetryAccounting(plan); return plan; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlPlugin.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlPlugin.java index 2aff4a3a455c6..098156ce35baf 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlPlugin.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlPlugin.java @@ -281,7 +281,7 @@ public Collection createComponents(PluginServices services) { if (ESQL_VIEWS_FEATURE_FLAG.isEnabled()) { components = new ArrayList<>(components); components.add(new ViewResolver(services.clusterService(), services.projectResolver(), services.client())); - components.add(new ViewService(services.clusterService(), functionRegistry, parser)); + components.add(new ViewService(services.clusterService(), parser)); } return components; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java index 43c677e889941..5c8b1ce4e2bfe 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java @@ -251,11 +251,10 @@ public void execute( EsqlStatement statement; if (request.parsedStatement() != null) { statement = request.parsedStatement(); - gatherPlanTelemetry(statement.plan(), planTelemetry); - QuerySettings.validate(statement, SettingsValidationContext.from(remoteClusterService)); } else { statement = parse(request); } + QuerySettings.validate(statement, SettingsValidationContext.from(remoteClusterService)); gatherSettingsMetrics(statement); parsingProfile.stop(); viewResolver.replaceViews( @@ -264,7 +263,6 @@ public void execute( query, request.params(), SettingsValidationContext.from(remoteClusterService), - planTelemetry, inferenceService.inferenceSettings(), viewName ).plan(), @@ -283,6 +281,7 @@ private void analyseAndExecute( ActionListener> listener ) { assert ThreadPool.assertCurrentThreadPool(ThreadPool.Names.SEARCH); + gatherPlanTelemetry(viewResolution.plan(), planTelemetry); PlanTimeProfile planTimeProfile = request.profile() ? new PlanTimeProfile() : null; ZoneId timeZone = request.timeZone() == null @@ -717,37 +716,37 @@ private static void releaseLocalRelationBlocks(AtomicReference localRelati } private EsqlStatement parse(EsqlQueryRequest request) { - return parser.parse( - request.query(), - request.params(), - SettingsValidationContext.from(remoteClusterService), - planTelemetry, - inferenceService.inferenceSettings() - ); + return parser.parse(request.query(), request.params(), inferenceService.inferenceSettings()); } /** - * Populates {@code planTelemetry} from a pre-built logical plan tree, mirroring what the parser - * does via {@code LogicalPlanBuilder.telemetryAccounting} and {@code ExpressionBuilder.visitFunctionName}. + * Populates {@code planTelemetry} from a logical plan tree. Called on the view-resolved plan in + * {@link #analyseAndExecute}, so it captures all nodes: the original statement plus any commands + * and functions introduced by view expansion. + *

* A single {@code forEachDown} pass collects both {@link TelemetryAware} command labels and - * {@link Function} names from each node's expressions, avoiding a second traversal of the plan tree. + * {@link Function} names. Named function calls (e.g. {@code TO_LONG(x)}) produce + * {@link UnresolvedFunction} nodes from the parser; the {@code else} branch covers concrete + * {@link Function} instances that arise from inline cast expressions (e.g. {@code x::long}) or + * programmatically-built plans (e.g. Prometheus plan builders). *

- * The parser produces {@link UnresolvedFunction} nodes for named function calls (e.g. - * {@code TO_LONG(x)}), so the {@code instanceof UnresolvedFunction} branch mirrors that path. - * The {@code else} branch handles concrete {@link Function} instances, which arise in two cases: - * inline cast expressions parsed by {@code ExpressionBuilder.castToType} (e.g. {@code x::long}), - * and functions instantiated directly in programmatically-built plans (e.g. Prometheus plan - * builders). + * Not all {@link TelemetryAware} nodes have a label (e.g. lookup-table {@link + * org.elasticsearch.xpack.esql.plan.logical.UnresolvedRelation} nodes introduced by + * {@code LOOKUP JOIN} have a {@code null} command name); those are skipped. + * Similarly, not all {@link Function} subclasses are user-callable functions registered in the + * function registry (e.g. binary operators and predicates like {@code Add} or {@code GreaterThan} + * are {@link Function} subclasses but are never in the registry); those are also skipped. */ static void gatherPlanTelemetry(LogicalPlan plan, PlanTelemetry planTelemetry) { + EsqlFunctionRegistry registry = planTelemetry.functionRegistry().snapshotRegistry(); plan.forEachDown(node -> { - if (node instanceof TelemetryAware ta) { + if (node instanceof TelemetryAware ta && ta.telemetryLabel() != null) { planTelemetry.command(ta); } node.forEachExpression(Function.class, f -> { if (f instanceof UnresolvedFunction uf) { planTelemetry.function(uf.name()); - } else { + } else if (registry.functionExists(f.getClass())) { planTelemetry.function(f.getClass()); } }); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/telemetry/PlanTelemetry.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/telemetry/PlanTelemetry.java index e6fed43b27400..693cdb95e5efe 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/telemetry/PlanTelemetry.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/telemetry/PlanTelemetry.java @@ -34,6 +34,10 @@ private static void add(Map map, String key) { map.compute(key.toUpperCase(Locale.ROOT), (k, count) -> count == null ? 1 : count + 1); } + public EsqlFunctionRegistry functionRegistry() { + return functionRegistry; + } + public void linkedProjectsCount(int linkedProjectsCount) { this.linkedProjectsCount = linkedProjectsCount; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/ViewService.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/ViewService.java index 6395c4e52f32f..b76b770b29f37 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/ViewService.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/ViewService.java @@ -29,11 +29,9 @@ import org.elasticsearch.logging.LogManager; import org.elasticsearch.logging.Logger; import org.elasticsearch.xpack.core.esql.EsqlFeatureFlags; -import org.elasticsearch.xpack.esql.expression.function.EsqlFunctionRegistry; import org.elasticsearch.xpack.esql.inference.InferenceSettings; import org.elasticsearch.xpack.esql.parser.EsqlParser; import org.elasticsearch.xpack.esql.parser.QueryParams; -import org.elasticsearch.xpack.esql.telemetry.PlanTelemetry; import java.util.HashMap; import java.util.Map; @@ -44,7 +42,6 @@ public class ViewService { private static final InferenceSettings EMPTY_INFERENCE_SETTINGS = new InferenceSettings(Settings.EMPTY); private final EsqlParser parser; - private final PlanTelemetry telemetry; protected final ClusterService clusterService; private final MasterServiceTaskQueue taskQueue; @@ -69,7 +66,7 @@ public class ViewService { private volatile int maxViewsCount; private volatile int maxViewLength; - public ViewService(ClusterService clusterService, EsqlFunctionRegistry functionRegistry, EsqlParser parser) { + public ViewService(ClusterService clusterService, EsqlParser parser) { this.clusterService = clusterService; this.parser = parser; this.taskQueue = clusterService.createTaskQueue( @@ -77,7 +74,6 @@ public ViewService(ClusterService clusterService, EsqlFunctionRegistry functionR Priority.NORMAL, new SequentialAckingBatchedTaskExecutor<>() ); - this.telemetry = new PlanTelemetry(functionRegistry); clusterService.getClusterSettings().initializeAndWatch(MAX_VIEWS_COUNT_SETTING, v -> this.maxViewsCount = v); clusterService.getClusterSettings().initializeAndWatch(MAX_VIEW_LENGTH_SETTING, v -> this.maxViewLength = v); } @@ -204,7 +200,7 @@ void validatePutView(ProjectMetadata metadata, View view) { ); }); // Parse the query to ensure it's valid, this will throw appropriate exceptions if not - parser.parseQuery(view.query(), new QueryParams(), telemetry, EMPTY_INFERENCE_SETTINGS); + parser.parseQuery(view.query(), new QueryParams()); } /** diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java index ebca1e03c2011..6e253d0a0223d 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java @@ -677,7 +677,6 @@ private LogicalPlan parseView(String query, String viewName) { query, new QueryParams(), new SettingsValidationContext(false, false), - new PlanTelemetry(TEST_FUNCTION_REGISTRY), new InferenceSettings(Settings.EMPTY), viewName ).plan(); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java index e62ef53770282..5586293121dd1 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java @@ -24,7 +24,6 @@ import org.elasticsearch.xpack.esql.plan.IndexPattern; import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan; import org.elasticsearch.xpack.esql.plan.logical.UnresolvedRelation; -import org.elasticsearch.xpack.esql.telemetry.PlanTelemetry; import java.math.BigInteger; import java.util.ArrayList; @@ -81,12 +80,7 @@ LogicalPlan processingCommand(String e) { } LogicalPlan processingCommand(String e, QueryParams params, Settings settings) { - return TEST_PARSER.parseQuery( - "row a = 1 | " + e, - params, - new PlanTelemetry(TEST_FUNCTION_REGISTRY), - new InferenceSettings(settings) - ); + return TEST_PARSER.parseQuery("row a = 1 | " + e, params, new InferenceSettings(settings)); } static UnresolvedAttribute attribute(String name) { diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionTelemetryTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionTelemetryTests.java deleted file mode 100644 index 5c7fb96edce38..0000000000000 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionTelemetryTests.java +++ /dev/null @@ -1,90 +0,0 @@ -/* - * 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.session; - -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.xpack.esql.expression.function.EsqlFunctionRegistry; -import org.elasticsearch.xpack.esql.inference.InferenceSettings; -import org.elasticsearch.xpack.esql.parser.EsqlConfig; -import org.elasticsearch.xpack.esql.parser.EsqlParser; -import org.elasticsearch.xpack.esql.parser.QueryParams; -import org.elasticsearch.xpack.esql.telemetry.PlanTelemetry; - -import static org.hamcrest.Matchers.equalTo; - -/** - * Tests for {@link EsqlSession#gatherPlanTelemetry}, which populates {@link PlanTelemetry} from - * a pre-built logical plan tree (bypassing the parser). This matters for internal callers such as - * the Prometheus plugin that construct plans programmatically via plan builders rather than parsing - * an ES|QL string. - *

- * Each test parses a query string to obtain both the reference {@link PlanTelemetry} and the - * {@link org.elasticsearch.xpack.esql.plan.logical.LogicalPlan}, then asserts that running - * {@link EsqlSession#gatherPlanTelemetry} post-hoc on that same plan produces identical telemetry. - */ -public class EsqlSessionTelemetryTests extends ESTestCase { - - private static final EsqlFunctionRegistry FUNCTION_REGISTRY = new EsqlFunctionRegistry(); - private static final EsqlParser PARSER = new EsqlParser(new EsqlConfig(FUNCTION_REGISTRY)); - - /** - * A single {@link org.elasticsearch.xpack.esql.capabilities.TelemetryAware} node with no - * function expressions: only the command label should be recorded. - */ - public void testTelemetryAwareNodeWithNoFunctions() { - assertEquivalentTelemetry("ROW x = 1"); - } - - /** - * A chain of {@link org.elasticsearch.xpack.esql.capabilities.TelemetryAware} nodes with no - * function expressions: all command labels across the tree should be recorded. - */ - public void testTelemetryAwareNodesAcrossTreeWithNoFunctions() { - assertEquivalentTelemetry("ROW x = 1 | WHERE true | LIMIT 100"); - } - - /** - * An {@link org.elasticsearch.xpack.esql.expression.function.UnresolvedFunction} in an - * expression (named function call, as produced by the parser): the function name should be - * recorded directly — exercises the {@code instanceof UnresolvedFunction} branch in - * {@link EsqlSession#gatherPlanTelemetry}. - */ - public void testUnresolvedFunction() { - assertEquivalentTelemetry("ROW y = 1 | EVAL x = TO_LONG(y)"); - } - - /** - * A concrete {@link org.elasticsearch.xpack.esql.core.expression.function.Function} instance - * (not an {@link org.elasticsearch.xpack.esql.expression.function.UnresolvedFunction}) in an - * expression. The parser produces concrete function instances for inline cast expressions (e.g. - * {@code y::long}), so this exercises the {@code else} branch in - * {@link EsqlSession#gatherPlanTelemetry} via a parsed query. The same branch also covers - * functions instantiated directly in programmatically-built plans (e.g. by - * {@code PromqlQueryPlanBuilder}). - */ - public void testConcreteFunction() { - assertEquivalentTelemetry("ROW y = 1 | EVAL x = y::long"); - } - - /** - * Parses {@code query} to obtain both the reference {@link PlanTelemetry} and the logical plan, - * then asserts that {@link EsqlSession#gatherPlanTelemetry} produces identical telemetry when - * run post-hoc on that same plan. - */ - private static void assertEquivalentTelemetry(String query) { - PlanTelemetry fromParsing = new PlanTelemetry(FUNCTION_REGISTRY); - var plan = PARSER.parseQuery(query, new QueryParams(), fromParsing, new InferenceSettings(Settings.EMPTY)); - - PlanTelemetry fromPostHoc = new PlanTelemetry(FUNCTION_REGISTRY); - EsqlSession.gatherPlanTelemetry(plan, fromPostHoc); - - assertThat("commands", fromPostHoc.commands(), equalTo(fromParsing.commands())); - assertThat("functions", fromPostHoc.functions(), equalTo(fromParsing.functions())); - } -} diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewService.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewService.java index 830174b6e0f8c..ce2e3f4f97eaf 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewService.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewService.java @@ -35,7 +35,6 @@ import java.util.stream.Collectors; import static org.elasticsearch.test.ESTestCase.indexSettings; -import static org.elasticsearch.xpack.esql.EsqlTestUtils.TEST_FUNCTION_REGISTRY; import static org.elasticsearch.xpack.esql.EsqlTestUtils.TEST_PARSER; import static org.elasticsearch.xpack.esql.view.ViewResolver.MAX_VIEW_DEPTH_SETTING; @@ -72,7 +71,7 @@ private static InMemoryViewService makeViewService(ThreadPool threadPool, Settin } private InMemoryViewService(ClusterService clusterService, ThreadPool threadPool, ViewMetadata metadata) { - super(clusterService, TEST_FUNCTION_REGISTRY, TEST_PARSER); + super(clusterService, TEST_PARSER); this.threadPool = threadPool; this.viewMetadata = metadata; } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewServiceTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewServiceTests.java index 68f61692ea038..a816775f019ee 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewServiceTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewServiceTests.java @@ -34,7 +34,6 @@ import org.elasticsearch.xpack.esql.plan.logical.UnionAll; import org.elasticsearch.xpack.esql.plan.logical.UnresolvedRelation; import org.elasticsearch.xpack.esql.session.Configuration; -import org.elasticsearch.xpack.esql.telemetry.PlanTelemetry; import org.hamcrest.BaseMatcher; import org.hamcrest.Description; import org.hamcrest.Matcher; @@ -73,7 +72,6 @@ public class InMemoryViewServiceTests extends AbstractStatementParserTests { static InMemoryViewService viewService; static InMemoryViewResolver viewResolver; - PlanTelemetry telemetry = new PlanTelemetry(TEST_FUNCTION_REGISTRY); QueryParams queryParams = new QueryParams(); ProjectId projectId = ProjectId.DEFAULT; @@ -995,7 +993,6 @@ private LogicalPlan parse(String query, String viewName) { query, queryParams, new SettingsValidationContext(false, false), - telemetry, EMPTY_INFERENCE_SETTINGS, viewName ).plan(); From d6d25ccd22f1998701d1c0cf358d6d27931fbb0e Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Mon, 23 Mar 2026 07:36:04 +0100 Subject: [PATCH 06/16] Introduce PreparedEsqlQueryRequest for pre-built ES|QL statements Adds a PreparedEsqlQueryRequest subclass that carries a pre-built EsqlStatement, bypassing the parser. Includes all supporting changes: - Override writeTo to signal local-only execution - Move QuerySettings validation into EsqlQueryRequest#parse so it applies to both parsed and pre-built paths - Add copy constructor to EsqlQueryRequest; use it in maybeWrapAsPrepared to eliminate the redundant garbage query-string overwrite - Separate sourceText (full original query) from display string in EsqlQueryRequest; fix sourceTextForConfiguration to return the full original query and fix a Source position validation crash on multi-node clusters - Smoke-test PreparedEsqlQueryRequest via random promotion in ITs --- .../core/esql/action/EsqlQueryRequest.java | 2 + .../action/AbstractEsqlIntegTestCase.java | 18 ++- .../xpack/esql/action/EsqlActionIT.java | 21 ---- .../xpack/esql/action/EsqlAsyncActionIT.java | 4 +- .../xpack/esql/action/EsqlDisruptionIT.java | 4 +- .../xpack/esql/action/EsqlQueryLoggingIT.java | 18 ++- .../xpack/esql/action/EsqlQueryRequest.java | 96 +++++++++------- .../esql/action/EsqlResponseListener.java | 2 +- .../esql/action/PreparedEsqlQueryRequest.java | 107 ++++++++++++++++++ .../esql/action/RestEsqlQueryAction.java | 2 +- .../xpack/esql/execution/PlanExecutor.java | 4 +- .../xpack/esql/parser/EsqlParser.java | 11 ++ .../esql/plugin/TransportEsqlQueryAction.java | 2 +- .../xpack/esql/querylog/EsqlLogContext.java | 2 +- .../xpack/esql/session/Configuration.java | 9 +- .../xpack/esql/session/EsqlSession.java | 12 +- .../esql/action/EsqlQueryRequestTests.java | 17 --- .../parser/AbstractStatementParserTests.java | 1 - .../esql/view/InMemoryViewServiceTests.java | 10 +- 19 files changed, 230 insertions(+), 112 deletions(-) create mode 100644 x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PreparedEsqlQueryRequest.java diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/esql/action/EsqlQueryRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/esql/action/EsqlQueryRequest.java index 95f9d79354268..ef26540e43321 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/esql/action/EsqlQueryRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/esql/action/EsqlQueryRequest.java @@ -9,6 +9,7 @@ import org.elasticsearch.action.LegacyActionRequest; import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.core.Nullable; import org.elasticsearch.index.query.QueryBuilder; import java.io.IOException; @@ -21,6 +22,7 @@ protected EsqlQueryRequest(StreamInput in) throws IOException { super(in); } + @Nullable public abstract String query(); public abstract QueryBuilder filter(); diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java index 335f7fa18b6cd..a8df1b36bf692 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java @@ -29,6 +29,9 @@ import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.junit.annotations.TestLogging; import org.elasticsearch.xpack.core.esql.action.ColumnInfo; +import org.elasticsearch.xpack.esql.expression.function.EsqlFunctionRegistry; +import org.elasticsearch.xpack.esql.parser.EsqlConfig; +import org.elasticsearch.xpack.esql.parser.EsqlParser; import org.elasticsearch.xpack.esql.plugin.EsqlPlugin; import org.elasticsearch.xpack.esql.plugin.QueryPragmas; import org.elasticsearch.xpack.esql.plugin.TransportEsqlQueryAction; @@ -176,12 +179,25 @@ protected QueryPragmas getPragmas() { public EsqlQueryResponse run(EsqlQueryRequest request) { try { - return client().execute(EsqlQueryAction.INSTANCE, request).actionGet(30, TimeUnit.SECONDS); + return client().execute(EsqlQueryAction.INSTANCE, maybeWrapAsPrepared(request)).actionGet(30, TimeUnit.SECONDS); } catch (ElasticsearchTimeoutException e) { throw new AssertionError("timeout", e); } } + /** + * Randomly wraps the given request in a {@link PreparedEsqlQueryRequest} to exercise the + * pre-built-plan code path. + */ + private EsqlQueryRequest maybeWrapAsPrepared(EsqlQueryRequest request) { + if (request instanceof PreparedEsqlQueryRequest || randomBoolean()) { + return request; + } + var parser = new EsqlParser(new EsqlConfig(new EsqlFunctionRegistry())); + var statement = parser.createStatement(request.query(), request.params()); + return PreparedEsqlQueryRequest.from(request, statement); + } + protected static QueryPragmas randomPragmas() { Settings.Builder settings = Settings.builder(); if (canUseQueryPragmas()) { diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionIT.java index 9fe12efc028f3..80ca59085f9b7 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionIT.java @@ -57,13 +57,8 @@ import org.elasticsearch.xpack.core.esql.action.ColumnInfo; import org.elasticsearch.xpack.esql.VerificationException; import org.elasticsearch.xpack.esql.analysis.AnalyzerSettings; -import org.elasticsearch.xpack.esql.core.expression.Alias; -import org.elasticsearch.xpack.esql.core.expression.Literal; -import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; import org.elasticsearch.xpack.esql.parser.ParsingException; -import org.elasticsearch.xpack.esql.plan.EsqlStatement; -import org.elasticsearch.xpack.esql.plan.logical.Row; import org.elasticsearch.xpack.esql.planner.PlannerSettings; import org.elasticsearch.xpack.esql.plugin.QueryPragmas; import org.junit.Before; @@ -155,22 +150,6 @@ public void testRow() { } } - /** - * Verifies that a pre-built {@link EsqlStatement} supplied via - * {@link EsqlQueryRequest#syncEsqlQueryRequestWithPlan} is executed - * without going through ES|QL string parsing. - */ - public void testRowWithParsedStatement() { - var plan = new Row(Source.EMPTY, List.of(new Alias(Source.EMPTY, "x", new Literal(Source.EMPTY, 1, DataType.INTEGER)))); - var statement = new EsqlStatement(plan, List.of()); - EsqlQueryRequest request = EsqlQueryRequest.syncEsqlQueryRequestWithPlan(statement); - request.pragmas(getPragmas()); - try (EsqlQueryResponse response = run(request)) { - assertThat(response.columns(), equalTo(List.of(new ColumnInfoImpl("x", "integer", null)))); - assertEquals(List.of(List.of(1)), getValuesList(response)); - } - } - public void testRowWithFilter() { long value = randomLongBetween(0, Long.MAX_VALUE); try (EsqlQueryResponse response = run(syncEsqlQueryRequest("ROW " + value).filter(randomQueryFilter()))) { diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlAsyncActionIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlAsyncActionIT.java index facb4aaef7ec3..2782ecc9d209b 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlAsyncActionIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlAsyncActionIT.java @@ -51,8 +51,8 @@ protected Collection> nodePlugins() { @Override public EsqlQueryResponse run(EsqlQueryRequest original) { EsqlQueryRequest request; - if (original.parsedStatement() != null) { - request = EsqlQueryRequest.asyncEsqlQueryRequestWithPlan(original.parsedStatement()); + if (original instanceof PreparedEsqlQueryRequest prepared) { + request = PreparedEsqlQueryRequest.async(prepared.statement()); } else { request = EsqlQueryRequest.asyncEsqlQueryRequest(original.query()); } diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlDisruptionIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlDisruptionIT.java index f03b475d09781..5d19e5e3cf3cb 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlDisruptionIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlDisruptionIT.java @@ -70,7 +70,7 @@ protected Collection> nodePlugins() { @Override public EsqlQueryResponse run(EsqlQueryRequest request) { // IndexResolver currently ignores failures from field-caps responses and can resolve to a smaller set of concrete indices. - boolean singleIndex = request.query().startsWith("from test |"); + boolean singleIndex = request.queryDescription().startsWith("from test |"); if (singleIndex && randomIntBetween(0, 100) <= 20) { return runQueryWithDisruption(request); } else { @@ -82,7 +82,7 @@ private EsqlQueryResponse runQueryWithDisruption(EsqlQueryRequest request) { final ServiceDisruptionScheme disruptionScheme = addRandomDisruptionScheme(); logger.info("--> start disruption scheme [{}]", disruptionScheme); disruptionScheme.startDisrupting(); - logger.info("--> executing esql query with disruption {} ", request.query()); + logger.info("--> executing esql query with disruption {} ", request.queryDescription()); if (randomBoolean()) { request.allowPartialResults(randomBoolean()); } diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlQueryLoggingIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlQueryLoggingIT.java index cdc854319a917..df4d04c18de53 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlQueryLoggingIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlQueryLoggingIT.java @@ -28,6 +28,7 @@ import org.junit.BeforeClass; import static org.elasticsearch.common.logging.activity.QueryLogging.QUERY_FIELD_INDICES; +import static org.elasticsearch.common.logging.activity.QueryLogging.QUERY_FIELD_QUERY; import static org.elasticsearch.common.logging.activity.QueryLogging.QUERY_FIELD_RESULT_COUNT; import static org.elasticsearch.common.logging.activity.QueryLogging.QUERY_FIELD_SHARDS; import static org.elasticsearch.test.ActivityLoggingUtils.assertMessageFailure; @@ -35,6 +36,7 @@ import static org.elasticsearch.test.ActivityLoggingUtils.getMessageData; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.xpack.esql.action.EsqlQueryRequest.syncEsqlQueryRequest; +import static org.elasticsearch.xpack.esql.action.PreparedEsqlQueryRequest.PREPARED_QUERY_PREFIX; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.junit.Assert.assertTrue; @@ -88,7 +90,11 @@ public void testLogging() throws Exception { private void assertQuery(String query, long hits) { try (var resp = run(query)) { var message = getMessageData(appender.getLastEventAndReset()); - assertMessageSuccess(message, EsqlLogContext.TYPE, query); + // When the request was randomly promoted to a PreparedEsqlQueryRequest the logged + // query will be the plan representation rather than the original query string. + var loggedQuery = message.get(QUERY_FIELD_QUERY); + var queryForAssert = loggedQuery != null && loggedQuery.startsWith(PREPARED_QUERY_PREFIX) ? PREPARED_QUERY_PREFIX : query; + assertMessageSuccess(message, EsqlLogContext.TYPE, queryForAssert); assertThat(Integer.valueOf(message.get(QUERY_FIELD_SHARDS + "successful")), greaterThanOrEqualTo(1)); assertThat(Integer.valueOf(message.getOrDefault(QUERY_FIELD_SHARDS + "skipped", "0")), greaterThanOrEqualTo(0)); assertThat(message.getOrDefault(QUERY_FIELD_SHARDS + "failed", "0"), equalTo("0")); @@ -108,7 +114,9 @@ private void assertQuery(String query, long hits) { private void assertFailedQuery(String query, String expectedMessage, Class expectedException) { expectThrows(VerificationException.class, () -> run(query)); var message = getMessageData(appender.getLastEventAndReset()); - assertMessageFailure(message, EsqlLogContext.TYPE, query, expectedException, expectedMessage); + var loggedQuery = message.get(QUERY_FIELD_QUERY); + var queryForAssert = loggedQuery != null && loggedQuery.startsWith(PREPARED_QUERY_PREFIX) ? PREPARED_QUERY_PREFIX : query; + assertMessageFailure(message, EsqlLogContext.TYPE, queryForAssert, expectedException, expectedMessage); } private int setupIndex(String name, String prefix) { @@ -151,7 +159,11 @@ public void testLoggingPartialShardFailure() throws Exception { var event = appender.getLastEventAndReset(); assertNotNull(event); var message = getMessageData(event); - assertMessageSuccess(message, EsqlLogContext.TYPE, "FROM esql_partial_test | LIMIT 100"); + var loggedQuery = message.get(QUERY_FIELD_QUERY); + var queryForAssert = loggedQuery != null && loggedQuery.startsWith(PREPARED_QUERY_PREFIX) + ? PREPARED_QUERY_PREFIX + : "FROM esql_partial_test | LIMIT 100"; + assertMessageSuccess(message, EsqlLogContext.TYPE, queryForAssert); assertThat(Integer.valueOf(message.get(QUERY_FIELD_SHARDS + "successful")), greaterThanOrEqualTo(1)); assertThat(Integer.valueOf(message.getOrDefault(QUERY_FIELD_SHARDS + "skipped", "0")), equalTo(0)); assertThat(Integer.valueOf(message.get(QUERY_FIELD_SHARDS + "failed")), greaterThanOrEqualTo(1)); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java index 34d083de78858..750970368d582 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java @@ -15,6 +15,7 @@ import org.elasticsearch.common.breaker.NoopCircuitBreaker; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.core.Nullable; import org.elasticsearch.core.Releasables; import org.elasticsearch.core.TimeValue; import org.elasticsearch.index.query.QueryBuilder; @@ -24,8 +25,12 @@ import org.elasticsearch.xpack.core.async.AsyncExecutionId; import org.elasticsearch.xpack.esql.Column; import org.elasticsearch.xpack.esql.approximation.ApproximationSettings; +import org.elasticsearch.xpack.esql.inference.InferenceSettings; +import org.elasticsearch.xpack.esql.parser.EsqlParser; import org.elasticsearch.xpack.esql.parser.QueryParams; import org.elasticsearch.xpack.esql.plan.EsqlStatement; +import org.elasticsearch.xpack.esql.plan.QuerySettings; +import org.elasticsearch.xpack.esql.plan.SettingsValidationContext; import org.elasticsearch.xpack.esql.plugin.EsqlQueryStatus; import org.elasticsearch.xpack.esql.plugin.QueryPragmas; @@ -69,14 +74,6 @@ public class EsqlQueryRequest extends org.elasticsearch.xpack.core.esql.action.E */ private final Map> tables = new TreeMap<>(); - /** - * An optional pre-built statement that bypasses ES|QL string parsing. - * This is transient and never serialized over the wire. It's used by internal callers - * (such as the Prometheus REST endpoints) that construct a {@link EsqlStatement} directly - * instead of going through ES|QL string construction and parsing. - */ - private EsqlStatement parsedStatement; - public static EsqlQueryRequest syncEsqlQueryRequest(String query) { return new EsqlQueryRequest(false, query); } @@ -85,31 +82,38 @@ public static EsqlQueryRequest asyncEsqlQueryRequest(String query) { return new EsqlQueryRequest(true, query); } - /** - * Creates a synchronous request with a pre-built statement, bypassing ES|QL string parsing. - * The query string is only used for logging/display since the plan is already built. - */ - public static EsqlQueryRequest syncEsqlQueryRequestWithPlan(EsqlStatement statement) { - String queryText = statement.plan().sourceText(); - EsqlQueryRequest request = new EsqlQueryRequest(false, queryText.isEmpty() ? "[pre-built plan]" : queryText); - request.parsedStatement = statement; - return request; + EsqlQueryRequest(boolean async, String query) { + this.async = async; + this.query = query; } /** - * Creates an asynchronous request with a pre-built statement, bypassing ES|QL string parsing. - * The query string is only used for logging/display since the plan is already built. + * Copy constructor. Copies all fields from {@code source}. Subclasses that need to override + * specific fields (e.g. {@link PreparedEsqlQueryRequest} overrides {@code query}) should do + * so after calling this constructor. If a new field is added to this class, it must also be + * added here. */ - public static EsqlQueryRequest asyncEsqlQueryRequestWithPlan(EsqlStatement statement) { - String queryText = statement.plan().sourceText(); - EsqlQueryRequest request = new EsqlQueryRequest(true, queryText.isEmpty() ? "[pre-built plan]" : queryText); - request.parsedStatement = statement; - return request; - } - - private EsqlQueryRequest(boolean async, String query) { - this.async = async; - this.query = query; + EsqlQueryRequest(EsqlQueryRequest source) { + this.async = source.async; + this.query = source.query; + this.columnar = source.columnar; + this.profile = source.profile; + this.includeCCSMetadata = source.includeCCSMetadata; + this.includeExecutionMetadata = source.includeExecutionMetadata; + this.timeZone = source.timeZone; + this.locale = source.locale; + this.filter = source.filter; + this.pragmas = source.pragmas; + this.params = source.params; + this.waitForCompletionTimeout = source.waitForCompletionTimeout; + this.keepAlive = source.keepAlive; + this.keepOnCompletion = source.keepOnCompletion; + this.onSnapshotBuild = source.onSnapshotBuild; + this.acceptedPragmaRisks = source.acceptedPragmaRisks; + this.allowPartialResults = source.allowPartialResults; + this.projectRouting = source.projectRouting; + this.approximation = source.approximation; + this.tables.putAll(source.tables); } public EsqlQueryRequest() {} @@ -120,11 +124,7 @@ public EsqlQueryRequest(StreamInput in) throws IOException { @Override public ActionRequestValidationException validate() { - ActionRequestValidationException validationException = null; - if (Strings.hasText(query) == false) { - validationException = addValidationError("[" + RequestXContent.QUERY_FIELD + "] is required", validationException); - } - + ActionRequestValidationException validationException = validateQuery(); if (onSnapshotBuild == false) { if (pragmas.isEmpty() == false && acceptedPragmaRisks == false) { validationException = addValidationError( @@ -142,21 +142,41 @@ public ActionRequestValidationException validate() { return validationException; } + protected ActionRequestValidationException validateQuery() { + if (Strings.hasText(query) == false) { + return addValidationError("[" + RequestXContent.QUERY_FIELD + "] is required", null); + } + return null; + } + public EsqlQueryRequest query(String query) { this.query = query; return this; } @Override + @Nullable public String query() { return query; } /** - * Returns the pre-built statement, or {@code null} if the query string should be parsed. + * Returns a non-null human-readable description of the query for logging, task descriptions, and error messages. + * For regular requests this is the same as {@link #query()}. Overridden by {@link PreparedEsqlQueryRequest} + * to return a display string when there is no query text. + */ + public String queryDescription() { + return query(); + } + + /** + * Parses the query string into an {@link EsqlStatement} and validates its settings. + * Overridden by {@link PreparedEsqlQueryRequest} to return a pre-built statement directly. */ - public EsqlStatement parsedStatement() { - return parsedStatement; + public EsqlStatement parse(EsqlParser parser, SettingsValidationContext settingsValidationCtx, InferenceSettings inferenceSettings) { + EsqlStatement statement = parser.parse(query(), params(), inferenceSettings); + QuerySettings.validate(statement, settingsValidationCtx); + return statement; } public boolean async() { @@ -318,7 +338,7 @@ public EsqlQueryRequest allowPartialResults(boolean allowPartialResults) { @Override public Task createTask(TaskId taskId, String type, String action, TaskId parentTaskId, Map headers) { var status = new EsqlQueryStatus(new AsyncExecutionId(UUIDs.randomBase64UUID(), taskId), keepAlive); - return new EsqlQueryRequestTask(query, taskId.getId(), type, action, parentTaskId, headers, status); + return new EsqlQueryRequestTask(queryDescription(), taskId.getId(), type, action, parentTaskId, headers, status); } private static class EsqlQueryRequestTask extends CancellableTask { diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlResponseListener.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlResponseListener.java index 14c1374da8241..932b640bb41c6 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlResponseListener.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlResponseListener.java @@ -103,7 +103,7 @@ public TimeValue stop() { * To correctly time the execution of a request, a {@link EsqlResponseListener} must be constructed immediately before execution begins. */ public EsqlResponseListener(RestChannel channel, RestRequest restRequest, EsqlQueryRequest esqlRequest) { - this(channel, restRequest, esqlRequest.query(), EsqlMediaTypeParser.getResponseMediaType(restRequest, esqlRequest)); + this(channel, restRequest, esqlRequest.queryDescription(), EsqlMediaTypeParser.getResponseMediaType(restRequest, esqlRequest)); } /** diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PreparedEsqlQueryRequest.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PreparedEsqlQueryRequest.java new file mode 100644 index 0000000000000..33fc2c5b43e95 --- /dev/null +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PreparedEsqlQueryRequest.java @@ -0,0 +1,107 @@ +/* + * 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.action; + +import org.elasticsearch.action.ActionRequestValidationException; +import org.elasticsearch.action.support.TransportAction; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.core.Nullable; +import org.elasticsearch.xpack.esql.inference.InferenceSettings; +import org.elasticsearch.xpack.esql.parser.EsqlParser; +import org.elasticsearch.xpack.esql.plan.EsqlStatement; +import org.elasticsearch.xpack.esql.plan.QuerySettings; +import org.elasticsearch.xpack.esql.plan.SettingsValidationContext; + +import java.io.IOException; + +/** + * An {@link EsqlQueryRequest} backed by a pre-built {@link EsqlStatement}, bypassing ES|QL string + * parsing. Used by internal callers (such as the Prometheus REST endpoints) that construct an + * {@link EsqlStatement} directly instead of going through ES|QL string construction and parsing. + * + *

The query string carried by this request is only used for logging and display; it plays no + * role in execution. + */ +public final class PreparedEsqlQueryRequest extends EsqlQueryRequest { + + public static final String PREPARED_QUERY_PREFIX = "[pre-built plan] "; + + private final EsqlStatement statement; + private final String queryDescription; + + private PreparedEsqlQueryRequest(boolean async, EsqlStatement statement) { + super(async, null); + this.statement = statement; + this.queryDescription = PREPARED_QUERY_PREFIX + statement.plan(); + } + + private PreparedEsqlQueryRequest(EsqlQueryRequest source, EsqlStatement statement) { + super(source); + query(null); + this.statement = statement; + this.queryDescription = PREPARED_QUERY_PREFIX + statement.plan(); + } + + /** + * Creates a synchronous request backed by the given pre-built statement. + */ + public static PreparedEsqlQueryRequest sync(EsqlStatement statement) { + return new PreparedEsqlQueryRequest(false, statement); + } + + /** + * Creates an asynchronous request backed by the given pre-built statement. + */ + public static PreparedEsqlQueryRequest async(EsqlStatement statement) { + return new PreparedEsqlQueryRequest(true, statement); + } + + /** + * Creates a request backed by {@code statement} with all other properties copied from {@code source}. + */ + public static PreparedEsqlQueryRequest from(EsqlQueryRequest source, EsqlStatement statement) { + return new PreparedEsqlQueryRequest(source, statement); + } + + /** + * Returns the pre-built statement carried by this request. + */ + public EsqlStatement statement() { + return statement; + } + + @Override + @Nullable + public String query() { + return null; + } + + @Override + public String queryDescription() { + return queryDescription; + } + + @Override + protected ActionRequestValidationException validateQuery() { + return null; // no query text for prepared requests — skip the Strings.hasText(query) check + } + + /** + * Returns the pre-built statement directly, without invoking the parser, after validating its settings. + */ + @Override + public EsqlStatement parse(EsqlParser parser, SettingsValidationContext settingsValidationCtx, InferenceSettings inferenceSettings) { + QuerySettings.validate(statement, settingsValidationCtx); + return statement; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + TransportAction.localOnly(); + } +} diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RestEsqlQueryAction.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RestEsqlQueryAction.java index c0dcf0abd1c52..7a5157577c612 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RestEsqlQueryAction.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RestEsqlQueryAction.java @@ -61,7 +61,7 @@ protected static RestChannelConsumer restChannelConsumer(EsqlQueryRequest esqlRe if (partialResults != null) { esqlRequest.allowPartialResults(partialResults); } - LOGGER.debug("Beginning execution of ESQL query.\nQuery string: [{}]", esqlRequest.query()); + LOGGER.debug("Beginning execution of ESQL query.\nQuery string: [{}]", esqlRequest.queryDescription()); return channel -> { RestCancellableNodeClient cancellableClient = new RestCancellableNodeClient(client, request.getHttpChannel()); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/execution/PlanExecutor.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/execution/PlanExecutor.java index 16efd24179569..c4394c1a13965 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/execution/PlanExecutor.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/execution/PlanExecutor.java @@ -139,7 +139,7 @@ private void onQuerySuccess( PlanTelemetry planTelemetry ) { planTelemetryManager.publish(planTelemetry, true); - queryLog.onQueryPhase(x, request.query()); + queryLog.onQueryPhase(x, request.queryDescription()); listener.onResponse(x); } @@ -154,7 +154,7 @@ private void onQueryFailure( // TODO when we decide if we will differentiate Kibana from REST, this String value will likely come from the request metrics.failed(clientId); planTelemetryManager.publish(planTelemetry, false); - queryLog.onQueryFailure(request.query(), ex, System.nanoTime() - begin); + queryLog.onQueryFailure(request.queryDescription(), ex, System.nanoTime() - begin); listener.onFailure(ex); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlParser.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlParser.java index e0e79bbe07ebd..1fe224608baac 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlParser.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlParser.java @@ -19,6 +19,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.logging.LogManager; import org.elasticsearch.logging.Logger; +import org.elasticsearch.xpack.esql.action.EsqlQueryRequest; import org.elasticsearch.xpack.esql.core.util.StringUtils; import org.elasticsearch.xpack.esql.inference.InferenceSettings; import org.elasticsearch.xpack.esql.plan.EsqlStatement; @@ -133,6 +134,11 @@ public EsqlStatement createStatement(String query, QueryParams params) { return parsed; } + /** + * Parses the given query into an {@link EsqlStatement}. Note that query settings (accessible via + * {@link EsqlStatement#settings()}) are not validated by this method — the caller is responsible for + * performing any necessary validation before using them. + */ public EsqlStatement parse(String query, QueryParams params, InferenceSettings inferenceSettings) { var parsed = createStatement(query, params, inferenceSettings, null); if (log.isDebugEnabled()) { @@ -145,6 +151,11 @@ public EsqlStatement parse(String query, QueryParams params, InferenceSettings i /** * Parse a view query with the given view name. The view name is used to tag all Source objects * so they can be correctly deserialized when the view positions exceed the outer query's length. + * + *

Unlike the top-level query, which is validated inside {@link EsqlQueryRequest#parse}, view + * bodies are parsed inside a callback that has no request object — so settings validation is + * performed here, where the {@link SettingsValidationContext} is available, rather than at the + * call site. */ public EsqlStatement parseView( String query, diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlQueryAction.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlQueryAction.java index d46cd8783bdf6..973ec7cbb483c 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlQueryAction.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlQueryAction.java @@ -545,7 +545,7 @@ public EsqlQueryTask createTask( id, type, action, - request.query(), // Pass the query as the description + request.queryDescription(), // Pass the query description as the task description parentTaskId, headers, originHeaders, diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/querylog/EsqlLogContext.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/querylog/EsqlLogContext.java index 4082b0ff64157..b971f67e9237b 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/querylog/EsqlLogContext.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/querylog/EsqlLogContext.java @@ -38,7 +38,7 @@ public class EsqlLogContext extends ActivityLoggerContext { } String getQuery() { - return request.query(); + return request.queryDescription(); } public Optional shardInfo() { diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/Configuration.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/Configuration.java index bbdeca5f62df4..ccb2ca90c63c7 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/Configuration.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/Configuration.java @@ -14,6 +14,7 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.compute.data.BlockStreamInput; +import org.elasticsearch.core.Nullable; import org.elasticsearch.xpack.esql.Column; import org.elasticsearch.xpack.esql.approximation.ApproximationSettings; import org.elasticsearch.xpack.esql.core.expression.FoldContext; @@ -88,7 +89,7 @@ public Configuration( QueryPragmas pragmas, int resultTruncationMaxSizeRegular, int resultTruncationDefaultSizeRegular, - String query, + @Nullable String query, boolean profile, Map> tables, long queryStartTimeNanos, @@ -109,7 +110,7 @@ public Configuration( this.resultTruncationDefaultSizeRegular = resultTruncationDefaultSizeRegular; this.resultTruncationMaxSizeTimeseries = resultTruncationMaxSizeTimeseries; this.resultTruncationDefaultSizeTimeseries = resultTruncationDefaultSizeTimeseries; - this.query = query; + this.query = query != null ? query : ""; this.profile = profile; this.tables = tables; assert tables != null; @@ -357,7 +358,7 @@ public Configuration( QueryPragmas pragmas, int resultTruncationMaxSizeRegular, int resultTruncationDefaultSizeRegular, - String query, + @Nullable String query, boolean profile, Map> tables, long queryStartTimeNanos, @@ -379,7 +380,7 @@ public Configuration( this.resultTruncationDefaultSizeRegular = resultTruncationDefaultSizeRegular; this.resultTruncationMaxSizeTimeseries = resultTruncationMaxSizeTimeseries; this.resultTruncationDefaultSizeTimeseries = resultTruncationDefaultSizeTimeseries; - this.query = query; + this.query = query != null ? query : ""; this.profile = profile; this.tables = tables; assert tables != null; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java index 5c8b1ce4e2bfe..50c47002deda4 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java @@ -245,16 +245,10 @@ public void execute( executionInfo.queryProfile().planning().start(); assert ThreadPool.assertCurrentThreadPool(ThreadPool.Names.SEARCH); assert executionInfo != null : "Null EsqlExecutionInfo"; - LOGGER.debug("ESQL query:\n{}", request.query()); + LOGGER.debug("ESQL query:\n{}", request.queryDescription()); TimeSpanMarker parsingProfile = executionInfo.queryProfile().parsing(); parsingProfile.start(); - EsqlStatement statement; - if (request.parsedStatement() != null) { - statement = request.parsedStatement(); - } else { - statement = parse(request); - } - QuerySettings.validate(statement, SettingsValidationContext.from(remoteClusterService)); + EsqlStatement statement = parse(request); gatherSettingsMetrics(statement); parsingProfile.stop(); viewResolver.replaceViews( @@ -716,7 +710,7 @@ private static void releaseLocalRelationBlocks(AtomicReference localRelati } private EsqlStatement parse(EsqlQueryRequest request) { - return parser.parse(request.query(), request.params(), inferenceService.inferenceSettings()); + return request.parse(parser, SettingsValidationContext.from(remoteClusterService), inferenceService.inferenceSettings()); } /** diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java index 30f27ff114dc9..730aa4b5e7c77 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequestTests.java @@ -40,16 +40,12 @@ import org.elasticsearch.xpack.core.async.AsyncTask; import org.elasticsearch.xpack.esql.Column; import org.elasticsearch.xpack.esql.approximation.ApproximationSettings; -import org.elasticsearch.xpack.esql.core.expression.Alias; -import org.elasticsearch.xpack.esql.core.expression.Literal; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; import org.elasticsearch.xpack.esql.parser.ParserUtils; import org.elasticsearch.xpack.esql.parser.ParsingException; import org.elasticsearch.xpack.esql.parser.QueryParam; import org.elasticsearch.xpack.esql.parser.QueryParams; -import org.elasticsearch.xpack.esql.plan.EsqlStatement; -import org.elasticsearch.xpack.esql.plan.logical.Row; import org.elasticsearch.xpack.esql.plugin.EsqlQueryStatus; import java.io.IOException; @@ -81,19 +77,6 @@ public class EsqlQueryRequestTests extends ESTestCase { private final NamedWriteableRegistry namedWriteableRegistry = new NamedWriteableRegistry(List.of(EsqlQueryStatus.ENTRY)); - public void testSyncRequestWithPlanCarriesParsedStatement() { - var plan = new Row(Source.EMPTY, List.of(new Alias(Source.EMPTY, "x", new Literal(Source.EMPTY, 1, DataType.INTEGER)))); - var statement = new EsqlStatement(plan, List.of()); - EsqlQueryRequest request = EsqlQueryRequest.syncEsqlQueryRequestWithPlan(statement); - assertFalse(request.async()); - assertSame(statement, request.parsedStatement()); - } - - public void testSyncRequestFromStringHasNullParsedStatement() { - EsqlQueryRequest request = EsqlQueryRequest.syncEsqlQueryRequest("ROW x = 1"); - assertNull(request.parsedStatement()); - } - public void testParseFields() throws IOException { String query = randomAlphaOfLengthBetween(1, 100); boolean columnar = randomBoolean(); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java index 5586293121dd1..f0dadc73592d2 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java @@ -31,7 +31,6 @@ import java.util.List; import java.util.Map; -import static org.elasticsearch.xpack.esql.EsqlTestUtils.TEST_FUNCTION_REGISTRY; import static org.elasticsearch.xpack.esql.EsqlTestUtils.TEST_PARSER; import static org.elasticsearch.xpack.esql.EsqlTestUtils.equalToIgnoringIds; import static org.elasticsearch.xpack.esql.core.tree.Source.EMPTY; diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewServiceTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewServiceTests.java index a816775f019ee..0443253314788 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewServiceTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewServiceTests.java @@ -56,7 +56,6 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; -import static org.elasticsearch.xpack.esql.EsqlTestUtils.TEST_FUNCTION_REGISTRY; import static org.elasticsearch.xpack.esql.EsqlTestUtils.TEST_PARSER; import static org.elasticsearch.xpack.esql.EsqlTestUtils.as; import static org.elasticsearch.xpack.esql.EsqlTestUtils.equalToIgnoringIds; @@ -989,13 +988,8 @@ private void addView(String name, String query, ViewService viewService) { } private LogicalPlan parse(String query, String viewName) { - return TEST_PARSER.parseView( - query, - queryParams, - new SettingsValidationContext(false, false), - EMPTY_INFERENCE_SETTINGS, - viewName - ).plan(); + return TEST_PARSER.parseView(query, queryParams, new SettingsValidationContext(false, false), EMPTY_INFERENCE_SETTINGS, viewName) + .plan(); } private static Matcher matchesPlan(LogicalPlan plan) { From 43e9e7de540db3a1ad2be334cb02404048909fa4 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Mon, 23 Mar 2026 09:08:27 +0100 Subject: [PATCH 07/16] Fix benchmark compilation errors after PlanTelemetry was removed from parser signatures --- .../benchmark/_nightly/esql/QueryPlanningBenchmark.java | 5 +---- .../benchmark/esql/ViewResolutionBenchmarkBase.java | 7 ++----- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/benchmarks/src/main/java/org/elasticsearch/benchmark/_nightly/esql/QueryPlanningBenchmark.java b/benchmarks/src/main/java/org/elasticsearch/benchmark/_nightly/esql/QueryPlanningBenchmark.java index aeee2cef4c78d..9c53c778f10f8 100644 --- a/benchmarks/src/main/java/org/elasticsearch/benchmark/_nightly/esql/QueryPlanningBenchmark.java +++ b/benchmarks/src/main/java/org/elasticsearch/benchmark/_nightly/esql/QueryPlanningBenchmark.java @@ -38,7 +38,6 @@ import org.elasticsearch.xpack.esql.plugin.QueryPragmas; import org.elasticsearch.xpack.esql.session.Configuration; import org.elasticsearch.xpack.esql.telemetry.Metrics; -import org.elasticsearch.xpack.esql.telemetry.PlanTelemetry; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Fork; @@ -74,7 +73,6 @@ public class QueryPlanningBenchmark { Utils.configureBenchmarkLogging(); } - private PlanTelemetry telemetry; private Analyzer manyFieldsAnalyzer; private LogicalPlanOptimizer defaultOptimizer; private Configuration config; @@ -118,7 +116,6 @@ public void setup() { // Assume all nodes are on the current version for the benchmark. TransportVersion minimumVersion = TransportVersion.current(); - telemetry = new PlanTelemetry(functionRegistry); manyFieldsAnalyzer = new Analyzer( new AnalyzerContext( config, @@ -136,7 +133,7 @@ public void setup() { } private LogicalPlan plan(EsqlParser parser, Analyzer analyzer, LogicalPlanOptimizer optimizer, String query) { - var parsed = parser.parseQuery(query, new QueryParams(), telemetry, new InferenceSettings(Settings.EMPTY)); + var parsed = parser.parseQuery(query, new QueryParams(), new InferenceSettings(Settings.EMPTY)); var analyzed = analyzer.analyze(parsed); var optimized = optimizer.optimize(analyzed); return optimized; diff --git a/benchmarks/src/main/java/org/elasticsearch/benchmark/esql/ViewResolutionBenchmarkBase.java b/benchmarks/src/main/java/org/elasticsearch/benchmark/esql/ViewResolutionBenchmarkBase.java index 376e371fbe4a0..a88a8274feb10 100644 --- a/benchmarks/src/main/java/org/elasticsearch/benchmark/esql/ViewResolutionBenchmarkBase.java +++ b/benchmarks/src/main/java/org/elasticsearch/benchmark/esql/ViewResolutionBenchmarkBase.java @@ -62,7 +62,6 @@ import org.elasticsearch.xpack.esql.plugin.QueryPragmas; import org.elasticsearch.xpack.esql.session.Configuration; import org.elasticsearch.xpack.esql.telemetry.Metrics; -import org.elasticsearch.xpack.esql.telemetry.PlanTelemetry; import org.elasticsearch.xpack.esql.view.ViewResolutionService; import org.elasticsearch.xpack.esql.view.ViewResolver; import org.openjdk.jmh.annotations.Benchmark; @@ -139,7 +138,6 @@ public abstract class ViewResolutionBenchmarkBase { private LogicalPlan preParsedPlan; private String queryString; private BiFunction viewParser; - private PlanTelemetry telemetry; private Analyzer analyzer; private LogicalPlanOptimizer optimizer; private ThreadPool threadPool; @@ -156,9 +154,8 @@ public void setup() { SettingsValidationContext validationCtx = new SettingsValidationContext(false, false); TransportVersion minimumVersion = TransportVersion.current(); - telemetry = new PlanTelemetry(functionRegistry); parser = new EsqlParser(new EsqlConfig(functionRegistry)); - viewParser = (query, viewName) -> parser.parseView(query, new QueryParams(), validationCtx, telemetry, inferenceSettings, viewName) + viewParser = (query, viewName) -> parser.parseView(query, new QueryParams(), validationCtx, inferenceSettings, viewName) .plan(); LinkedHashMap mapping = new LinkedHashMap<>(); @@ -268,7 +265,7 @@ static ViewMetadata buildViewChain(int depth, boolean withFilter) { } private LogicalPlan parsePlan(String query) { - return parser.parseQuery(query, new QueryParams(), telemetry, new InferenceSettings(Settings.EMPTY)); + return parser.parseQuery(query, new QueryParams(), new InferenceSettings(Settings.EMPTY)); } private ViewResolver createResolver(boolean viewsEnabled, ViewMetadata viewMetadata) { From 3abafe79c027c6c1a54db0e3479e442c9d3f220b Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Mon, 23 Mar 2026 08:16:51 +0000 Subject: [PATCH 08/16] [CI] Auto commit changes from spotless --- .../benchmark/esql/ViewResolutionBenchmarkBase.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/benchmarks/src/main/java/org/elasticsearch/benchmark/esql/ViewResolutionBenchmarkBase.java b/benchmarks/src/main/java/org/elasticsearch/benchmark/esql/ViewResolutionBenchmarkBase.java index a88a8274feb10..a2d047b2fd7b3 100644 --- a/benchmarks/src/main/java/org/elasticsearch/benchmark/esql/ViewResolutionBenchmarkBase.java +++ b/benchmarks/src/main/java/org/elasticsearch/benchmark/esql/ViewResolutionBenchmarkBase.java @@ -155,8 +155,7 @@ public void setup() { TransportVersion minimumVersion = TransportVersion.current(); parser = new EsqlParser(new EsqlConfig(functionRegistry)); - viewParser = (query, viewName) -> parser.parseView(query, new QueryParams(), validationCtx, inferenceSettings, viewName) - .plan(); + viewParser = (query, viewName) -> parser.parseView(query, new QueryParams(), validationCtx, inferenceSettings, viewName).plan(); LinkedHashMap mapping = new LinkedHashMap<>(); for (int i = 0; i < 5; i++) { From 647c995fdae840f6d3d5f4302628ad612611b8a0 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Mon, 23 Mar 2026 17:26:47 +0100 Subject: [PATCH 09/16] Address review comments --- .../esql/action/PreparedEsqlQueryRequest.java | 6 ++++- .../xpack/esql/session/EsqlSession.java | 24 +++++++------------ .../xpack/esql/view/ViewService.java | 6 ----- 3 files changed, 13 insertions(+), 23 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PreparedEsqlQueryRequest.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PreparedEsqlQueryRequest.java index 33fc2c5b43e95..8f9704894af11 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PreparedEsqlQueryRequest.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PreparedEsqlQueryRequest.java @@ -42,7 +42,6 @@ private PreparedEsqlQueryRequest(boolean async, EsqlStatement statement) { private PreparedEsqlQueryRequest(EsqlQueryRequest source, EsqlStatement statement) { super(source); - query(null); this.statement = statement; this.queryDescription = PREPARED_QUERY_PREFIX + statement.plan(); } @@ -81,6 +80,11 @@ public String query() { return null; } + @Override + public EsqlQueryRequest query(String query) { + throw new UnsupportedOperationException("PreparedEsqlQueryRequest is backed by a pre-built statement, not a query string"); + } + @Override public String queryDescription() { return queryDescription; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java index 50c47002deda4..a6c81f17e8b29 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java @@ -714,33 +714,25 @@ private EsqlStatement parse(EsqlQueryRequest request) { } /** - * Populates {@code planTelemetry} from a logical plan tree. Called on the view-resolved plan in - * {@link #analyseAndExecute}, so it captures all nodes: the original statement plus any commands - * and functions introduced by view expansion. - *

- * A single {@code forEachDown} pass collects both {@link TelemetryAware} command labels and - * {@link Function} names. Named function calls (e.g. {@code TO_LONG(x)}) produce - * {@link UnresolvedFunction} nodes from the parser; the {@code else} branch covers concrete - * {@link Function} instances that arise from inline cast expressions (e.g. {@code x::long}) or - * programmatically-built plans (e.g. Prometheus plan builders). - *

- * Not all {@link TelemetryAware} nodes have a label (e.g. lookup-table {@link - * org.elasticsearch.xpack.esql.plan.logical.UnresolvedRelation} nodes introduced by - * {@code LOOKUP JOIN} have a {@code null} command name); those are skipped. - * Similarly, not all {@link Function} subclasses are user-callable functions registered in the - * function registry (e.g. binary operators and predicates like {@code Add} or {@code GreaterThan} - * are {@link Function} subclasses but are never in the registry); those are also skipped. + * Populates {@code planTelemetry} from the view-resolved plan, capturing commands and functions + * from the original statement plus any nodes introduced by view expansion. */ static void gatherPlanTelemetry(LogicalPlan plan, PlanTelemetry planTelemetry) { EsqlFunctionRegistry registry = planTelemetry.functionRegistry().snapshotRegistry(); plan.forEachDown(node -> { if (node instanceof TelemetryAware ta && ta.telemetryLabel() != null) { + // Not all TelemetryAware nodes have a label — e.g. lookup index UnresolvedRelation + // nodes introduced by LOOKUP JOIN have a null command name; those are skipped. planTelemetry.command(ta); } node.forEachExpression(Function.class, f -> { if (f instanceof UnresolvedFunction uf) { planTelemetry.function(uf.name()); } else if (registry.functionExists(f.getClass())) { + // Concrete Function instances arise from inline cast expressions (e.g. x::long) + // or programmatically-built plans (e.g. Prometheus plan builders). + // Not all Function subclasses are user-callable (e.g. Add, GreaterThan are + // Function subclasses but are never in the registry); those are skipped. planTelemetry.function(f.getClass()); } }); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/ViewService.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/ViewService.java index b76b770b29f37..9acf056a46a18 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/ViewService.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/ViewService.java @@ -24,12 +24,8 @@ import org.elasticsearch.common.Priority; import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Setting; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.Nullable; -import org.elasticsearch.logging.LogManager; -import org.elasticsearch.logging.Logger; import org.elasticsearch.xpack.core.esql.EsqlFeatureFlags; -import org.elasticsearch.xpack.esql.inference.InferenceSettings; import org.elasticsearch.xpack.esql.parser.EsqlParser; import org.elasticsearch.xpack.esql.parser.QueryParams; @@ -38,8 +34,6 @@ import java.util.Set; public class ViewService { - private static final Logger logger = LogManager.getLogger(ViewService.class); - private static final InferenceSettings EMPTY_INFERENCE_SETTINGS = new InferenceSettings(Settings.EMPTY); private final EsqlParser parser; protected final ClusterService clusterService; From 7178423a8fb728c8664e7e9f1586a341f0c81af7 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Mon, 23 Mar 2026 17:34:35 +0100 Subject: [PATCH 10/16] Require callers to provide a query description rather than materialising statement.plan() --- .../action/AbstractEsqlIntegTestCase.java | 2 +- .../xpack/esql/action/EsqlAsyncActionIT.java | 2 +- .../esql/action/PreparedEsqlQueryRequest.java | 20 +++++++++---------- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java index a8df1b36bf692..9d136a21f0132 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java @@ -195,7 +195,7 @@ private EsqlQueryRequest maybeWrapAsPrepared(EsqlQueryRequest request) { } var parser = new EsqlParser(new EsqlConfig(new EsqlFunctionRegistry())); var statement = parser.createStatement(request.query(), request.params()); - return PreparedEsqlQueryRequest.from(request, statement); + return PreparedEsqlQueryRequest.from(request, statement, "pre-built statement for testing"); } protected static QueryPragmas randomPragmas() { diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlAsyncActionIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlAsyncActionIT.java index 2782ecc9d209b..7f5f1f21bb5d7 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlAsyncActionIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlAsyncActionIT.java @@ -52,7 +52,7 @@ protected Collection> nodePlugins() { public EsqlQueryResponse run(EsqlQueryRequest original) { EsqlQueryRequest request; if (original instanceof PreparedEsqlQueryRequest prepared) { - request = PreparedEsqlQueryRequest.async(prepared.statement()); + request = PreparedEsqlQueryRequest.async(prepared.statement(), "pre-built async statement for testing"); } else { request = EsqlQueryRequest.asyncEsqlQueryRequest(original.query()); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PreparedEsqlQueryRequest.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PreparedEsqlQueryRequest.java index 8f9704894af11..5ea2d68091348 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PreparedEsqlQueryRequest.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PreparedEsqlQueryRequest.java @@ -34,37 +34,37 @@ public final class PreparedEsqlQueryRequest extends EsqlQueryRequest { private final EsqlStatement statement; private final String queryDescription; - private PreparedEsqlQueryRequest(boolean async, EsqlStatement statement) { + private PreparedEsqlQueryRequest(boolean async, EsqlStatement statement, String queryDescription) { super(async, null); this.statement = statement; - this.queryDescription = PREPARED_QUERY_PREFIX + statement.plan(); + this.queryDescription = PREPARED_QUERY_PREFIX + queryDescription; } - private PreparedEsqlQueryRequest(EsqlQueryRequest source, EsqlStatement statement) { + private PreparedEsqlQueryRequest(EsqlQueryRequest source, EsqlStatement statement, String queryDescription) { super(source); this.statement = statement; - this.queryDescription = PREPARED_QUERY_PREFIX + statement.plan(); + this.queryDescription = PREPARED_QUERY_PREFIX + queryDescription; } /** * Creates a synchronous request backed by the given pre-built statement. */ - public static PreparedEsqlQueryRequest sync(EsqlStatement statement) { - return new PreparedEsqlQueryRequest(false, statement); + public static PreparedEsqlQueryRequest sync(EsqlStatement statement, String queryDescription) { + return new PreparedEsqlQueryRequest(false, statement, queryDescription); } /** * Creates an asynchronous request backed by the given pre-built statement. */ - public static PreparedEsqlQueryRequest async(EsqlStatement statement) { - return new PreparedEsqlQueryRequest(true, statement); + public static PreparedEsqlQueryRequest async(EsqlStatement statement, String queryDescription) { + return new PreparedEsqlQueryRequest(true, statement, queryDescription); } /** * Creates a request backed by {@code statement} with all other properties copied from {@code source}. */ - public static PreparedEsqlQueryRequest from(EsqlQueryRequest source, EsqlStatement statement) { - return new PreparedEsqlQueryRequest(source, statement); + public static PreparedEsqlQueryRequest from(EsqlQueryRequest source, EsqlStatement statement, String queryDescription) { + return new PreparedEsqlQueryRequest(source, statement, queryDescription); } /** From 99cd9bbba4af0186e2d6bfae82836b219a0e1e8b Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Mon, 23 Mar 2026 18:02:21 +0100 Subject: [PATCH 11/16] Test Source deserialization with a null-query Configuration Guard against regressions where Sources with real line/column positions are deserialized against a Configuration whose query is null/empty: text must degrade gracefully to empty without throwing. --- .../xpack/esql/core/tree/SourceTests.java | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/core/tree/SourceTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/core/tree/SourceTests.java index bc95f99e36037..773ca4a89564b 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/core/tree/SourceTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/core/tree/SourceTests.java @@ -7,8 +7,12 @@ package org.elasticsearch.xpack.esql.core.tree; import org.elasticsearch.common.io.stream.BytesStreamOutput; +import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput; +import org.elasticsearch.xpack.esql.io.stream.PlanStreamOutput; +import org.elasticsearch.xpack.esql.session.Configuration; import java.io.IOException; import java.util.Arrays; @@ -16,6 +20,7 @@ import java.util.function.Function; import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode; +import static org.elasticsearch.xpack.esql.ConfigurationTestUtils.randomConfiguration; import static org.hamcrest.Matchers.equalTo; public class SourceTests extends ESTestCase { @@ -47,6 +52,46 @@ public void testEqualsAndHashCode() { ); } + /** + * When a {@link Configuration} carries a {@code null} query (e.g. for pre-built plans that + * bypass query-string parsing), the stored query is an empty string. If a plan built from a + * real query string is later executed via such a configuration, Source objects deserialized on + * data nodes must not throw — line/column are preserved, and the source text snippet degrades + * gracefully to empty. + */ + public void testReadFromWithNullQueryYieldsEmptyText() throws IOException { + Source original = new Source(1, 1, "FROM index"); + Configuration writeConfig = randomConfiguration("FROM index | LIMIT 10"); + Configuration readConfig = randomConfiguration(null); // null → "" + try (BytesStreamOutput out = new BytesStreamOutput(); PlanStreamOutput planOut = new PlanStreamOutput(out, writeConfig)) { + original.writeTo(planOut); + try (PlanStreamInput in = new PlanStreamInput(out.bytes().streamInput(), new NamedWriteableRegistry(List.of()), readConfig)) { + Source deserialized = Source.readFrom(in); + assertThat(deserialized.source(), equalTo(original.source())); + assertThat(deserialized.text(), equalTo("")); + } + } + } + + /** + * Same as above but with a Source from line 2 of a multi-line query. Without the + * {@code query.isEmpty()} early-return in {@code sourceText()}, the line-range check in + * {@code textOffset()} would throw for an empty query — this test guards against that regression. + */ + public void testReadFromWithNullQueryMultilineSourceYieldsEmptyText() throws IOException { + Source original = new Source(2, 3, "LIMIT"); + Configuration writeConfig = randomConfiguration("FROM index\n| LIMIT 20"); + Configuration readConfig = randomConfiguration(null); + try (BytesStreamOutput out = new BytesStreamOutput(); PlanStreamOutput planOut = new PlanStreamOutput(out, writeConfig)) { + original.writeTo(planOut); + try (PlanStreamInput in = new PlanStreamInput(out.bytes().streamInput(), new NamedWriteableRegistry(List.of()), readConfig)) { + Source deserialized = Source.readFrom(in); + assertThat(deserialized.source(), equalTo(original.source())); + assertThat(deserialized.text(), equalTo("")); + } + } + } + public void testReadEmpty() throws IOException { try (BytesStreamOutput out = new BytesStreamOutput()) { randomSource().writeTo(out); From 0c5cfebe708d5b85f8fe3d21148662cc41e06e4a Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Mon, 23 Mar 2026 18:25:07 +0100 Subject: [PATCH 12/16] Fix telemetry collection for settings (fixes TelemetryIT) --- .../elasticsearch/xpack/esql/session/EsqlSession.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java index a6c81f17e8b29..a1ac618947275 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java @@ -275,7 +275,7 @@ private void analyseAndExecute( ActionListener> listener ) { assert ThreadPool.assertCurrentThreadPool(ThreadPool.Names.SEARCH); - gatherPlanTelemetry(viewResolution.plan(), planTelemetry); + gatherPlanTelemetry(viewResolution.plan(), statement.settings()); PlanTimeProfile planTimeProfile = request.profile() ? new PlanTimeProfile() : null; ZoneId timeZone = request.timeZone() == null @@ -714,10 +714,10 @@ private EsqlStatement parse(EsqlQueryRequest request) { } /** - * Populates {@code planTelemetry} from the view-resolved plan, capturing commands and functions - * from the original statement plus any nodes introduced by view expansion. + * Populates {@code planTelemetry} from the view-resolved plan, capturing commands, functions, + * and settings from the original statement plus any nodes introduced by view expansion. */ - static void gatherPlanTelemetry(LogicalPlan plan, PlanTelemetry planTelemetry) { + private void gatherPlanTelemetry(LogicalPlan plan, List settings) { EsqlFunctionRegistry registry = planTelemetry.functionRegistry().snapshotRegistry(); plan.forEachDown(node -> { if (node instanceof TelemetryAware ta && ta.telemetryLabel() != null) { @@ -737,6 +737,9 @@ static void gatherPlanTelemetry(LogicalPlan plan, PlanTelemetry planTelemetry) { } }); }); + if (settings != null) { + settings.forEach(s -> planTelemetry.setting(s.name())); + } } private void gatherSettingsMetrics(EsqlStatement statement) { From e2d5bc9c277eaf2e848eaabb3afb9070236fe691 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 24 Mar 2026 07:32:59 +0100 Subject: [PATCH 13/16] Fix maybeWrapAsPrepared to use real cluster settings when pre-parsing When wrapping a request as a PreparedEsqlQueryRequest, the parser was called with Settings.EMPTY, baking in default InferenceSettings (e.g. rerank/completion row limits and enabled flags) into the pre-built plan. Since PreparedEsqlQueryRequest.parse() returns the plan directly without re-parsing, any cluster-settings-dependent parse behaviour (row limits, enabled/disabled checks) was silently bypassed on the prepared path. Fix by reading the current effective cluster settings from the cluster state metadata so the pre-built plan reflects the actual configured values. --- .../xpack/esql/action/AbstractEsqlIntegTestCase.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java index 9d136a21f0132..354bcb064dca6 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java @@ -30,6 +30,7 @@ import org.elasticsearch.test.junit.annotations.TestLogging; import org.elasticsearch.xpack.core.esql.action.ColumnInfo; import org.elasticsearch.xpack.esql.expression.function.EsqlFunctionRegistry; +import org.elasticsearch.xpack.esql.inference.InferenceSettings; import org.elasticsearch.xpack.esql.parser.EsqlConfig; import org.elasticsearch.xpack.esql.parser.EsqlParser; import org.elasticsearch.xpack.esql.plugin.EsqlPlugin; @@ -194,7 +195,8 @@ private EsqlQueryRequest maybeWrapAsPrepared(EsqlQueryRequest request) { return request; } var parser = new EsqlParser(new EsqlConfig(new EsqlFunctionRegistry())); - var statement = parser.createStatement(request.query(), request.params()); + var inferenceSettings = new InferenceSettings(clusterService().state().metadata().settings()); + var statement = parser.parse(request.query(), request.params(), inferenceSettings); return PreparedEsqlQueryRequest.from(request, statement, "pre-built statement for testing"); } From 95a44de7a4aad70a76ad7e8e460b89b49e11b075 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 24 Mar 2026 16:40:17 +0100 Subject: [PATCH 14/16] Fix gatherPlanTelemetry counting STATS for INLINE STATS commands When INLINE STATS is used, the plan tree contains InlineStats wrapping an Aggregate. The generic forEachDown pass in gatherPlanTelemetry visited both nodes and recorded both "INLINE STATS" and "STATS", causing TelemetryIT to fail with an unexpected STATS entry. The old parser-based telemetry (removed in 0c63022d606) only recorded the top-level command node per pipeline step, so the inner Aggregate was never counted. Fix the post-hoc pass to mirror that behaviour: collect inner Aggregate nodes of all InlineStats commands first, then skip them when recording command telemetry. --- .../elasticsearch/xpack/esql/session/EsqlSession.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java index a1ac618947275..e296c60e96f77 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java @@ -84,6 +84,7 @@ import org.elasticsearch.xpack.esql.plan.QuerySettings; import org.elasticsearch.xpack.esql.plan.SettingsValidationContext; import org.elasticsearch.xpack.esql.plan.logical.Explain; +import org.elasticsearch.xpack.esql.plan.logical.InlineStats; import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan; import org.elasticsearch.xpack.esql.plan.logical.join.InlineJoin; import org.elasticsearch.xpack.esql.plan.logical.local.LocalRelation; @@ -719,10 +720,16 @@ private EsqlStatement parse(EsqlQueryRequest request) { */ private void gatherPlanTelemetry(LogicalPlan plan, List settings) { EsqlFunctionRegistry registry = planTelemetry.functionRegistry().snapshotRegistry(); + // Collect Aggregate nodes that are the inner aggregate of an INLINE STATS command; those + // should not be counted as standalone STATS commands. + Set inlineStatsAggregates = new HashSet<>(); + plan.forEachDown(InlineStats.class, inlineStats -> inlineStatsAggregates.add(inlineStats.aggregate())); plan.forEachDown(node -> { - if (node instanceof TelemetryAware ta && ta.telemetryLabel() != null) { + if (node instanceof TelemetryAware ta && ta.telemetryLabel() != null && inlineStatsAggregates.contains(node) == false) { // Not all TelemetryAware nodes have a label — e.g. lookup index UnresolvedRelation // nodes introduced by LOOKUP JOIN have a null command name; those are skipped. + // Aggregate nodes that are the inner aggregate of INLINE STATS are also skipped — + // the INLINE STATS node itself is counted instead. planTelemetry.command(ta); } node.forEachExpression(Function.class, f -> { From 8bf9e818c221f367a2e2d1dd461ec0e0b16a33cb Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 24 Mar 2026 17:53:29 +0100 Subject: [PATCH 15/16] Fix testExplainMultiNode flakiness when maybeWrapAsPrepared is used MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When maybeWrapAsPrepared wraps a request as a PreparedEsqlQueryRequest, configuration.query() is null so Source text cannot be reconstructed during plan deserialization on remote nodes. The profile and EXPLAIN requests are wrapped independently, so one plan may carry expression text (e.g. "value > 50") while the other has an empty source — causing the structural comparison to fail spuriously. Fix by normalising source text in determinizePlanString alongside the existing position normalisation: expression text before "@_:_" in "source" fields is stripped, so the comparison is immune to whether either plan was built from a prepared request. --- .../java/org/elasticsearch/xpack/esql/action/EsqlActionIT.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionIT.java index 40741f86ebd67..52ed907eb12ea 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionIT.java @@ -2214,6 +2214,8 @@ private String determinizePlanString(String plan) { // Normalize source position references like @1:19 or @_:19 .replaceAll("@\\d+:\\d+", "@_:_") .replaceAll("@_:\\d+", "@_:_") + // Normalize source text (may be absent when query is null, e.g. PreparedEsqlQueryRequest) + .replaceAll("(\"source\":\"?)[^@\"]*(@_:_)", "$1$2") // Remove memory addresses .replaceAll("@[0-9a-f]+", "@_") // Remove UUIDs From c1c96d81d9e10d53b8ae929002971330ee116420 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Wed, 25 Mar 2026 08:12:56 +0100 Subject: [PATCH 16/16] Fix TelemetryIT: track SUBQUERY command in telemetry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old parser-based telemetry (telemetryAccounting) was called only on top-level pipeline step nodes, so Subquery nodes — created internally inside visitFromCommand while building UnionAll — were never reached. The test expectation reflected this limitation rather than intent. The new post-hoc gatherPlanTelemetry pass traverses the full plan tree and correctly picks up Subquery nodes, which were already TelemetryAware. Update the test to expect SUBQUERY to appear alongside UNIONALL. --- .../java/org/elasticsearch/xpack/esql/action/TelemetryIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/TelemetryIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/TelemetryIT.java index 071e2d2d51460..5ac3b54c8d98d 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/TelemetryIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/TelemetryIT.java @@ -209,7 +209,7 @@ public static Iterable parameters() { | WHERE id > 10 """, EsqlCapabilities.Cap.SUBQUERY_IN_FROM_COMMAND.isEnabled() - ? Map.of("FROM", 2, "UNIONALL", 1, "WHERE", 2) + ? Map.of("FROM", 2, "UNIONALL", 1, "WHERE", 2, "SUBQUERY", 1) : Collections.emptyMap(), Collections.emptyMap(), EsqlCapabilities.Cap.SUBQUERY_IN_FROM_COMMAND.isEnabled()