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

Auto-completion wrongly suggesting size field when using terms agg within composite agg #120512

Closed
wants to merge 4 commits into from

Conversation

vladpro25
Copy link
Contributor

@vladpro25 vladpro25 commented Dec 6, 2021

Closes: #53112

Describe the issue:

[Console] Auto-completion wrongly suggesting size field when using terms agg within composite agg

Steps to reproduce

Using Dev Tools autocomplete:

  1. Create a composite aggregation
  2. Use terms aggregation as source
    (e.g., follow this tutorial: https://www.elastic.co/blog/composite-aggregations-elasticsearch-pizza-delivery-metrics)

Final result

terms aggregation within composite aggregation does not allow for size field.

Confirmation

NewProject

@vladpro25 vladpro25 self-assigned this Dec 6, 2021
@vladpro25 vladpro25 added Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more Feature:Dev Tools Feature:Console Dev Tools Console Feature v8.1.0 labels Dec 6, 2021
@vladpro25
Copy link
Contributor Author

@elasticmachine merge upstream

@vladpro25 vladpro25 marked this pull request as ready for review December 6, 2021 16:53
@vladpro25 vladpro25 requested a review from a team as a code owner December 6, 2021 16:53
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-stack-management (Team:Stack Management)

Copy link
Contributor

@yuliacech yuliacech left a comment

Choose a reason for hiding this comment

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

Hi @vladpro25, thank you so much for opening this PR!
If I understand correctly, your suggested change will remove size parameter from terms aggregation. But the linked issue requests to remove size fromterms aggregation only when it's used as a source in composite aggregation. Could you please confirm with the Elasticsearch team that size parameter can't be used when terms is a source for composite aggregation? I was not able to find this in the docs.
Terms aggregation docs
Composite aggregation docs

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

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

cc @vladpro25

Copy link
Contributor

@alexwizp alexwizp left a comment

Choose a reason for hiding this comment

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

@vladpro25 it's definitely wrong fix. size should be reverted for terms aggregation.

@yuliacech I've tried execute composite aggregation locally and looks like ES doesn't supports that case:

image

But not sure that it's too easy to understand that we are inside of composite agg. @vladpro25 let's discuss it offline tomorrow

@yuliacech
Copy link
Contributor

Thanks for checking this, @alexwizp! I opened this docs issue elastic/elasticsearch#81431 to add this information to the Elasticsearch guide.

@yuliacech
Copy link
Contributor

Hi @vladpro25, the current assumption in the code is that aggregations behave the same when used alone and when used as a source for other aggregations, like terms is used inside composite aggregation. This makes excluding size param from the terms aggregation only when used as a source difficult.

@alexwizp alexwizp closed this Feb 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Console Dev Tools Console Feature Feature:Dev Tools release_note:fix Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Console] Auto-completion wrongly suggesting size field when using terms agg within composite agg
6 participants