Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[ES|QL] Use the new structure of lexer parser from ES #210518

Closed
stratoula opened this issue Feb 11, 2025 · 5 comments · Fixed by #212430
Closed

[ES|QL] Use the new structure of lexer parser from ES #210518

stratoula opened this issue Feb 11, 2025 · 5 comments · Fixed by #212430
Assignees
@drewdaemon
Labels
enhancement New value added to drive a business result Feature:ES|QL ES|QL related features in Kibana impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Team:ESQL ES|QL related features in Kibana

Comments

@stratoula
Copy link
Contributor

Describe the feature:

ES here elastic/elasticsearch#122075 breaks the lexer and parser to smaller chunks for readability. We need to:

  • check that the new structure works exactly the same for kibana
  • change the lexer / parser script to accommodate the new structure
@stratoula stratoula added Feature:ES|QL ES|QL related features in Kibana Team:ESQL ES|QL related features in Kibana labels Feb 11, 2025
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-esql (Team:ESQL)

@stratoula stratoula added enhancement New value added to drive a business result impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. labels Feb 11, 2025
@stratoula
Copy link
Contributor Author

@drewdaemon I did a quick check that everything works ok. #210766

We need to set up the -lib option https://github.com/antlr/antlr4/blob/master/doc/tool-options.md to make antlr take under consideration the new folders

It seems to work as expected, I didnt find any regressions with a very brief testing.

The src/platform/packages/shared/kbn-esql-ast/scripts/fix_generated_antlr.jsis failing though and I am not sure I understand why. Do you want to add this to your list? (without changing our current priorities).

@drewdaemon
Copy link
Contributor

@stratoula sure thing. And yeah I have no idea what that script does. Will look into it.

@drewdaemon drewdaemon self-assigned this Feb 13, 2025
@drewdaemon drewdaemon mentioned this issue Feb 25, 2025
Merged
@drewdaemon
Copy link
Contributor

I like that this change mirrors the refactor in #195418

@stratoula
Copy link
Contributor Author

yeah ++

kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Feb 26, 2025
@drewdaemon
[ES|QL] pull in multi-file grammars (elastic#212430)
98875f3 
## Summary

Resolve elastic#210518

Successful grammar update PR!
elastic#212442

(cherry picked from commit 2435a17)
JoseLuisGJ pushed a commit to JoseLuisGJ/kibana that referenced this issue Feb 27, 2025
@drewdaemon @JoseLuisGJ
[ES|QL] pull in multi-file grammars (elastic#212430)
e2b4100 
## Summary

Resolve elastic#210518

Successful grammar update PR!
elastic#212442
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@drewdaemon drewdaemon

Labels
enhancement New value added to drive a business result Feature:ES|QL ES|QL related features in Kibana impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Team:ESQL ES|QL related features in Kibana
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[ES|QL] pull in multi-file grammars elastic/kibana
3 participants
@drewdaemon @elasticmachine @stratoula