-
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
[Metrics UI] Fix OR logic on redundant groupBy detection #116695
Conversation
Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI) |
💚 Build Succeeded
Metrics [docs]Async chunks
To update your PR or re-run it, just comment with: |
I wonder if instead of trying to validate their inputs we can make it more generic and anytime they are filtering on a field that is also being grouped there can be a generic warning message that they may get unexpected results and then link to some documentation explaining how and why with some examples. Even if they are using an OR like in the issue #116195, it seems still valid to notify them that they'd get less results than may be expected. |
@neptunian Yeah that approach makes more sense. I can see about writing up some docs for this and redo this PR. |
Summary
Fixes #116195
In this PR,
host.name
will still be flagged as redundant in this case:But with these changes, it will NOT be flagged in this case: