Skip to content

[ES|QL] Add support for STATS BY nested function#175288

Merged
dej611 merged 12 commits intoelastic:mainfrom
dej611:fix/esql-nested-stats
Jan 24, 2024
Merged

[ES|QL] Add support for STATS BY nested function#175288
dej611 merged 12 commits intoelastic:mainfrom
dej611:fix/esql-nested-stats

Conversation

@dej611
Copy link
Copy Markdown
Contributor

@dej611 dej611 commented Jan 23, 2024

Summary

Add support for the validation and autocomplete engine for the feature introduced in elastic/elasticsearch#104387

The validation engine should not mark the syntax as invalid now.
The autocomplete changes are a little bit more subtle:

  • after the by option command a varX and functions are now suggested in additions to [columns]
  • when typing an expression within the by scope the autocomplete should now understand and help with that, without promoting it when not needed.

by_suggestions

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 Jan 23, 2024
@dej611
Copy link
Copy Markdown
Contributor Author

dej611 commented Jan 23, 2024

/ci

@dej611
Copy link
Copy Markdown
Contributor Author

dej611 commented Jan 23, 2024

/ci

@dej611
Copy link
Copy Markdown
Contributor Author

dej611 commented Jan 23, 2024

/ci

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

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

@stratoula
Copy link
Copy Markdown
Contributor

@elasticmachine merge upstream

@stratoula
Copy link
Copy Markdown
Contributor

/ci

@stratoula
Copy link
Copy Markdown
Contributor

@dej611 as this is a very cool feature, can we suggest also all the eval functions (as they are supported).

image

@dej611
Copy link
Copy Markdown
Contributor Author

dej611 commented Jan 24, 2024

I wasn't sure whether to suggest them or not, but I can enable that.
I was just concerned by the suggestion long list in this context.
To be sure, this is what I can do:

  • suggest a function after the by
  • suggest a function after the = (assign) operator within the by scope

Does it make sense to you @stratoula ?

@stratoula
Copy link
Copy Markdown
Contributor

Yes!!! I think is cool to suggest them even if the list goes long!

@dej611
Copy link
Copy Markdown
Contributor Author

dej611 commented Jan 24, 2024

/ci

@dej611
Copy link
Copy Markdown
Contributor Author

dej611 commented Jan 24, 2024

Updated logic to propose also functions.
Added also additional tests for more scenarios.

by_suggestions

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.

This looks great Marco! LGTM, thanx a ton

@kibana-ci
Copy link
Copy Markdown

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #63 / discover/group3 discover default columns "after all" hook for "should not change columns if discover:modifyColumnsOnSwitch is off"

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.3MB 2.3MB +483.0B

History

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

@dej611
Copy link
Copy Markdown
Contributor Author

dej611 commented Jan 24, 2024

I've completely missed the support for nested functions in the STATS scope in this PR.
I'll make a follow up PR with the support for that.

@dej611 dej611 merged commit 183039d into elastic:main Jan 24, 2024
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Jan 24, 2024
dej611 added a commit that referenced this pull request Jan 29, 2024
## Summary

This is the second part of #175288 where nested expressions are
supported in STATS scope as well.
I've tweaked the previous test suite here to check all agg functions
with nested expressions in various scenarios.

### 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 <efstratia.kalafateli@elastic.co>
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
## Summary

Add support for the validation and autocomplete engine for the feature
introduced in elastic/elasticsearch#104387

The validation engine should not mark the syntax as invalid now.
The autocomplete changes are a little bit more subtle:
* after the `by` option command a `varX` and functions are now suggested
in additions to `[columns]`
* when typing an expression within the `by` scope the autocomplete
should now understand and help with that, without promoting it when not
needed.


![by_suggestions](https://github.com/elastic/kibana/assets/924948/266c403b-34a6-436f-a5aa-43353b77269e)


### 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: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
## Summary

This is the second part of elastic#175288 where nested expressions are
supported in STATS scope as well.
I've tweaked the previous test suite here to check all agg functions
with nested expressions in various scenarios.

### 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 <efstratia.kalafateli@elastic.co>
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
## Summary

This is the second part of elastic#175288 where nested expressions are
supported in STATS scope as well.
I've tweaked the previous test suite here to check all agg functions
with nested expressions in various scenarios.

### 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 <efstratia.kalafateli@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

Development

Successfully merging this pull request may close these issues.

5 participants