ES|QL: Initial grammar and changes for FORK (snapshot)#121948
Merged
ioanatia merged 51 commits intoelastic:mainfrom Feb 20, 2025
Merged
ES|QL: Initial grammar and changes for FORK (snapshot)#121948ioanatia merged 51 commits intoelastic:mainfrom
ioanatia merged 51 commits intoelastic:mainfrom
Conversation
ioanatia
added
>feature
Team:Search Relevance
elasticsearchmachine
added
needs:triage
ioanatia
added
:Analytics/ES|QL
elasticsearchmachine
added
Team:Analytics
Collaborator
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
Collaborator
|
Hi @ioanatia, I've created a changelog YAML for you. |
23 tasks
ChrisHegarty
changed the title
nik9000
reviewed
Feb 19, 2025
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java
Outdated
Show resolved
Hide resolved
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java
Show resolved
Hide resolved
costin
approved these changes
Feb 20, 2025
Member
There was a problem hiding this comment.
My biggest comment is around Fork not being Nary which forces manual invocation of the analyzer (function resolution), verifier, etc.. to be called manually just for fork because the infrastructure doesn't see the nested plans.
As this will be addressed in a follow-up, the PR LGTM.
costin
reviewed
Feb 20, 2025
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java
Outdated
Show resolved
Hide resolved
Contributor
Author
|
@elasticmachine test this please |
costin
added a commit
to costin/elasticsearch
that referenced
this pull request
Feb 21, 2025
Fork grammar duplicated nested command declaration causing additional lexing to occur resulting in invalid field name declaration Relates to elastic#121948
costin
added a commit
that referenced
this pull request
Feb 21, 2025
Fork grammar duplicated nested command declaration causing additional lexing to occur resulting in invalid field name declaration Relates to #121948
costin
added a commit
that referenced
this pull request
Mar 15, 2025
Due to recent grammar changes made ( token to no longer be reported by its text rather by his internal token name. Due to the use of pushMode, the symbol is not treated as a literal rather as a symbol. To address this, the parser listener looks at the error message and changes the message before returning it to the user. Replace hacky regex approach with Vocabulary substitution (not as pluggable as it could be yet much better) Fix #124145 Relates #123085 #121948 Co-authored-by: Alexander Spies <alexander.spies@elastic.co>
omricohenn
pushed a commit
to omricohenn/elasticsearch
that referenced
this pull request
Mar 28, 2025
Due to recent grammar changes made ( token to no longer be reported by its text rather by his internal token name. Due to the use of pushMode, the symbol is not treated as a literal rather as a symbol. To address this, the parser listener looks at the error message and changes the message before returning it to the user. Replace hacky regex approach with Vocabulary substitution (not as pluggable as it could be yet much better) Fix elastic#124145 Relates elastic#123085 elastic#121948 Co-authored-by: Alexander Spies <alexander.spies@elastic.co>
nik9000
added a commit
to nik9000/elasticsearch
that referenced
this pull request
May 1, 2025
Due to recent grammar changes made ( token to no longer be reported by its text rather by his internal token name. Due to the use of pushMode, the symbol is not treated as a literal rather as a symbol. To address this, the parser listener looks at the error message and changes the message before returning it to the user. Fix elastic#124145 Relates elastic#123085 elastic#121948
nik9000
added a commit
that referenced
this pull request
May 1, 2025
Due to recent grammar changes made ( token to no longer be reported by its text rather by his internal token name. Due to the use of pushMode, the symbol is not treated as a literal rather as a symbol. To address this, the parser listener looks at the error message and changes the message before returning it to the user. Fix #124145 Relates #123085 #121948
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While this adds the main building blocks for FORK execution, follow ups are required (see issue: #121950)