diff --git a/muted-tests.yml b/muted-tests.yml index 1dea20c70318..4987c698354c 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -562,9 +562,6 @@ tests: - class: org.elasticsearch.xpack.rank.rrf.RRFRankClientYamlTestSuiteIT method: test {yaml=rrf/950_pinned_interaction/rrf with pinned retriever as a sub-retriever} issue: https://github.com/elastic/elasticsearch/issues/129845 -- class: org.elasticsearch.xpack.test.rest.XPackRestIT - method: test {p0=esql/60_usage/Basic ESQL usage output (telemetry) non-snapshot version} - issue: https://github.com/elastic/elasticsearch/issues/129888 - class: org.elasticsearch.index.mapper.vectors.DenseVectorFieldMapperTests method: testExistsQueryMinimalMapping issue: https://github.com/elastic/elasticsearch/issues/129911 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 e56d00692955..a8ea1ba95dc1 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 @@ -120,6 +120,13 @@ public class LogicalPlanBuilder extends ExpressionBuilder { protected LogicalPlan plan(ParseTree ctx) { LogicalPlan p = ParserUtils.typedParsing(this, ctx, LogicalPlan.class); + if (p instanceof Explain == false && p.anyMatch(logicalPlan -> logicalPlan instanceof Explain)) { + throw new ParsingException(source(ctx), "EXPLAIN does not support downstream commands"); + } + if (p instanceof Explain explain && explain.query().anyMatch(logicalPlan -> logicalPlan instanceof Explain)) { + // TODO this one is never reached because the Parser fails to understand multiple round brackets + throw new ParsingException(source(ctx), "EXPLAIN cannot be used inside another EXPLAIN command"); + } var errors = this.context.params().parsingErrors(); if (errors.hasNext() == false) { return p; 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 6e246785f0b6..4ff65f59bbd7 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 @@ -190,15 +190,10 @@ public class EsqlSession { assert executionInfo != null : "Null EsqlExecutionInfo"; LOGGER.debug("ESQL query:\n{}", request.query()); LogicalPlan parsed = parse(request.query(), request.params()); - Explain explain = findExplain(parsed); - if (explain != null) { + if (parsed instanceof Explain explain) { explainMode = true; - if (explain == parsed) { - parsed = explain.query(); - parsedPlanString = parsed.toString(); - } else { - throw new VerificationException("EXPLAIN does not support downstream commands"); - } + parsed = explain.query(); + parsedPlanString = parsed.toString(); } analyzedPlan(parsed, executionInfo, request.filter(), new EsqlCCSUtils.CssPartialErrorsActionListener(executionInfo, listener) { @Override @@ -211,19 +206,6 @@ public class EsqlSession { }); } - private Explain findExplain(LogicalPlan parsed) { - if (parsed instanceof Explain e) { - return e; - } - for (LogicalPlan child : parsed.children()) { - Explain result = findExplain(child); - if (result != null) { - return result; - } - } - return null; - } - /** * Execute an analyzed plan. Most code should prefer calling {@link #execute} but * this is public for testing. diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java index b0f83672b5ef..cb5c53193949 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java @@ -3496,6 +3496,16 @@ public class StatementParserTests extends AbstractStatementParserTests { } } + public void testExplainErrors() { + assumeTrue("Requires EXPLAIN capability", EsqlCapabilities.Cap.EXPLAIN.isEnabled()); + // TODO this one is incorrect + expectError("explain ( from test ) | limit 1", "line 1:23: mismatched input '|' expecting {'|', ',', ')', 'metadata'}"); + expectError( + "explain (row x=\"Elastic\" | eval y=concat(x,to_upper(\"search\"))) | mv_expand y", + "line 1:1: EXPLAIN does not support downstream commands" + ); + } + public void testRerankDefaultInferenceIdAndScoreAttribute() { assumeTrue("RERANK requires corresponding capability", EsqlCapabilities.Cap.RERANK.isEnabled()); diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/220_explain.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/220_explain.yml index e6b68c7f682a..be9163bd2a2f 100644 --- a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/220_explain.yml +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/220_explain.yml @@ -97,5 +97,5 @@ explainDownstream: query: 'EXPLAIN (row a = 1) | eval b = 2' catch: "bad_request" - - match: { error.type: "verification_exception" } + - match: { error.type: "parsing_exception" } - contains: { error.reason: "EXPLAIN does not support downstream commands" } diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/60_usage.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/60_usage.yml index 3aca6d2ff500..1276f026ffdf 100644 --- a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/60_usage.yml +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/60_usage.yml @@ -46,7 +46,7 @@ setup: - do: {xpack.usage: {}} - match: { esql.available: true } - match: { esql.enabled: true } - - length: { esql.features: 26 } + - length: { esql.features: 27 } - set: {esql.features.dissect: dissect_counter} - set: {esql.features.drop: drop_counter} - set: {esql.features.eval: eval_counter}