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] Open bracket parsing bug #196241

Merged
merged 8 commits into from
Oct 18, 2024
Merged

[ES|QL] Open bracket parsing bug #196241

merged 8 commits into from
Oct 18, 2024
+26 −2

Conversation

vadimkibana
Copy link
Contributor

@vadimkibana vadimkibana commented Oct 15, 2024 •
edited by kibanamachine
Loading

Summary

Closes #191683

Gracefully handles open square brackets in FROM command.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@vadimkibana vadimkibana added bug Fixes for quality problems that affect the customer experience review release_note:skip Skip the PR/issue when compiling release notes v9.0.0 backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Feature:ES|QL ES|QL related features in Kibana Team:ESQL ES|QL related features in Kibana v8.16.0 labels Oct 15, 2024
@vadimkibana vadimkibana requested a review from a team as a code owner October 15, 2024 08:15
@elasticmachine
Copy link
Contributor

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

@stratoula
Copy link
Contributor

I tested that and it works great 🙌 . I see a unit test failing though, can you check it vadim?

@vadimkibana vadimkibana enabled auto-merge (squash) October 15, 2024 13:44
stratoula
stratoula approved these changes Oct 16, 2024
View reviewed changes
Copy link
Contributor

@stratoula stratoula left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Bug has been fixed! (but CI still fails)

auto-merge was automatically disabled October 17, 2024 15:53

Head branch was pushed to by a user without write access

@streamich streamich requested a review from a team as a code owner October 17, 2024 15:53
@vadimkibana vadimkibana enabled auto-merge (squash) October 17, 2024 21:00
vadimkibana
vadimkibana commented Oct 17, 2024
View reviewed changes
...gins/security_solution/public/detection_engine/rule_creation_ui/hooks/use_esql_index.test.ts
it('should return empty array if invalid query is causing a TypeError in ES|QL parser', async () => {
const typeErrorCausingQuery = 'from auditbeat* []';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test worked by accident in the past. The parser threw an error here, not because of the query syntax error, but because of an internal error in the parser that crashed parsing.

@stratoula stratoula added v8.17.0 backport:version Backport to applied version labels and removed backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Oct 18, 2024
vitaliidm
vitaliidm reviewed Oct 18, 2024
View reviewed changes
Copy link
Contributor

@vitaliidm vitaliidm left a comment

Choose a reason for hiding this comment

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

Page crashes when I trying to insert [ symbol

Screen.Recording.2024-10-18.at.11.36.36.mov

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #66 / console app text input with a data URI in the load_from query with invalid data shows a toast error

Metrics [docs]

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.4MB 3.4MB +14.0B

History

@stratoula
Copy link
Contributor

@vitaliidm it works fine for me

image

Have you ran kbn bootstrap first?

vitaliidm
vitaliidm approved these changes Oct 18, 2024
View reviewed changes
Copy link
Contributor

@vitaliidm vitaliidm left a comment

Choose a reason for hiding this comment

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

I refetched and rebuilt branch.
Everything works, thanks for working on it

@vadimkibana vadimkibana merged commit 3849d99 into elastic:main Oct 18, 2024
45 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.16, 8.x

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

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 18, 2024
@vadimkibana
[ES|QL] Open bracket parsing bug (elastic#196241)
e6e8b3b 
## Summary

Closes elastic#191683

Gracefully handles open square brackets in `FROM` command.

### Checklist

Delete any items that are not applicable to this PR.

- [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

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)

(cherry picked from commit 3849d99)
@kibanamachine kibanamachine mentioned this pull request Oct 18, 2024
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 18, 2024
@vadimkibana
[ES|QL] Open bracket parsing bug (elastic#196241)
57e59f6 
## Summary

Closes elastic#191683

Gracefully handles open square brackets in `FROM` command.

### Checklist

Delete any items that are not applicable to this PR.

- [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

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)

(cherry picked from commit 3849d99)
@kibanamachine kibanamachine mentioned this pull request Oct 18, 2024
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.16
✅ 8.x

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

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 18, 2024
@kibanamachine @vadimkibana
[8.16] [ES|QL] Open bracket parsing bug (#196241) (#196892)
d146fb9 
# Backport

This will backport the following commits from `main` to `8.16`:
- [[ES|QL] Open bracket parsing bug
(#196241)](#196241)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Vadim
Kibana","email":"82822460+vadimkibana@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-10-18T14:15:29Z","message":"[ES|QL]
Open bracket parsing bug (#196241)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/191683\r\n\r\nGracefully
handles open square brackets in `FROM` command.\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n### For
maintainers\r\n\r\n- [x] This was checked for breaking API changes and
was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)","sha":"3849d99a41e2e42c9a9e06b97c3affa2e75999d1","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","review","release_note:skip","v9.0.0","Feature:ES|QL","Team:ESQL","v8.16.0","backport:version","v8.17.0"],"title":"[ES|QL]
Open bracket parsing
bug","number":196241,"url":"https://github.com/elastic/kibana/pull/196241","mergeCommit":{"message":"[ES|QL]
Open bracket parsing bug (#196241)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/191683\r\n\r\nGracefully
handles open square brackets in `FROM` command.\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n### For
maintainers\r\n\r\n- [x] This was checked for breaking API changes and
was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)","sha":"3849d99a41e2e42c9a9e06b97c3affa2e75999d1"}},"sourceBranch":"main","suggestedTargetBranches":["8.16","8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/196241","number":196241,"mergeCommit":{"message":"[ES|QL]
Open bracket parsing bug (#196241)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/191683\r\n\r\nGracefully
handles open square brackets in `FROM` command.\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n### For
maintainers\r\n\r\n- [x] This was checked for breaking API changes and
was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)","sha":"3849d99a41e2e42c9a9e06b97c3affa2e75999d1"}},{"branch":"8.16","label":"v8.16.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Vadim Kibana <82822460+vadimkibana@users.noreply.github.com>
kibanamachine added a commit that referenced this pull request Oct 18, 2024
@kibanamachine @vadimkibana
[8.x] [ES|QL] Open bracket parsing bug (#196241) (#196893)
2296239 
# Backport

This will backport the following commits from `main` to `8.x`:
- [[ES|QL] Open bracket parsing bug
(#196241)](#196241)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Vadim
Kibana","email":"82822460+vadimkibana@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-10-18T14:15:29Z","message":"[ES|QL]
Open bracket parsing bug (#196241)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/191683\r\n\r\nGracefully
handles open square brackets in `FROM` command.\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n### For
maintainers\r\n\r\n- [x] This was checked for breaking API changes and
was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)","sha":"3849d99a41e2e42c9a9e06b97c3affa2e75999d1","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","review","release_note:skip","v9.0.0","Feature:ES|QL","Team:ESQL","v8.16.0","backport:version","v8.17.0"],"title":"[ES|QL]
Open bracket parsing
bug","number":196241,"url":"https://github.com/elastic/kibana/pull/196241","mergeCommit":{"message":"[ES|QL]
Open bracket parsing bug (#196241)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/191683\r\n\r\nGracefully
handles open square brackets in `FROM` command.\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n### For
maintainers\r\n\r\n- [x] This was checked for breaking API changes and
was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)","sha":"3849d99a41e2e42c9a9e06b97c3affa2e75999d1"}},"sourceBranch":"main","suggestedTargetBranches":["8.16","8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/196241","number":196241,"mergeCommit":{"message":"[ES|QL]
Open bracket parsing bug (#196241)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/191683\r\n\r\nGracefully
handles open square brackets in `FROM` command.\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n### For
maintainers\r\n\r\n- [x] This was checked for breaking API changes and
was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)","sha":"3849d99a41e2e42c9a9e06b97c3affa2e75999d1"}},{"branch":"8.16","label":"v8.16.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Vadim Kibana <82822460+vadimkibana@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stratoula stratoula stratoula approved these changes

@vitaliidm vitaliidm vitaliidm approved these changes

Assignees
No one assigned
Labels
backport:version Backport to applied version labels bug Fixes for quality problems that affect the customer experience 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 v8.16.0 v8.17.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[ES|QL] entering [ after a source breaks the editor
5 participants
@vadimkibana @elasticmachine @stratoula @kibanamachine @vitaliidm