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.

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

[ES|QL] Use non array types for multi-value functions #174709

Closed
dej611 opened this issue Jan 11, 2024 · 1 comment · Fixed by #175986
Closed

[ES|QL] Use non array types for multi-value functions #174709

dej611 opened this issue Jan 11, 2024 · 1 comment · Fixed by #175986
Assignees
Labels
enhancement New value added to drive a business result Feature:ES|QL ES|QL related features in Kibana Team:Visualizations Visualization editors, elastic-charts and infrastructure

Comments

@dej611
Copy link
Contributor

dej611 commented Jan 11, 2024

Describe the bug:

Currently the Kibana ES|QL validation code thinks that multi-values are marked as array/list type somehow from ES, but they are not. They have actually the same type as anything else.
This is a problem when using any mv_* functions as regular fields are marked as error.

So we need to fix this on the Kibana validation code:

  • fix the mv_* function signatures
  • remove any list/array special logic in the validation code.
@dej611 dej611 added enhancement New value added to drive a business result Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:ES|QL ES|QL related features in Kibana labels Jan 11, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-visualizations (Team:Visualizations)

mistic pushed a commit that referenced this issue Feb 1, 2024
…on (#175986)

## Summary

This PR is a batch of fixes and (tiny) new features for validation and
autocomplete items in ES|QL client side area.
Content of the PR:
* 🐛 remove array/multi-value definition annotation for `mv_*`
functions - fix #174709
<img width="305" alt="Screenshot 2024-01-31 at 13 43 37"
src="https://github.com/elastic/kibana/assets/924948/2a458e74-9c87-4a67-8b42-0071e7a04f48">
<img width="287" alt="Screenshot 2024-01-31 at 13 43 25"
src="https://github.com/elastic/kibana/assets/924948/7205d984-d0c9-410b-8b39-c1d530c3f54f">
* while the type check has been relaxed for lists, the actual
non-multivalued argument type is retained and used for validation
<img width="758" alt="Screenshot 2024-01-31 at 13 44 06"
src="https://github.com/elastic/kibana/assets/924948/384e9dfa-aaf8-4e0f-953e-a2d656776ea7">

* 🐛 Remove array/multi-value definition annotation for `mv_expand`
command - fix #174709

<img width="556" alt="Screenshot 2024-01-31 at 14 36 19"
src="https://github.com/elastic/kibana/assets/924948/0debbccc-da9a-4e6a-bfc9-3fded61300b5">
<img width="277" alt="Screenshot 2024-01-31 at 14 36 13"
src="https://github.com/elastic/kibana/assets/924948/9438832b-d8d5-48c5-a030-b8983057546e">


* ✨ add metadata fields validation + autocomplete (fields are
retrieved via Kibana `uiSettings`) ( part of #172646 )
  

![metadata_fix](https://github.com/elastic/kibana/assets/924948/958e6609-ab73-4949-8652-f368c6604a09)
  
<img width="760" alt="Screenshot 2024-01-31 at 13 45 43"
src="https://github.com/elastic/kibana/assets/924948/b3f13d99-3e12-4586-b39c-f4ea3691a2f1">
  * ✨ Quick fixes now available also for metadata fields
  

![metadata_fixes](https://github.com/elastic/kibana/assets/924948/b273418f-a754-4c50-9539-a128f35fc4cd)

* 🐛 fixed autocomplete for `NOT` operation

<img width="549" alt="Screenshot 2024-01-31 at 13 46 43"
src="https://github.com/elastic/kibana/assets/924948/29b7aa7c-db66-485a-a5b0-329e66df842f">

* fixed autocomplete for `in` operation, with or without `not`:
* ✨ now a missing list bracket is correctly detected and
suggested after `in`
* ✨ the content for the list is suggested and takes into
account both the left side of the `in`/`not in` operator, but also the
current content of the list


![in_list_fix_2](https://github.com/elastic/kibana/assets/924948/50ea8db5-23da-411c-b81e-5ba3397b65c1)

![in_list_fix](https://github.com/elastic/kibana/assets/924948/cbb73ad2-4073-4eb6-9c12-61835858fd5e)

![not_in_list_fix](https://github.com/elastic/kibana/assets/924948/9d7fa756-3d34-49fe-ac7a-28e2edbe7fee)

* fixed `grok` and `dissect` in various ways
*:sparkles: the pattern provided by autocomplete for both was not valid
in `dissect` nor useful at all, so I've changed to something more useful
like `'"%{WORD:firstWord}"'` for `grok` and `'"%{firstWord}"'` for
`dissect` to match the first word in the text field.
* :bug: there was a bug in the validation engine as both `grok` and
`dissect` could generate new columns based on matches, so now a new
field query is fired (only when either a `GROK` or a `DISSECT` command
is detected) which enriches the set of fields available (similar to the
enrich fields)
  * ✅ Added tons of tests here


![grok_fix](https://github.com/elastic/kibana/assets/924948/a23c8edc-0702-4531-aaa5-6126e375913b)

![dissect_fix](https://github.com/elastic/kibana/assets/924948/d5591753-775e-4768-be00-31c4371be330)

* 🐛 fixed an issue with proposing an assign (`=`) operator when it
should not
  * `... EVAL var0 = round( ...) <here>` not within another assignment
  * `... EVAL var0 = 1 <here>` not within another assignment
  * `... EVAL var0 = 1 year + 2 <here>` not within another assignment
  * `... | WHERE field` should not shadow a field name in this case

* 🐛 fix an annoying auto trigger suggestion on field selection, and
kept it only for functions

![auto_trigger_fix](https://github.com/elastic/kibana/assets/924948/31d0c296-6338-450d-8f81-e08cd1c7526d)

* 🐛 fixed an error in console with autocomplete when a typed
function does not exists
* ✅  Add tests for the hover feature
* 🔥 Removed some unused functions detected via code coverage
analysis


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

---------

Co-authored-by: Stratoula Kalafateli <[email protected]>
fkanout pushed a commit to fkanout/kibana that referenced this issue Feb 7, 2024
…on (elastic#175986)

## Summary

This PR is a batch of fixes and (tiny) new features for validation and
autocomplete items in ES|QL client side area.
Content of the PR:
* 🐛 remove array/multi-value definition annotation for `mv_*`
functions - fix elastic#174709
<img width="305" alt="Screenshot 2024-01-31 at 13 43 37"
src="https://github.com/elastic/kibana/assets/924948/2a458e74-9c87-4a67-8b42-0071e7a04f48">
<img width="287" alt="Screenshot 2024-01-31 at 13 43 25"
src="https://github.com/elastic/kibana/assets/924948/7205d984-d0c9-410b-8b39-c1d530c3f54f">
* while the type check has been relaxed for lists, the actual
non-multivalued argument type is retained and used for validation
<img width="758" alt="Screenshot 2024-01-31 at 13 44 06"
src="https://github.com/elastic/kibana/assets/924948/384e9dfa-aaf8-4e0f-953e-a2d656776ea7">

* 🐛 Remove array/multi-value definition annotation for `mv_expand`
command - fix elastic#174709

<img width="556" alt="Screenshot 2024-01-31 at 14 36 19"
src="https://github.com/elastic/kibana/assets/924948/0debbccc-da9a-4e6a-bfc9-3fded61300b5">
<img width="277" alt="Screenshot 2024-01-31 at 14 36 13"
src="https://github.com/elastic/kibana/assets/924948/9438832b-d8d5-48c5-a030-b8983057546e">


* ✨ add metadata fields validation + autocomplete (fields are
retrieved via Kibana `uiSettings`) ( part of elastic#172646 )
  

![metadata_fix](https://github.com/elastic/kibana/assets/924948/958e6609-ab73-4949-8652-f368c6604a09)
  
<img width="760" alt="Screenshot 2024-01-31 at 13 45 43"
src="https://github.com/elastic/kibana/assets/924948/b3f13d99-3e12-4586-b39c-f4ea3691a2f1">
  * ✨ Quick fixes now available also for metadata fields
  

![metadata_fixes](https://github.com/elastic/kibana/assets/924948/b273418f-a754-4c50-9539-a128f35fc4cd)

* 🐛 fixed autocomplete for `NOT` operation

<img width="549" alt="Screenshot 2024-01-31 at 13 46 43"
src="https://github.com/elastic/kibana/assets/924948/29b7aa7c-db66-485a-a5b0-329e66df842f">

* fixed autocomplete for `in` operation, with or without `not`:
* ✨ now a missing list bracket is correctly detected and
suggested after `in`
* ✨ the content for the list is suggested and takes into
account both the left side of the `in`/`not in` operator, but also the
current content of the list


![in_list_fix_2](https://github.com/elastic/kibana/assets/924948/50ea8db5-23da-411c-b81e-5ba3397b65c1)

![in_list_fix](https://github.com/elastic/kibana/assets/924948/cbb73ad2-4073-4eb6-9c12-61835858fd5e)

![not_in_list_fix](https://github.com/elastic/kibana/assets/924948/9d7fa756-3d34-49fe-ac7a-28e2edbe7fee)

* fixed `grok` and `dissect` in various ways
*:sparkles: the pattern provided by autocomplete for both was not valid
in `dissect` nor useful at all, so I've changed to something more useful
like `'"%{WORD:firstWord}"'` for `grok` and `'"%{firstWord}"'` for
`dissect` to match the first word in the text field.
* :bug: there was a bug in the validation engine as both `grok` and
`dissect` could generate new columns based on matches, so now a new
field query is fired (only when either a `GROK` or a `DISSECT` command
is detected) which enriches the set of fields available (similar to the
enrich fields)
  * ✅ Added tons of tests here


![grok_fix](https://github.com/elastic/kibana/assets/924948/a23c8edc-0702-4531-aaa5-6126e375913b)

![dissect_fix](https://github.com/elastic/kibana/assets/924948/d5591753-775e-4768-be00-31c4371be330)

* 🐛 fixed an issue with proposing an assign (`=`) operator when it
should not
  * `... EVAL var0 = round( ...) <here>` not within another assignment
  * `... EVAL var0 = 1 <here>` not within another assignment
  * `... EVAL var0 = 1 year + 2 <here>` not within another assignment
  * `... | WHERE field` should not shadow a field name in this case

* 🐛 fix an annoying auto trigger suggestion on field selection, and
kept it only for functions

![auto_trigger_fix](https://github.com/elastic/kibana/assets/924948/31d0c296-6338-450d-8f81-e08cd1c7526d)

* 🐛 fixed an error in console with autocomplete when a typed
function does not exists
* ✅  Add tests for the hover feature
* 🔥 Removed some unused functions detected via code coverage
analysis


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

---------

Co-authored-by: Stratoula Kalafateli <[email protected]>
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this issue Feb 15, 2024
…on (elastic#175986)

## Summary

This PR is a batch of fixes and (tiny) new features for validation and
autocomplete items in ES|QL client side area.
Content of the PR:
* 🐛 remove array/multi-value definition annotation for `mv_*`
functions - fix elastic#174709
<img width="305" alt="Screenshot 2024-01-31 at 13 43 37"
src="https://github.com/elastic/kibana/assets/924948/2a458e74-9c87-4a67-8b42-0071e7a04f48">
<img width="287" alt="Screenshot 2024-01-31 at 13 43 25"
src="https://github.com/elastic/kibana/assets/924948/7205d984-d0c9-410b-8b39-c1d530c3f54f">
* while the type check has been relaxed for lists, the actual
non-multivalued argument type is retained and used for validation
<img width="758" alt="Screenshot 2024-01-31 at 13 44 06"
src="https://github.com/elastic/kibana/assets/924948/384e9dfa-aaf8-4e0f-953e-a2d656776ea7">

* 🐛 Remove array/multi-value definition annotation for `mv_expand`
command - fix elastic#174709

<img width="556" alt="Screenshot 2024-01-31 at 14 36 19"
src="https://github.com/elastic/kibana/assets/924948/0debbccc-da9a-4e6a-bfc9-3fded61300b5">
<img width="277" alt="Screenshot 2024-01-31 at 14 36 13"
src="https://github.com/elastic/kibana/assets/924948/9438832b-d8d5-48c5-a030-b8983057546e">


* ✨ add metadata fields validation + autocomplete (fields are
retrieved via Kibana `uiSettings`) ( part of elastic#172646 )
  

![metadata_fix](https://github.com/elastic/kibana/assets/924948/958e6609-ab73-4949-8652-f368c6604a09)
  
<img width="760" alt="Screenshot 2024-01-31 at 13 45 43"
src="https://github.com/elastic/kibana/assets/924948/b3f13d99-3e12-4586-b39c-f4ea3691a2f1">
  * ✨ Quick fixes now available also for metadata fields
  

![metadata_fixes](https://github.com/elastic/kibana/assets/924948/b273418f-a754-4c50-9539-a128f35fc4cd)

* 🐛 fixed autocomplete for `NOT` operation

<img width="549" alt="Screenshot 2024-01-31 at 13 46 43"
src="https://github.com/elastic/kibana/assets/924948/29b7aa7c-db66-485a-a5b0-329e66df842f">

* fixed autocomplete for `in` operation, with or without `not`:
* ✨ now a missing list bracket is correctly detected and
suggested after `in`
* ✨ the content for the list is suggested and takes into
account both the left side of the `in`/`not in` operator, but also the
current content of the list


![in_list_fix_2](https://github.com/elastic/kibana/assets/924948/50ea8db5-23da-411c-b81e-5ba3397b65c1)

![in_list_fix](https://github.com/elastic/kibana/assets/924948/cbb73ad2-4073-4eb6-9c12-61835858fd5e)

![not_in_list_fix](https://github.com/elastic/kibana/assets/924948/9d7fa756-3d34-49fe-ac7a-28e2edbe7fee)

* fixed `grok` and `dissect` in various ways
*:sparkles: the pattern provided by autocomplete for both was not valid
in `dissect` nor useful at all, so I've changed to something more useful
like `'"%{WORD:firstWord}"'` for `grok` and `'"%{firstWord}"'` for
`dissect` to match the first word in the text field.
* :bug: there was a bug in the validation engine as both `grok` and
`dissect` could generate new columns based on matches, so now a new
field query is fired (only when either a `GROK` or a `DISSECT` command
is detected) which enriches the set of fields available (similar to the
enrich fields)
  * ✅ Added tons of tests here


![grok_fix](https://github.com/elastic/kibana/assets/924948/a23c8edc-0702-4531-aaa5-6126e375913b)

![dissect_fix](https://github.com/elastic/kibana/assets/924948/d5591753-775e-4768-be00-31c4371be330)

* 🐛 fixed an issue with proposing an assign (`=`) operator when it
should not
  * `... EVAL var0 = round( ...) <here>` not within another assignment
  * `... EVAL var0 = 1 <here>` not within another assignment
  * `... EVAL var0 = 1 year + 2 <here>` not within another assignment
  * `... | WHERE field` should not shadow a field name in this case

* 🐛 fix an annoying auto trigger suggestion on field selection, and
kept it only for functions

![auto_trigger_fix](https://github.com/elastic/kibana/assets/924948/31d0c296-6338-450d-8f81-e08cd1c7526d)

* 🐛 fixed an error in console with autocomplete when a typed
function does not exists
* ✅  Add tests for the hover feature
* 🔥 Removed some unused functions detected via code coverage
analysis


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

---------

Co-authored-by: Stratoula Kalafateli <[email protected]>
fkanout pushed a commit to fkanout/kibana that referenced this issue Mar 4, 2024
…on (elastic#175986)

## Summary

This PR is a batch of fixes and (tiny) new features for validation and
autocomplete items in ES|QL client side area.
Content of the PR:
* 🐛 remove array/multi-value definition annotation for `mv_*`
functions - fix elastic#174709
<img width="305" alt="Screenshot 2024-01-31 at 13 43 37"
src="https://github.com/elastic/kibana/assets/924948/2a458e74-9c87-4a67-8b42-0071e7a04f48">
<img width="287" alt="Screenshot 2024-01-31 at 13 43 25"
src="https://github.com/elastic/kibana/assets/924948/7205d984-d0c9-410b-8b39-c1d530c3f54f">
* while the type check has been relaxed for lists, the actual
non-multivalued argument type is retained and used for validation
<img width="758" alt="Screenshot 2024-01-31 at 13 44 06"
src="https://github.com/elastic/kibana/assets/924948/384e9dfa-aaf8-4e0f-953e-a2d656776ea7">

* 🐛 Remove array/multi-value definition annotation for `mv_expand`
command - fix elastic#174709

<img width="556" alt="Screenshot 2024-01-31 at 14 36 19"
src="https://github.com/elastic/kibana/assets/924948/0debbccc-da9a-4e6a-bfc9-3fded61300b5">
<img width="277" alt="Screenshot 2024-01-31 at 14 36 13"
src="https://github.com/elastic/kibana/assets/924948/9438832b-d8d5-48c5-a030-b8983057546e">


* ✨ add metadata fields validation + autocomplete (fields are
retrieved via Kibana `uiSettings`) ( part of elastic#172646 )
  

![metadata_fix](https://github.com/elastic/kibana/assets/924948/958e6609-ab73-4949-8652-f368c6604a09)
  
<img width="760" alt="Screenshot 2024-01-31 at 13 45 43"
src="https://github.com/elastic/kibana/assets/924948/b3f13d99-3e12-4586-b39c-f4ea3691a2f1">
  * ✨ Quick fixes now available also for metadata fields
  

![metadata_fixes](https://github.com/elastic/kibana/assets/924948/b273418f-a754-4c50-9539-a128f35fc4cd)

* 🐛 fixed autocomplete for `NOT` operation

<img width="549" alt="Screenshot 2024-01-31 at 13 46 43"
src="https://github.com/elastic/kibana/assets/924948/29b7aa7c-db66-485a-a5b0-329e66df842f">

* fixed autocomplete for `in` operation, with or without `not`:
* ✨ now a missing list bracket is correctly detected and
suggested after `in`
* ✨ the content for the list is suggested and takes into
account both the left side of the `in`/`not in` operator, but also the
current content of the list


![in_list_fix_2](https://github.com/elastic/kibana/assets/924948/50ea8db5-23da-411c-b81e-5ba3397b65c1)

![in_list_fix](https://github.com/elastic/kibana/assets/924948/cbb73ad2-4073-4eb6-9c12-61835858fd5e)

![not_in_list_fix](https://github.com/elastic/kibana/assets/924948/9d7fa756-3d34-49fe-ac7a-28e2edbe7fee)

* fixed `grok` and `dissect` in various ways
*:sparkles: the pattern provided by autocomplete for both was not valid
in `dissect` nor useful at all, so I've changed to something more useful
like `'"%{WORD:firstWord}"'` for `grok` and `'"%{firstWord}"'` for
`dissect` to match the first word in the text field.
* :bug: there was a bug in the validation engine as both `grok` and
`dissect` could generate new columns based on matches, so now a new
field query is fired (only when either a `GROK` or a `DISSECT` command
is detected) which enriches the set of fields available (similar to the
enrich fields)
  * ✅ Added tons of tests here


![grok_fix](https://github.com/elastic/kibana/assets/924948/a23c8edc-0702-4531-aaa5-6126e375913b)

![dissect_fix](https://github.com/elastic/kibana/assets/924948/d5591753-775e-4768-be00-31c4371be330)

* 🐛 fixed an issue with proposing an assign (`=`) operator when it
should not
  * `... EVAL var0 = round( ...) <here>` not within another assignment
  * `... EVAL var0 = 1 <here>` not within another assignment
  * `... EVAL var0 = 1 year + 2 <here>` not within another assignment
  * `... | WHERE field` should not shadow a field name in this case

* 🐛 fix an annoying auto trigger suggestion on field selection, and
kept it only for functions

![auto_trigger_fix](https://github.com/elastic/kibana/assets/924948/31d0c296-6338-450d-8f81-e08cd1c7526d)

* 🐛 fixed an error in console with autocomplete when a typed
function does not exists
* ✅  Add tests for the hover feature
* 🔥 Removed some unused functions detected via code coverage
analysis


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

---------

Co-authored-by: Stratoula Kalafateli <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Feature:ES|QL ES|QL related features in Kibana Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants