Skip to content

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
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

4 participants