[ES|QL] Kibana parser issues #166173
Assignees
Labels
bug
Fixes for quality problems that affect the customer experience
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:Visualizations
Visualization editors, elastic-charts and infrastructure
Comments
dej611
added
bug
|
Pinging @elastic/kibana-visualizations (Team:Visualizations) |
stratoula
added
the
impact:medium
65 tasks
|
Handled as part of #166242 |
dej611
added a commit
that referenced
this issue
Oct 26, 2023
## Summary Fixes #166242 This is a PR to use an higher level AST instead of the raw ParserTree to compute all Monaco features for ES|QL. ## Grammar This PR drops the existing grammar in favour of the ES one, which a tiny tweak in order to support any case for the query. I think we could fully adopt the ES grammar without tweaks once [this issue](tunnelvisionlabs/antlr4ts#535) is resolved in the antlr4ts dependency. The adoption of the ES grammar helped also to resolve #166173 . <img width="407" alt="Screenshot 2023-09-21 at 12 16 45" src="https://github.com/elastic/kibana/assets/924948/38aa507b-8371-4387-ab8a-95423cdab684"> <img width="427" alt="Screenshot 2023-09-21 at 12 16 40" src="https://github.com/elastic/kibana/assets/924948/462d9b57-24cf-45f8-bcf1-070bbdb67817"> <img width="444" alt="Screenshot 2023-09-21 at 12 16 29" src="https://github.com/elastic/kibana/assets/924948/a6b5a904-c16f-40c6-a377-4e3e2005975f"> <img width="415" alt="Screenshot 2023-09-21 at 12 16 23" src="https://github.com/elastic/kibana/assets/924948/e7732f91-17b2-4af5-9d52-0eb96fa1999e"> <img width="404" alt="Screenshot 2023-09-21 at 12 16 17" src="https://github.com/elastic/kibana/assets/924948/99311f65-1390-4a90-8bb6-12b263d808fd"> ## AST generator The AST is a subset of the ESQL grammar parser tree, which is easier to navigate. Tests will be provided to the tool in order to test that the tool can translate even the craziest query string. **Note** that this is not preventing the query to be submitted to ES: even if a validation case fails here or the AST fails to translate part of the expression ES will always have the last word about error and warnings, which will be promptly notified to the user. Commands coverage: * `ROW` * [x] `ROW a=1` * [x] `ROW a=1, b=1` * `FROM` * [x] `FROM a` * [x] `FROM a, b` * [x] `FROM a [METADATA ...]` * [x] `FROM a, b [METADATA ...]` * `METADATA` won't have indentifiers yet * `SHOW` * [x] `INFO` * [x] `FUNCTIONS` * `EVAL` * [x] `EVAL a = b + 1` * [x] `EVAL a = fnA( ... ) + fnB( ... )` * [x] `EVAL b + 1` * [x] `EVAL fnA( ... ) + fnB( ... )` * `STATS` * [x] `STATS a = fn(b) + fn(c)` * [x] `STATS a = fn(b) + fn(c) BY group` * [x] `STATS a = fn(b) + fn(c) BY groupA, groupB` * [x] `STATS fn(b) + fn(c)` * [x] `STATS fn(b) + fn(c) BY groupA` * [x] `STATS fn(b) + fn(c) BY groupA, groupB` * [x] `WHERE` * [x] `LIMIT` * [x] `KEEP` * [x] `PROJECT` * [x] `DROP` * `RENAME`. * [x] `RENAME a as b` * [x] `RENAME `a + 1 / 5` as `myField` * [x] `DISSECT` * [x] `GROK` * [x] `ENRICH` * [x] `MV_EXPAND` * [x] `SORT` Collect all user defined variables * [x] Basic support for assigned variables (i.e. `eval a = ...`) * [x] Field type shadowing with assignment * [x] Advanced support for non-assigned cases? (i.e. `eval a + 1`) ## Validation With this new approach it would be possible to catch most of warnings and errors while typing, preventing users to submit invalid queries to ES. Note that here the validation function can detect multiple errors and warnings at once, while ES only returns one error at the time - improving the current sub-optimal editing experience provided.  Tasks * Create definitions * [x] Define function definitions * [x] Define aggs definitions * [x] Define command definitions * [x] Define command options definitions * Create basic validation walker function * [x] Support both errors and warnings * [x] Validate commands * [x] Validate command options * [x] Validate functions * [x] Validate source/columns (I phase - just formally) * [x] Validate policies (I phase - just formally) * [x] Validate literals * [x] Validate duration/math syntax * [x] Validate variables * Improve validation work with high level checks * [x] Validate source/columns (II phase - leverage ES metadata) * [x] Validate policies (II phase - leverage ES metadata) * [x] Optimize queries * phase I: just make queries strictly when required * Write tests * [x] Source commands * [x] Options * [x] Processing commands * [x] Options * [x] Sorting commands * [x] All options, combinations * [x] Enrich commands * [x] Options * [x] Warnings cases * [x] Integrate all of it as Monaco diagnostics module * [x] Solve issue with long queries and single line mode * Provide better sync between client and server side error management * [x] Show server side warnings only on absence of errors * [x] Clear up server side errors on code change ## Autocomplete Using the same provider here to provide better contextual suggestions. First example giving better suggestions (filtered field based on types and functions based on returned type):  * Commands * [x] Show all commands on empty string * [x] Show all but source commands after first pipe * [x] Suggest option at the right moment ## Hover Basic hover information about used functions: <img width="688" alt="Screenshot 2023-10-16 at 16 22 16" src="https://github.com/elastic/kibana/assets/924948/e623c71d-4bbc-4511-8bb8-d818e465e554"> <img width="533" alt="Screenshot 2023-10-16 at 16 21 54" src="https://github.com/elastic/kibana/assets/924948/af7a244f-be5b-46a0-8704-09c1ffc48a9e"> # Things not covered in this PR * Load function definitions from `SHOW FUNCTIONS` at language bootstrap * Make `validate` function an independent module? * Provide a "formal" validation option (i.e. mute `Unknown <entity> [...]` messages) * Have `autocomplete` on par with the current state in `main` ## Validation cases not covered yet * `... | eval var = 1 year` => ❌ it should report an error * `... | eval 1 + [1, 2, 3]` =>⚠️ it should report a null warning * same applies for `-`, `*`, `/`, `%` * any location aware field/variable check * `... | stats a = avg(field) | keep b` => ❌ `b` should be reported as unknown * `... | keep a | eval b` => ❌ `b` should be reported as unknown * `... | rename a as b, d as c, c as b` => ❌ `c` should be reported as unknown * `... | enrich policy on fieldFromEnrich` => ❌ `fieldFromEnrich` is not reported as unknown * `... | eval a = round(b), b = round(a)` => ❌ `b` should be reported an unknown * `from ... [metadata <list of meta fields>` => ❌ `meta fields` won't be validated as there's currently no available API to get them ## Hover Advanced usage of hover feature. ## Signature TBD --------- Co-authored-by: kibanamachine <[email protected]>
23 tasks
dej611
added a commit
that referenced
this issue
Dec 7, 2023
## Summary Fixes #166242 , #166876, #166173 , part of #166092, #166084 List of tasks: * [x] AST work ( #166185 ) * [x] Basic validate work ( #166185 ) * [x] Hover feature ( #166185 ) * [x] Initial autocomplete work with new AST ( #166185 ) * Complete validation feature for MVP * [x] wildcards support ( #170014 ) * [x] remote index validation support ( #171996 ) * [x] wildcard support as `count` argument ( #172054 ) * [x] Aggressive caching for field queries: * cache as much as possible the `FROM` queries - possible clear the cache every 10/15 minutes? * do not fire a query when code == submitted code * cache as much as possible the custom `FROM` built from `ENRICH` policies - same clear policy as above * [x] Add unsupported fields warning messages * [x] Notify usage of `project` command with deprecation `warning` * Complete autocomplete work ( #171664 ) * [x] `stats` * [x] `where` * [x] `eval` * `math syntax` * [x] Aggressive cache for fields queries? ( #171866 ) * [x] Fix when cursor is not at the end position ( #172060 ) * [x] Revisit copy messages * Label Kibana-only messages * [x] Extend hover feature ( #171940 ) * [x] Disable editor query highlight for warnings ( #171968 ) * Fix editor highlight with new grammar * [x] on multi-line ( #172080 ) * [x] for functions ( #172287 ) ## Release notes Enhance ES|QL query editing experience with client side validation. Enhance ES|QL suggestions experience with more in context suggestions leveraging field and variable types. Show meta informations on ES|QL query hover on policy names. Show function signature on ES|QL query hover on function text. ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [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 - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Stratoula Kalafateli <[email protected]> Co-authored-by: Abdon Pijpelink <[email protected]>
|
Fixed by #170071 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug:
These are some example of query parser issues found so far:
from kibana_sample_data_ecommerce | drop "wrong|index"[esql] > Couldn't parse Elasticsearch ES|QL query. You may need to add backticks to names containing special characters. Check your query and try again. Error: line 1:49: mismatched input 'index"' expecting {'dissect', 'drop', 'enrich', 'eval', 'grok', 'inlinestats', 'keep', 'limit', 'mv_expand', 'project', 'rename', 'sort', 'stats', 'where'}(probably due to some pipe string manipulation in the code while fetching fields)wrong|indexworks fine with back ticksfrom kibana_sample_data_ecommerce | drop wrong-index-char with no quotes, but ES will make it pass correctlyfrom kibana_sample_data_ecommerce | drop 'wrong-index'from kibana_sample_data_ecommerce | drop "wrong-index"`as quoting char. I think it makes sense to cover also single'and double"quote chars. wdyt @stratoula ?'category'is a different field fromcategory, which looks strange to me. (e.g. Lens formula considers both the same field)from kibana_sample_data_ecommerce | drop 123123The text was updated successfully, but these errors were encountered: