Skip to content

[ES|QL] Stop ANTLR token monkey-patching in parser#225629

Merged
vadimkibana merged 3 commits intoelastic:mainfrom
vadimkibana:fix-do-not-patch-token-stream
Jul 1, 2025
Merged

[ES|QL] Stop ANTLR token monkey-patching in parser#225629
vadimkibana merged 3 commits intoelastic:mainfrom
vadimkibana:fix-do-not-patch-token-stream

Conversation

@vadimkibana
Copy link
Contributor

@vadimkibana vadimkibana commented Jun 27, 2025 •
edited by kibanamachine
Loading

Summary

A small cleanup in parser code in the ES|QL AST package. We want to show users readable syntax error token names like [ and ( (instead of LP), however, we should not monkey-patch ANTLR generated files to achieve this. Instead, this PR moves the message improvement code to the validation & autocomplete package, where the messages are actually used.

Checklist

@vadimkibana
Copy link
Contributor Author

vadimkibana commented Jun 27, 2025

/ci

@vadimkibana vadimkibana marked this pull request as ready for review June 30, 2025 16:32
@vadimkibana vadimkibana requested a review from a team as a code owner June 30, 2025 16:32
@vadimkibana vadimkibana added review release_note:skip Skip the PR/issue when compiling release notes Feature:ES|QL ES|QL related features in Kibana Team:ESQL ES|QL related features in Kibana t// backport:version Backport to applied version labels v9.2.0 labels Jun 30, 2025
@elasticmachine
Copy link
Contributor

elasticmachine commented Jun 30, 2025

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

@vadimkibana vadimkibana changed the title remove antlr token monkey-patching from parser [ES|QL] Stop ANTLR token monkey-patching in parser Jul 1, 2025
@vadimkibana vadimkibana enabled auto-merge (squash) July 1, 2025 12:23
@vadimkibana vadimkibana merged commit 750cc7c into elastic:main Jul 1, 2025
10 checks passed
@kibanamachine
Copy link
Contributor

kibanamachine commented Jul 1, 2025

Starting backport for target branches: 8.19, 9.1

https://github.com/elastic/kibana/actions/runs/16001261032

@elasticmachine
Copy link
Contributor

elasticmachine commented Jul 1, 2025

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] x-pack/platform/test/alerting_api_integration/security_and_spaces/group1/config.ts / alerting api integration security and spaces enabled Alerts - Group 1 alerts rule gaps get gaps summary by rule ids space_1_all_with_restricted_fixture at space1 get gaps summary by rule ids (space_1_all_with_restricted_fixture at space1) should return gaps summary for multiple rules

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 9.8MB 9.8MB -223.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
kbnUiSharedDeps-srcJs 3.7MB 3.7MB -92.0B

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 1, 2025
@vadimkibana
[ES|QL] Stop ANTLR token monkey-patching in parser (elastic#225629)
6409fd7 
## Summary

A small cleanup in parser code in the ES|QL AST package. We want to show
users readable syntax error token names like `[` and `(` (instead of
`LP`), however, we should not monkey-patch ANTLR generated files to
achieve this. Instead, this PR moves the message improvement code to the
validation & autocomplete package, where the messages are actually used.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit 750cc7c)
@kibanamachine kibanamachine mentioned this pull request Jul 1, 2025
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 1, 2025
@vadimkibana
[ES|QL] Stop ANTLR token monkey-patching in parser (elastic#225629)
1f60f28 
## Summary

A small cleanup in parser code in the ES|QL AST package. We want to show
users readable syntax error token names like `[` and `(` (instead of
`LP`), however, we should not monkey-patch ANTLR generated files to
achieve this. Instead, this PR moves the message improvement code to the
validation & autocomplete package, where the messages are actually used.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit 750cc7c)
@kibanamachine kibanamachine mentioned this pull request Jul 1, 2025
Merged
@kibanamachine
Copy link
Contributor

kibanamachine commented Jul 1, 2025

💚 All backports created successfully

Status Branch Result
✅ 8.19
✅ 9.1

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@kibanamachine kibanamachine mentioned this pull request Jul 1, 2025
Merged
8 tasks
kibanamachine added a commit that referenced this pull request Jul 1, 2025
@kibanamachine @vadimkibana
[8.19] [ES|QL] Stop ANTLR token monkey-patching in parser (#225629) (#…
fa52d1c 
…226010)

# Backport

This will backport the following commits from `main` to `8.19`:
- [[ES|QL] Stop ANTLR token monkey-patching in parser
(#225629)](#225629)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Vadim
Kibana","email":"82822460+vadimkibana@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-07-01T13:50:01Z","message":"[ES|QL]
Stop ANTLR token monkey-patching in parser (#225629)\n\n## Summary\n\nA
small cleanup in parser code in the ES|QL AST package. We want to
show\nusers readable syntax error token names like `[` and `(` (instead
of\n`LP`), however, we should not monkey-patch ANTLR generated files
to\nachieve this. Instead, this PR moves the message improvement code to
the\nvalidation & autocomplete package, where the messages are actually
used.\n\n\n### Checklist\n\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"750cc7c4c1b91f2713b97a27c89e07dadffbb816","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["review","release_note:skip","Feature:ES|QL","Team:ESQL","backport:version","v9.1.0","v8.19.0","v9.2.0"],"title":"[ES|QL]
Stop ANTLR token monkey-patching in
parser","number":225629,"url":"https://github.com/elastic/kibana/pull/225629","mergeCommit":{"message":"[ES|QL]
Stop ANTLR token monkey-patching in parser (#225629)\n\n## Summary\n\nA
small cleanup in parser code in the ES|QL AST package. We want to
show\nusers readable syntax error token names like `[` and `(` (instead
of\n`LP`), however, we should not monkey-patch ANTLR generated files
to\nachieve this. Instead, this PR moves the message improvement code to
the\nvalidation & autocomplete package, where the messages are actually
used.\n\n\n### Checklist\n\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"750cc7c4c1b91f2713b97a27c89e07dadffbb816"}},"sourceBranch":"main","suggestedTargetBranches":["9.1","8.19"],"targetPullRequestStates":[{"branch":"9.1","label":"v9.1.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/225629","number":225629,"mergeCommit":{"message":"[ES|QL]
Stop ANTLR token monkey-patching in parser (#225629)\n\n## Summary\n\nA
small cleanup in parser code in the ES|QL AST package. We want to
show\nusers readable syntax error token names like `[` and `(` (instead
of\n`LP`), however, we should not monkey-patch ANTLR generated files
to\nachieve this. Instead, this PR moves the message improvement code to
the\nvalidation & autocomplete package, where the messages are actually
used.\n\n\n### Checklist\n\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"750cc7c4c1b91f2713b97a27c89e07dadffbb816"}}]}]
BACKPORT-->

Co-authored-by: Vadim Kibana <82822460+vadimkibana@users.noreply.github.com>
kibanamachine added a commit that referenced this pull request Jul 1, 2025
@kibanamachine @vadimkibana
[9.1] [ES|QL] Stop ANTLR token monkey-patching in parser (#225629) (#…
209a5df 
…226011)

# Backport

This will backport the following commits from `main` to `9.1`:
- [[ES|QL] Stop ANTLR token monkey-patching in parser
(#225629)](#225629)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Vadim
Kibana","email":"82822460+vadimkibana@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-07-01T13:50:01Z","message":"[ES|QL]
Stop ANTLR token monkey-patching in parser (#225629)\n\n## Summary\n\nA
small cleanup in parser code in the ES|QL AST package. We want to
show\nusers readable syntax error token names like `[` and `(` (instead
of\n`LP`), however, we should not monkey-patch ANTLR generated files
to\nachieve this. Instead, this PR moves the message improvement code to
the\nvalidation & autocomplete package, where the messages are actually
used.\n\n\n### Checklist\n\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"750cc7c4c1b91f2713b97a27c89e07dadffbb816","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["review","release_note:skip","Feature:ES|QL","Team:ESQL","backport:version","v9.1.0","v8.19.0","v9.2.0"],"title":"[ES|QL]
Stop ANTLR token monkey-patching in
parser","number":225629,"url":"https://github.com/elastic/kibana/pull/225629","mergeCommit":{"message":"[ES|QL]
Stop ANTLR token monkey-patching in parser (#225629)\n\n## Summary\n\nA
small cleanup in parser code in the ES|QL AST package. We want to
show\nusers readable syntax error token names like `[` and `(` (instead
of\n`LP`), however, we should not monkey-patch ANTLR generated files
to\nachieve this. Instead, this PR moves the message improvement code to
the\nvalidation & autocomplete package, where the messages are actually
used.\n\n\n### Checklist\n\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"750cc7c4c1b91f2713b97a27c89e07dadffbb816"}},"sourceBranch":"main","suggestedTargetBranches":["9.1","8.19"],"targetPullRequestStates":[{"branch":"9.1","label":"v9.1.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/225629","number":225629,"mergeCommit":{"message":"[ES|QL]
Stop ANTLR token monkey-patching in parser (#225629)\n\n## Summary\n\nA
small cleanup in parser code in the ES|QL AST package. We want to
show\nusers readable syntax error token names like `[` and `(` (instead
of\n`LP`), however, we should not monkey-patch ANTLR generated files
to\nachieve this. Instead, this PR moves the message improvement code to
the\nvalidation & autocomplete package, where the messages are actually
used.\n\n\n### Checklist\n\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"750cc7c4c1b91f2713b97a27c89e07dadffbb816"}}]}]
BACKPORT-->

Co-authored-by: Vadim Kibana <82822460+vadimkibana@users.noreply.github.com>
@drewdaemon
Copy link
Contributor

drewdaemon commented Jul 14, 2025 •
edited
Loading

We lost the literal tokens because of some grammar changes from Elasticsearch. Elasticsearch then added this substitution logic to their parser and I did the same to ours. I preferred it that way because it is fixing the bug at the level it was introduced (the parser). That said, I guess I have no strong objection to moving the substitutions to the validation level

But I would like us to not lose the comment with the reference to the Elasticsearch change. That points us to their substitutions which we should probably strive to match. I will reinstate that link in a follow-up PR.

drewdaemon added a commit that referenced this pull request Jul 14, 2025
@drewdaemon
[ES|QL] reinstate comment
3be63d5 
Follow-on to #225629
@drewdaemon drewdaemon mentioned this pull request Jul 14, 2025
Merged
drewdaemon added a commit that referenced this pull request Jul 16, 2025
@drewdaemon
[ES|QL] reinstate comment (#227884)
1abf412 
Follow-on to #225629
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 16, 2025
@drewdaemon
[ES|QL] reinstate comment (elastic#227884)
caadbaa 
Follow-on to elastic#225629

(cherry picked from commit 1abf412)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 16, 2025
@drewdaemon
[ES|QL] reinstate comment (elastic#227884)
aac5af6 
Follow-on to elastic#225629

(cherry picked from commit 1abf412)
Bluefinger pushed a commit to Bluefinger/kibana that referenced this pull request Jul 22, 2025
@drewdaemon @Bluefinger
[ES|QL] reinstate comment (elastic#227884)
dc7b6a0 
Follow-on to elastic#225629
kertal pushed a commit to kertal/kibana that referenced this pull request Jul 25, 2025
@vadimkibana @kertal
[ES|QL] Stop ANTLR token monkey-patching in parser (elastic#225629)
9efe558 
## Summary

A small cleanup in parser code in the ES|QL AST package. We want to show
users readable syntax error token names like `[` and `(` (instead of
`LP`), however, we should not monkey-patch ANTLR generated files to
achieve this. Instead, this PR moves the message improvement code to the
validation & autocomplete package, where the messages are actually used.


### Checklist


- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
kertal pushed a commit to kertal/kibana that referenced this pull request Jul 25, 2025
@drewdaemon @kertal
[ES|QL] reinstate comment (elastic#227884)
4b408df 
Follow-on to elastic#225629
@drewdaemon drewdaemon mentioned this pull request Sep 9, 2025
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sddonne sddonne sddonne approved these changes

Assignees

No one assigned

Labels

backport:version Backport to applied version labels Feature:ES|QL ES|QL related features in Kibana release_note:skip Skip the PR/issue when compiling release notes review Team:ESQL ES|QL related features in Kibana t// v8.19.0 v9.1.0 v9.2.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@vadimkibana @elasticmachine @kibanamachine @drewdaemon @sddonne