Add option to pivots which allows omitting empty values. #15814
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dennisoelkers
force-pushed
the
fix/allow-skipping-empty-values-in-pivots
branch
from
dc4a50e
to
58a978b
Compare
...org/graylog/plugins/views/aggregations/SearchWithAggregationsSupportingMissingBucketsIT.java
Outdated
Show resolved
Hide resolved
dennisoelkers
force-pushed
the
fix/allow-skipping-empty-values-in-pivots
branch
from
1844740
to
2b8822b
Compare
9 tasks
|
Does not work when multiple fields added in single "Group by" widget:
The rest seems ok. |
luk-kaminski
approved these changes
Jul 12, 2023
linuspahl
reviewed
Jul 14, 2023
| </SkipEmptyValuesHoverForHelp> | ||
| </SkipEmptyValuesLabel> | ||
| </SkipEmptyValuesCheckbox> | ||
| )} |
There was a problem hiding this comment.
Have you considered unifying the layout with e.g. the visualization config checkboxes?
There was a problem hiding this comment.
Yes, I did it similar to the Rollup columns checkbox, because it is right next to it. I think we should create a follow-up task to make all consistent.
linuspahl
approved these changes
Jul 17, 2023
dennisoelkers
added a commit
that referenced
this pull request
Jul 18, 2023
* Add optional option to skip empty values for pivot config. * Implement strategy for skipping empty values when configured in pivot. * Allow option only for `values` pivot. * Fixing up inconsistencies. * Adding helper function for pivot creation. * Adding test case for disabling empty values. * Adding changelog snippet. * Adjusting expectations in test. * Reusing constant in test. * Handling skipping empty values for multi-field grouping. * Handle partially missing fields in aggregations properly. * Still use multi-terms in case empty values should be skipped.
9 tasks
linuspahl
pushed a commit
that referenced
this pull request
Jul 20, 2023
…5982) * Add optional option to skip empty values for pivot config. * Implement strategy for skipping empty values when configured in pivot. * Allow option only for `values` pivot. * Fixing up inconsistencies. * Adding helper function for pivot creation. * Adding test case for disabling empty values. * Adding changelog snippet. * Adjusting expectations in test. * Reusing constant in test. * Handling skipping empty values for multi-field grouping. * Handle partially missing fields in aggregations properly. * Still use multi-terms in case empty values should be skipped.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Motivation and Context
In
5.0, we introduced returning empty values for individual groupings whenever documents are missing fields which are used for aggregations. To allow the user to have more control about if these should be returned or not, this PR is introducing an option for each grouping to define if empty values should be skipped. By default this is off, both for widgets which are already existing and for new ones.Fixes Graylog2/graylog-plugin-enterprise#4851.
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: