ESQL: Move explain command validation off EsqlSession (#129918)

This commit is contained in:
Andrei Stefan 2025-06-24 18:05:29 +03:00 committed by GitHub
parent ae3c3601fd
commit 9c52106c61
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 22 additions and 26 deletions

View file

@ -562,9 +562,6 @@ tests:
- class: org.elasticsearch.xpack.rank.rrf.RRFRankClientYamlTestSuiteIT - class: org.elasticsearch.xpack.rank.rrf.RRFRankClientYamlTestSuiteIT
method: test {yaml=rrf/950_pinned_interaction/rrf with pinned retriever as a sub-retriever} method: test {yaml=rrf/950_pinned_interaction/rrf with pinned retriever as a sub-retriever}
issue: https://github.com/elastic/elasticsearch/issues/129845 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 - class: org.elasticsearch.index.mapper.vectors.DenseVectorFieldMapperTests
method: testExistsQueryMinimalMapping method: testExistsQueryMinimalMapping
issue: https://github.com/elastic/elasticsearch/issues/129911 issue: https://github.com/elastic/elasticsearch/issues/129911

View file

@ -120,6 +120,13 @@ public class LogicalPlanBuilder extends ExpressionBuilder {
protected LogicalPlan plan(ParseTree ctx) { protected LogicalPlan plan(ParseTree ctx) {
LogicalPlan p = ParserUtils.typedParsing(this, ctx, LogicalPlan.class); 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(); var errors = this.context.params().parsingErrors();
if (errors.hasNext() == false) { if (errors.hasNext() == false) {
return p; return p;

View file

@ -190,15 +190,10 @@ public class EsqlSession {
assert executionInfo != null : "Null EsqlExecutionInfo"; assert executionInfo != null : "Null EsqlExecutionInfo";
LOGGER.debug("ESQL query:\n{}", request.query()); LOGGER.debug("ESQL query:\n{}", request.query());
LogicalPlan parsed = parse(request.query(), request.params()); LogicalPlan parsed = parse(request.query(), request.params());
Explain explain = findExplain(parsed); if (parsed instanceof Explain explain) {
if (explain != null) {
explainMode = true; explainMode = true;
if (explain == parsed) {
parsed = explain.query(); parsed = explain.query();
parsedPlanString = parsed.toString(); parsedPlanString = parsed.toString();
} else {
throw new VerificationException("EXPLAIN does not support downstream commands");
}
} }
analyzedPlan(parsed, executionInfo, request.filter(), new EsqlCCSUtils.CssPartialErrorsActionListener(executionInfo, listener) { analyzedPlan(parsed, executionInfo, request.filter(), new EsqlCCSUtils.CssPartialErrorsActionListener(executionInfo, listener) {
@Override @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 * Execute an analyzed plan. Most code should prefer calling {@link #execute} but
* this is public for testing. * this is public for testing.

View file

@ -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() { public void testRerankDefaultInferenceIdAndScoreAttribute() {
assumeTrue("RERANK requires corresponding capability", EsqlCapabilities.Cap.RERANK.isEnabled()); assumeTrue("RERANK requires corresponding capability", EsqlCapabilities.Cap.RERANK.isEnabled());

View file

@ -97,5 +97,5 @@ explainDownstream:
query: 'EXPLAIN (row a = 1) | eval b = 2' query: 'EXPLAIN (row a = 1) | eval b = 2'
catch: "bad_request" catch: "bad_request"
- match: { error.type: "verification_exception" } - match: { error.type: "parsing_exception" }
- contains: { error.reason: "EXPLAIN does not support downstream commands" } - contains: { error.reason: "EXPLAIN does not support downstream commands" }

View file

@ -46,7 +46,7 @@ setup:
- do: {xpack.usage: {}} - do: {xpack.usage: {}}
- match: { esql.available: true } - match: { esql.available: true }
- match: { esql.enabled: true } - match: { esql.enabled: true }
- length: { esql.features: 26 } - length: { esql.features: 27 }
- set: {esql.features.dissect: dissect_counter} - set: {esql.features.dissect: dissect_counter}
- set: {esql.features.drop: drop_counter} - set: {esql.features.drop: drop_counter}
- set: {esql.features.eval: eval_counter} - set: {esql.features.eval: eval_counter}