Skip to content

ESQL: Catch parsing exception #124958

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 16, 2025
Merged

ESQL: Catch parsing exception #124958

merged 3 commits into from
Mar 16, 2025

Conversation

costin
Copy link
Member

@costin costin commented Mar 16, 2025

When an invalid popMode occurs (due to an invalid query), ANTLR throws
an out-of-channel exception, bypassing the existing checks.
This PR extends the checks and properly reports the error back to the
user

Fix #119025

@costin costin added >bug auto-backport Automatically create backport pull requests when merged :Analytics/ES|QL AKA ESQL ES|QL-ui Impacts ES|QL UI v8.18.1 v8.19.0 v9.0.1 v9.1.0 v8.17.4 labels Mar 16, 2025
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Mar 16, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/kibana-esql (ES|QL-ui)

@elasticsearchmachine
Copy link
Collaborator

Hi @costin, I've created a changelog YAML for you.

costin added 2 commits March 15, 2025 18:37
When an invalid popMode occurs (due to an invalid query), ANTLR throws
 an out-of-channel exception, bypassing the existing checks.
This PR extends the checks and properly reports the error back to the
 user

Fix elastic#119025
Copy link
Member

@fang-xing-esql fang-xing-esql left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@costin costin merged commit dc15462 into elastic:main Mar 16, 2025
17 checks passed
@costin costin deleted the fix/119025 branch March 16, 2025 03:37
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.18 Commit could not be cherrypicked due to conflicts
8.x Commit could not be cherrypicked due to conflicts
9.0 Commit could not be cherrypicked due to conflicts
8.17 Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 124958

costin added a commit to costin/elasticsearch that referenced this pull request Mar 16, 2025
When an invalid popMode occurs (due to an invalid query), ANTLR throws
 an out-of-channel exception, bypassing the existing checks.
This PR extends the checks and properly reports the error back to the
 user

Fix elastic#119025

(cherry picked from commit dc15462)

# Conflicts:
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlParser.java
costin added a commit to costin/elasticsearch that referenced this pull request Mar 16, 2025
When an invalid popMode occurs (due to an invalid query), ANTLR throws
 an out-of-channel exception, bypassing the existing checks.
This PR extends the checks and properly reports the error back to the
 user

Fix elastic#119025

(cherry picked from commit dc15462)

# Conflicts:
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlParser.java
#	x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java
costin added a commit to costin/elasticsearch that referenced this pull request Mar 16, 2025
When an invalid popMode occurs (due to an invalid query), ANTLR throws
 an out-of-channel exception, bypassing the existing checks.
This PR extends the checks and properly reports the error back to the
 user

Fix elastic#119025

(cherry picked from commit dc15462)

# Conflicts:
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlParser.java
#	x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java
@costin
Copy link
Member Author

costin commented Mar 16, 2025

💚 All backports created successfully

Status Branch Result
8.x
9.0
8.18
8.17

Questions ?

Please refer to the Backport tool documentation

costin added a commit to costin/elasticsearch that referenced this pull request Mar 16, 2025
When an invalid popMode occurs (due to an invalid query), ANTLR throws
 an out-of-channel exception, bypassing the existing checks.
This PR extends the checks and properly reports the error back to the
 user

Fix elastic#119025

(cherry picked from commit dc15462)

# Conflicts:
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlParser.java
#	x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java
@costin
Copy link
Member Author

costin commented Mar 16, 2025

💚 All backports created successfully

Status Branch Result
9.0

Questions ?

Please refer to the Backport tool documentation

costin added a commit to costin/elasticsearch that referenced this pull request Mar 16, 2025
When an invalid popMode occurs (due to an invalid query), ANTLR throws
 an out-of-channel exception, bypassing the existing checks.
This PR extends the checks and properly reports the error back to the
 user

Fix elastic#119025

(cherry picked from commit dc15462)

# Conflicts:
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlParser.java
#	x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java
costin added a commit to costin/elasticsearch that referenced this pull request Mar 17, 2025
When an invalid popMode occurs (due to an invalid query), ANTLR throws
 an out-of-channel exception, bypassing the existing checks.
This PR extends the checks and properly reports the error back to the
 user

Fix elastic#119025

(cherry picked from commit dc15462)

# Conflicts:
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlParser.java
#	x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java
@costin
Copy link
Member Author

costin commented Mar 17, 2025

💚 All backports created successfully

Status Branch Result
9.0

Questions ?

Please refer to the Backport tool documentation

costin added a commit to costin/elasticsearch that referenced this pull request Mar 17, 2025
When an invalid popMode occurs (due to an invalid query), ANTLR throws
 an out-of-channel exception, bypassing the existing checks.
This PR extends the checks and properly reports the error back to the
 user

Fix elastic#119025

(cherry picked from commit dc15462)

# Conflicts:
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlParser.java
#	x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java
@costin
Copy link
Member Author

costin commented Mar 17, 2025

💚 All backports created successfully

Status Branch Result
8.18
8.17

Questions ?

Please refer to the Backport tool documentation

costin added a commit to costin/elasticsearch that referenced this pull request Mar 17, 2025
When an invalid popMode occurs (due to an invalid query), ANTLR throws
 an out-of-channel exception, bypassing the existing checks.
This PR extends the checks and properly reports the error back to the
 user

Fix elastic#119025

(cherry picked from commit dc15462)

# Conflicts:
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlParser.java
#	x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java
costin added a commit to costin/elasticsearch that referenced this pull request Mar 17, 2025
When an invalid popMode occurs (due to an invalid query), ANTLR throws
 an out-of-channel exception, bypassing the existing checks.
This PR extends the checks and properly reports the error back to the
 user

Fix elastic#119025

(cherry picked from commit dc15462)

# Conflicts:
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlParser.java
@costin
Copy link
Member Author

costin commented Mar 17, 2025

💚 All backports created successfully

Status Branch Result
8.x

Questions ?

Please refer to the Backport tool documentation

elasticsearchmachine pushed a commit that referenced this pull request Mar 17, 2025
* ESQL: Catch parsing exception (#124958)

When an invalid popMode occurs (due to an invalid query), ANTLR throws
 an out-of-channel exception, bypassing the existing checks.
This PR extends the checks and properly reports the error back to the
 user

Fix #119025

(cherry picked from commit dc15462)

# Conflicts:
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlParser.java

* Update StatementParserTests.java

Fix test for 8.x branch
elasticsearchmachine pushed a commit that referenced this pull request Mar 17, 2025
* ESQL: Catch parsing exception (#124958)

When an invalid popMode occurs (due to an invalid query), ANTLR throws
 an out-of-channel exception, bypassing the existing checks.
This PR extends the checks and properly reports the error back to the
 user

Fix #119025

(cherry picked from commit dc15462)

# Conflicts:
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlParser.java
#	x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java

* Update StatementParserTests.java

Fix test for 8.x branch
elasticsearchmachine pushed a commit that referenced this pull request Mar 17, 2025
* ESQL: Catch parsing exception (#124958)

When an invalid popMode occurs (due to an invalid query), ANTLR throws
 an out-of-channel exception, bypassing the existing checks.
This PR extends the checks and properly reports the error back to the
 user

Fix #119025

(cherry picked from commit dc15462)

# Conflicts:
#	x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlParser.java
#	x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java

* Update StatementParserTests.java

Fix test for 8.x branch
@@ -153,6 +154,9 @@ private <T> T invokeParser(
return result.apply(new AstBuilder(new ExpressionBuilder.ParsingContext(params, metrics)), 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)
} catch (EmptyStackException ese) {
throw new ParsingException("Invalid query [{}]", query);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately exception is very generic and does not bring any details on position,
however I wonder if we could make any assumptions as for what went wrong here?

It might be frustrating to see message like this for not too complex query like:

                stats
                min = min(a),
                max = max(a)) WHERE (a % 3 > 10 OR a / 2 > 100),
                avg = avg(a) WHERE a / 2 > 100
                BY a

note max(a)).

We know this could be caused by extra ) or ]. I wonder if we should attempt to count them in here to highlight this could be an issue?

weizijun added a commit to weizijun/elasticsearch that referenced this pull request Mar 17, 2025
* main: (95 commits)
  Mute org.elasticsearch.datastreams.lifecycle.DataStreamLifecycleServiceIT testLifecycleAppliedToFailureStore elastic#124999
  Merge template mappings properly during validation (elastic#124784)
  [Build] Rework internal build plugin plugin to work with Isolated Projects (elastic#123461)
  [Build] Require reason for usesDefaultDistribution (elastic#124707)
  Mute org.elasticsearch.packaging.test.DockerTests test011SecurityEnabledStatus elastic#124990
  Mute org.elasticsearch.xpack.ilm.TimeSeriesDataStreamsIT testRolloverAction elastic#124987
  Mute org.elasticsearch.packaging.test.BootstrapCheckTests test10Install elastic#124957
  Mute org.elasticsearch.integration.DataStreamLifecycleServiceRuntimeSecurityIT testRolloverLifecycleAndForceMergeAuthorized elastic#124978
  Mute org.elasticsearch.xpack.esql.action.CrossClusterAsyncQueryStopIT testStopQuery elastic#124977
  Mute org.elasticsearch.xpack.esql.action.CrossClusterAsyncQueryStopIT testStopQueryLocal elastic#121672
  Mention zero-window state in networking docs (elastic#124967)
  Remove remoteAddress field from TransportResponse (elastic#120016)
  Include failures in partial response (elastic#124929)
  Prevent work starvation bug if using scaling EsThreadPoolExecutor with core pool size = 0  (elastic#124732)
  Re-enable analysis stemmer test (elastic#124961)
  Mute org.elasticsearch.xpack.esql.action.CrossClusterAsyncQueryStopIT testStopQueryLocalNoRemotes elastic#124959
  ESQL: Catch parsing exception (elastic#124958)
  ESQL: Improve error message for ( and [ (elastic#124177)
  Mute org.elasticsearch.xpack.esql.qa.single_node.EsqlSpecIT test {lookup-join.MvJoinKeyFromRow SYNC} elastic#124951
  Mute org.elasticsearch.datastreams.lifecycle.DataStreamLifecycleServiceIT testErrorRecordingOnRetention elastic#124950
  ...

# Conflicts:
#	server/src/main/java/org/elasticsearch/index/mapper/vectors/DenseVectorFieldMapper.java
#	server/src/test/java/org/elasticsearch/index/mapper/vectors/DenseVectorFieldTypeTests.java
omricohenn pushed a commit to omricohenn/elasticsearch that referenced this pull request Mar 28, 2025
When an invalid popMode occurs (due to an invalid query), ANTLR throws
 an out-of-channel exception, bypassing the existing checks.
This PR extends the checks and properly reports the error back to the
 user

Fix elastic#119025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL auto-backport Automatically create backport pull requests when merged backport pending >bug ES|QL-ui Impacts ES|QL UI Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.17.4 v8.18.1 v8.19.0 v9.0.1 v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Server error while parsing ESQL query
4 participants