-
Notifications
You must be signed in to change notification settings - Fork 25.4k
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
Conversation
Pinging @elastic/es-analytical-engine (Team:Analytics) |
Hi @ioanatia, I've created a changelog YAML for you. |
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
There was a problem hiding this 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.
There was a problem hiding this 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.
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java
Outdated
Show resolved
Hide resolved
@elasticmachine test this please |
Fork grammar duplicated nested command declaration causing additional lexing to occur resulting in invalid field name declaration Relates to elastic#121948
Fork grammar duplicated nested command declaration causing additional lexing to occur resulting in invalid field name declaration Relates to #121948
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>
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>
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
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
While this adds the main building blocks for FORK execution, follow ups are required (see issue: #121950)