-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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] METRICS
command definition and validation
#184905
[ES|QL] METRICS
command definition and validation
#184905
Conversation
I've added validation to check for agg-in-agg function errors. |
@drewdaemon I've solved the missing field error by using |
Vadim do you want to do this in this PR or a follow-up? #186005 |
Just tried it out. Looks great! |
@drewdaemon yes, I'll better to do it as a follow up, this is already too big. |
/** | ||
* Looks for first nested aggregate function in an aggregate function, recursively. | ||
*/ | ||
const findNestedAggFunctionOfAggFunction = (agg: ESQLFunction): ESQLFunction | undefined => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const findNestedAggFunctionOfAggFunction = (agg: ESQLFunction): ESQLFunction | undefined => { | |
const findNestedAggFunctionInAggFunction = (agg: ESQLFunction): ESQLFunction | undefined => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with a couple comments. Thanks for all your work on this!
packages/kbn-esql-validation-autocomplete/src/validation/validation.ts
Outdated
Show resolved
Hide resolved
packages/kbn-esql-validation-autocomplete/src/validation/validation.ts
Outdated
Show resolved
Hide resolved
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Public APIs missing comments
Page load bundle
Unknown metric groupsAPI count
History
To update your PR or re-run it, just comment with: |
## Summary Partially addresses elastic#184498 The main contribution of this PR is the `METRICS` command validation cases: <img width="778" alt="image" src="https://github.com/elastic/kibana/assets/82822460/3d768952-3fa3-4928-b251-204c30d20c4b"> See own-review below for more comments. ### 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#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <[email protected]>
## Summary Partially addresses elastic#184498 The main contribution of this PR is the `METRICS` command validation cases: <img width="778" alt="image" src="https://github.com/elastic/kibana/assets/82822460/3d768952-3fa3-4928-b251-204c30d20c4b"> See own-review below for more comments. ### 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#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <[email protected]>
Summary
Partially addresses #184498
The main contribution of this PR is the
METRICS
command validation cases:See own-review below for more comments.
Checklist
Delete any items that are not applicable to this PR.
For maintainers