Skip to content

[ES|QL] New sync with ES changes#176283

Merged
dej611 merged 19 commits intoelastic:mainfrom
dej611:feat/esql-stats-and-logs-sync
Feb 9, 2024
Merged

[ES|QL] New sync with ES changes#176283
dej611 merged 19 commits intoelastic:mainfrom
dej611:feat/esql-stats-and-logs-sync

Conversation

@dej611
Copy link
Copy Markdown
Contributor

@dej611 dej611 commented Feb 6, 2024 •
edited
Loading

Summary

Sync with elastic/elasticsearch#104958 for support of builtin fn in STATS

  • validation ✅
  • autocomplete ✅
  • also fixed STATS BY <field> syntax

new_stats

Sync with elastic/elasticsearch#104913 for new log function

  • validation ✅ - also warning for negative values
  • autocomplete ✅
    add_log

Sync with elastic/elasticsearch#105064 for removal of PROJECT command

  • validation ✅ (both new and legacy syntax supported)
  • autocomplete ✅ (will only suggest new syntax)

remove_project

Sync with elastic/elasticsearch#105221 for removal of mandatory brackets for METADATA command option

  • validation ✅ (added warning deprecation message when using brackets)
  • autocomplete ✅
    fix_metadata

Sync with elastic/elasticsearch#105224 for change of syntax for ENRICH ccq mode

  • validation ✅
  • autocomplete ✅ (not directly promoted, the user has to type _ to trigger it)
  • hover ✅
  • code actions ✅
    fix_ccq_enrich
    fix_ccq_enrich_2

Do not merge until those 5 get merged.

Additional things in this PR:

  • Added more tests for callbacks not passed scenario
    • covered more cases like those with dissect
  • Added more tests for signature params number (calling a function with an extra arg should return an error)
  • Cleaned up some more unused code
  • Improved messages on too many arguments for functions

Checklist

@dej611 dej611 added Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// release_note:skip Skip the PR/issue when compiling release notes Feature:ES|QL ES|QL related features in Kibana v8.13.0 labels Feb 6, 2024
@dej611
Copy link
Copy Markdown
Contributor Author

dej611 commented Feb 6, 2024

/ci

@dej611
Copy link
Copy Markdown
Contributor Author

dej611 commented Feb 8, 2024

/ci

@dej611
Copy link
Copy Markdown
Contributor Author

dej611 commented Feb 8, 2024

/ci

@dej611
Copy link
Copy Markdown
Contributor Author

dej611 commented Feb 8, 2024

/ci

@dej611
Copy link
Copy Markdown
Contributor Author

dej611 commented Feb 8, 2024

/ci

@dej611 dej611 marked this pull request as ready for review February 9, 2024 08:26
@dej611 dej611 requested a review from a team as a code owner February 9, 2024 08:26
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Feb 9, 2024

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

@dej611
Copy link
Copy Markdown
Contributor Author

dej611 commented Feb 9, 2024

/ci

stratoula
stratoula reviewed Feb 9, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

Great job as always! One observation:

  • This is valid (and pretty awesome) but we mark it as invalid
image

and in general nested math functions with aggregations are supported now!

@dej611
Copy link
Copy Markdown
Contributor Author

dej611 commented Feb 9, 2024

/ci

@dej611
Copy link
Copy Markdown
Contributor Author

dej611 commented Feb 9, 2024

@stratoula just pushed an extension to support that as well 👍

stratoula
stratoula approved these changes Feb 9, 2024
View reviewed changes
Copy link
Copy Markdown
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.

Works great! There is also the cidr_match validation problem but we can solve it either in this or in another PR. I am approving, nice addition!

image

We could improve the suggestions in stats but is not so important at this point. The most important thing is the client side validation and it works fine now 👍

@kibana-ci
Copy link
Copy Markdown

kibana-ci commented Feb 9, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

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 2.9MB 2.9MB +1.0KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@dej611
Copy link
Copy Markdown
Contributor Author

dej611 commented Feb 9, 2024

We could improve the suggestions in stats but is not so important at this point. The most important thing is the client side validation and it works fine now

That should be already in, no?

There is also the cidr_match validation problem but we can solve it either in this or in another PR. I am approving, nice addition!

As discussed offline I'd prefer to do it in a follow up.

@dej611 dej611 merged commit 7bbea56 into elastic:main Feb 9, 2024
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Feb 9, 2024
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
@dej611 @CoenWarmer
[ES|QL] New sync with ES changes (elastic#176283)
731dfe9 
## Summary

Sync with elastic/elasticsearch#104958 for
support of builtin fn in STATS
  * validation ✅ 
  * autocomplete ✅ 
  * also fixed `STATS BY <field>` syntax


![new_stats](https://github.com/elastic/kibana/assets/924948/735f9842-b1d3-4aa0-9d51-4b2f9b136ed3)


Sync with elastic/elasticsearch#104913 for new
`log` function
  * validation ✅  - also warning for negative values
  * autocomplete ✅ 

![add_log](https://github.com/elastic/kibana/assets/924948/146b945d-a23b-45ec-9df2-2d2b291e883b)

Sync with elastic/elasticsearch#105064 for
removal of `PROJECT` command
  * validation ✅  (both new and legacy syntax supported)
  * autocomplete ✅  (will only suggest new syntax)


![remove_project](https://github.com/elastic/kibana/assets/924948/b6f40afe-a26d-4917-b7a1-d8ae97c5368b)

Sync with elastic/elasticsearch#105221 for
removal of mandatory brackets for `METADATA` command option
* validation ✅ (added warning deprecation message when using brackets)
  * autocomplete ✅ 

![fix_metadata](https://github.com/elastic/kibana/assets/924948/c65db176-dd94-45f3-9524-45453e62f51a)


Sync with elastic/elasticsearch#105224 for
change of syntax for ENRICH ccq mode
  * validation ✅ 
* autocomplete ✅ (not directly promoted, the user has to type `_` to
trigger it)
  * hover ✅ 
  * code actions ✅ 

![fix_ccq_enrich](https://github.com/elastic/kibana/assets/924948/0900edd9-a0a7-4ac8-bc12-e39a72359984)

![fix_ccq_enrich_2](https://github.com/elastic/kibana/assets/924948/74b0908f-d385-4723-b3d4-c09108f47a73)


Do not merge until those 5 get merged.

Additional things in this PR:
* Added more tests for `callbacks` not passed scenario
  * covered more cases like those with `dissect`
* Added more tests for signature params number (calling a function with
an extra arg should return an error)
* Cleaned up some more unused code
* Improved messages on too many arguments for functions

### 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
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
@dej611 @fkanout
[ES|QL] New sync with ES changes (elastic#176283)
953209e 
## Summary

Sync with elastic/elasticsearch#104958 for
support of builtin fn in STATS
  * validation ✅ 
  * autocomplete ✅ 
  * also fixed `STATS BY <field>` syntax


![new_stats](https://github.com/elastic/kibana/assets/924948/735f9842-b1d3-4aa0-9d51-4b2f9b136ed3)


Sync with elastic/elasticsearch#104913 for new
`log` function
  * validation ✅  - also warning for negative values
  * autocomplete ✅ 

![add_log](https://github.com/elastic/kibana/assets/924948/146b945d-a23b-45ec-9df2-2d2b291e883b)

Sync with elastic/elasticsearch#105064 for
removal of `PROJECT` command
  * validation ✅  (both new and legacy syntax supported)
  * autocomplete ✅  (will only suggest new syntax)


![remove_project](https://github.com/elastic/kibana/assets/924948/b6f40afe-a26d-4917-b7a1-d8ae97c5368b)

Sync with elastic/elasticsearch#105221 for
removal of mandatory brackets for `METADATA` command option
* validation ✅ (added warning deprecation message when using brackets)
  * autocomplete ✅ 

![fix_metadata](https://github.com/elastic/kibana/assets/924948/c65db176-dd94-45f3-9524-45453e62f51a)


Sync with elastic/elasticsearch#105224 for
change of syntax for ENRICH ccq mode
  * validation ✅ 
* autocomplete ✅ (not directly promoted, the user has to type `_` to
trigger it)
  * hover ✅ 
  * code actions ✅ 

![fix_ccq_enrich](https://github.com/elastic/kibana/assets/924948/0900edd9-a0a7-4ac8-bc12-e39a72359984)

![fix_ccq_enrich_2](https://github.com/elastic/kibana/assets/924948/74b0908f-d385-4723-b3d4-c09108f47a73)


Do not merge until those 5 get merged.

Additional things in this PR:
* Added more tests for `callbacks` not passed scenario
  * covered more cases like those with `dissect`
* Added more tests for signature params number (calling a function with
an extra arg should return an error)
* Cleaned up some more unused code
* Improved messages on too many arguments for functions

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

Assignees

No one assigned

Labels

backport:skip This PR does not require backporting Feature:ES|QL ES|QL related features in Kibana release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v8.13.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@dej611 @elasticmachine @kibana-ci @stratoula @kibanamachine