ES|QL: allow executing pre-built parsed statements#144415
ES|QL: allow executing pre-built parsed statements#144415felixbarny merged 22 commits intoelastic:mainfrom
Conversation
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.
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
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.
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java
Outdated
Show resolved
Hide resolved
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java
Outdated
Show resolved
Hide resolved
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.
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.
|
The The flow looks like this:
Creating a dynamic ES|QL string to achieve that doesn't seem feasible and wouldn't allow us to re-use the existing parsing logic, making this very brittle. |
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java
Outdated
Show resolved
Hide resolved
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java
Outdated
Show resolved
Hide resolved
...esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlAsyncActionIT.java
Show resolved
Hide resolved
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryRequest.java
Outdated
Show resolved
Hide resolved
…st-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
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
d220dbb to
d6d25cc
Compare
alex-spies
left a comment
There was a problem hiding this comment.
Thanks @felixbarny , I think this came out rather nicely.
This mostly LGTM, except that I think this is currently breaking the plan telemetry tests. Before merging, it'd be great if @luigidellaquila or @idegtiarenko could have another look at the plan telemetry refactoring, specifically.
Except for that, I have mostly minor comments. Please resolve these at your own discretion. (Maybe don't skip those that aren't marked "nit"/"thought", though.)
.../plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PreparedEsqlQueryRequest.java
Show resolved
Hide resolved
.../plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PreparedEsqlQueryRequest.java
Outdated
Show resolved
Hide resolved
.../plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PreparedEsqlQueryRequest.java
Show resolved
Hide resolved
.../plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PreparedEsqlQueryRequest.java
Show resolved
Hide resolved
.../plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PreparedEsqlQueryRequest.java
Show resolved
Hide resolved
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java
Outdated
Show resolved
Hide resolved
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java
Outdated
Show resolved
Hide resolved
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/ViewService.java
Outdated
Show resolved
Hide resolved
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/LogicalPlanBuilder.java
Show resolved
Hide resolved
.../internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java
Show resolved
Hide resolved
.../plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PreparedEsqlQueryRequest.java
Show resolved
Hide resolved
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java
Show resolved
Hide resolved
…ing statement.plan()
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.
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.
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 0c63022) 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.
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.
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.
…atement' into promql-query-range-parsed-statement
- Pass query description to PreparedEsqlQueryRequest.sync (new required arg added in elastic#144415) - Pass index path param instead of hardcoded "*" to PromqlQueryPlanBuilder - Add INDEX_PARAM constant; replace unused EsqlQueryRequest import with PreparedEsqlQueryRequest
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.
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.
The context of this change is that I'm working on a Prometheus query_range endpoint that internally executes an ES|QL
PROMQLcommand and then translates the response to the expected JSON format.This change allows the query range rest action to provide a
LogicalPlaninstead of having to construct a temporary query string. We're also planning to add more Prometheus endpoints that would benefit from this. For example, the different Prometheus metadata APIs that will rely on theTS_INFOcommand under the hood.Overview of the changes:
EsqlQueryRequest.syncEsqlQueryRequestWithPlan(EsqlStatement)so internal callers can pass a pre-built logical planparsedStatementsupport toEsqlQueryRequestand execute it directly inEsqlSessioninstead of reparsing query textEsqlQueryRequestTests,EsqlActionIT#testRowWithParsedStatement) to verify parsed plans are actually executedStack context