Skip to content

ES|QL: Initial grammar and changes for FORK (snapshot) #121948

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 51 commits into from
Feb 20, 2025

Conversation

ioanatia
Copy link
Contributor

@ioanatia ioanatia commented Feb 6, 2025

While this adds the main building blocks for FORK execution, follow ups are required (see issue: #121950)

@ioanatia ioanatia added >feature Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch labels Feb 6, 2025
@elasticsearchmachine elasticsearchmachine added needs:triage Requires assignment of a team area label v9.1.0 and removed Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch labels Feb 6, 2025
@ioanatia ioanatia added :Analytics/ES|QL AKA ESQL Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch and removed needs:triage Requires assignment of a team area label labels Feb 6, 2025
@elasticsearchmachine elasticsearchmachine added Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) and removed Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch labels Feb 6, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

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

@ioanatia ioanatia mentioned this pull request Feb 6, 2025
23 tasks
@ioanatia ioanatia marked this pull request as draft February 6, 2025 20:54
@ChrisHegarty ChrisHegarty changed the title ES|QL: Initial grammar and changes for FORK (behind snapshot) ES|QL: Initial grammar and changes for FORK (snapshot) Feb 19, 2025
Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

LGTM. I'd love @costin or someone else with more language expertise to have a look too.

Copy link
Member

@costin costin left a comment

Choose a reason for hiding this comment

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

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.

@ioanatia ioanatia removed the test-release Trigger CI checks against release build label Feb 20, 2025
@ioanatia
Copy link
Contributor Author

ioanatia commented Feb 20, 2025

@elasticmachine test this please

@ioanatia ioanatia merged commit a26b596 into elastic:main Feb 20, 2025
17 checks passed
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
@ChrisHegarty ChrisHegarty added the ES|QL-ui Impacts ES|QL UI label Mar 4, 2025
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL ES|QL-ui Impacts ES|QL UI >feature Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants