[Aggs] Force return 0 on empty buckets on count if null flag is disabled#207308
[Aggs] Force return 0 on empty buckets on count if null flag is disabled#207308dej611 merged 11 commits intoelastic:mainfrom
Conversation
|
/ci |
|
Pinging @elastic/kibana-visualizations (Team:Visualizations) |
lukasolson
left a comment
There was a problem hiding this comment.
Code & functionality LGTM
markov00
left a comment
There was a problem hiding this comment.
This doesn't fully solve what was requested in the original issue.
null/zero is one thing, but the fact that count with timeshift doesn't use the extended_bounds is not addressed by the PR.
Actually not just count but also other metrics doesn't include the extended_bounds when using timeshift.
This results is missing buckets at the beginning and end of the table if they are empty/zero.
src/platform/plugins/shared/data/common/search/aggs/metrics/count.test.ts
Outdated
Show resolved
Hide resolved
|
If I'm not wrong the current concept of But in this PR we are also doing the opposite: we are saying that |
I do not think the problem is related with the If we want to extends this behaviour to other aggs as well I think we'll need to rethink the agg model and how they work. |
…unt.test.ts Co-authored-by: Marco Vettorello <vettorello.marco@gmail.com>
|
Discussed offline with @markov00 this PR. For the partial result we've tested a PoC who seems to work, but needs to be validated in a follow up PR. |
…unt.ts Co-authored-by: Peter Pisljar <peter.pisljar@gmail.com>
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Page load bundle
History
|
|
Starting backport for target branches: 9.0 |
…led (elastic#207308) ## Summary Fixes elastic#206555 This PR is an attempt to address the `null` bucket issue with `count` in Lens formula via the `emptyAsNull` flag. ### 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 ### Risks This PR introduces potentially some breaking changes, as count `null` values, in particular coming from shifted computations, as now converted to `0` if the flag has been enabled. This change is not news in the code base as other aggs like `distinct_count` or `value_count` already implements it, but not `count`. Apparently no test failed with this change, I've also added new unit ones to freeze the current behaviour and detect future changes. --------- Co-authored-by: Marco Vettorello <vettorello.marco@gmail.com> Co-authored-by: Peter Pisljar <peter.pisljar@gmail.com> (cherry picked from commit 0d9ce86)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
… disabled (#207308) (#209737) # Backport This will backport the following commits from `main` to `9.0`: - [[Aggs] Force return 0 on empty buckets on count if null flag is disabled (#207308)](#207308) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Marco Liberati","email":"dej611@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-02-05T11:04:09Z","message":"[Aggs] Force return 0 on empty buckets on count if null flag is disabled (#207308)\n\n## Summary\r\n\r\nFixes #206555 \r\n\r\nThis PR is an attempt to address the `null` bucket issue with `count` in\r\nLens formula via the `emptyAsNull` flag.\r\n\r\n### Checklist\r\n\r\n* [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n### Risks\r\n\r\nThis PR introduces potentially some breaking changes, as count `null`\r\nvalues, in particular coming from shifted computations, as now converted\r\nto `0` if the flag has been enabled.\r\nThis change is not news in the code base as other aggs like\r\n`distinct_count` or `value_count` already implements it, but not\r\n`count`.\r\nApparently no test failed with this change, I've also added new unit\r\nones to freeze the current behaviour and detect future changes.\r\n\r\n---------\r\n\r\nCo-authored-by: Marco Vettorello <vettorello.marco@gmail.com>\r\nCo-authored-by: Peter Pisljar <peter.pisljar@gmail.com>","sha":"0d9ce86d0b81089598e8483d8e67bafa210a510b","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Visualizations","Feature:Lens","backport:prev-minor","v9.1.0"],"title":"[Aggs] Force return 0 on empty buckets on count if null flag is disabled","number":207308,"url":"https://github.com/elastic/kibana/pull/207308","mergeCommit":{"message":"[Aggs] Force return 0 on empty buckets on count if null flag is disabled (#207308)\n\n## Summary\r\n\r\nFixes #206555 \r\n\r\nThis PR is an attempt to address the `null` bucket issue with `count` in\r\nLens formula via the `emptyAsNull` flag.\r\n\r\n### Checklist\r\n\r\n* [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n### Risks\r\n\r\nThis PR introduces potentially some breaking changes, as count `null`\r\nvalues, in particular coming from shifted computations, as now converted\r\nto `0` if the flag has been enabled.\r\nThis change is not news in the code base as other aggs like\r\n`distinct_count` or `value_count` already implements it, but not\r\n`count`.\r\nApparently no test failed with this change, I've also added new unit\r\nones to freeze the current behaviour and detect future changes.\r\n\r\n---------\r\n\r\nCo-authored-by: Marco Vettorello <vettorello.marco@gmail.com>\r\nCo-authored-by: Peter Pisljar <peter.pisljar@gmail.com>","sha":"0d9ce86d0b81089598e8483d8e67bafa210a510b"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/207308","number":207308,"mergeCommit":{"message":"[Aggs] Force return 0 on empty buckets on count if null flag is disabled (#207308)\n\n## Summary\r\n\r\nFixes #206555 \r\n\r\nThis PR is an attempt to address the `null` bucket issue with `count` in\r\nLens formula via the `emptyAsNull` flag.\r\n\r\n### Checklist\r\n\r\n* [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n### Risks\r\n\r\nThis PR introduces potentially some breaking changes, as count `null`\r\nvalues, in particular coming from shifted computations, as now converted\r\nto `0` if the flag has been enabled.\r\nThis change is not news in the code base as other aggs like\r\n`distinct_count` or `value_count` already implements it, but not\r\n`count`.\r\nApparently no test failed with this change, I've also added new unit\r\nones to freeze the current behaviour and detect future changes.\r\n\r\n---------\r\n\r\nCo-authored-by: Marco Vettorello <vettorello.marco@gmail.com>\r\nCo-authored-by: Peter Pisljar <peter.pisljar@gmail.com>","sha":"0d9ce86d0b81089598e8483d8e67bafa210a510b"}}]}] BACKPORT--> Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>
…led (elastic#207308) ## Summary Fixes elastic#206555 This PR is an attempt to address the `null` bucket issue with `count` in Lens formula via the `emptyAsNull` flag. ### 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 ### Risks This PR introduces potentially some breaking changes, as count `null` values, in particular coming from shifted computations, as now converted to `0` if the flag has been enabled. This change is not news in the code base as other aggs like `distinct_count` or `value_count` already implements it, but not `count`. Apparently no test failed with this change, I've also added new unit ones to freeze the current behaviour and detect future changes. --------- Co-authored-by: Marco Vettorello <vettorello.marco@gmail.com> Co-authored-by: Peter Pisljar <peter.pisljar@gmail.com>
Summary
Fixes #206555
This PR is an attempt to address the
nullbucket issue withcountin Lens formula via theemptyAsNullflag.Checklist
Risks
This PR introduces potentially some breaking changes, as count
nullvalues, in particular coming from shifted computations, as now converted to0if the flag has been enabled.This change is not news in the code base as other aggs like
distinct_countorvalue_countalready implements it, but notcount.Apparently no test failed with this change, I've also added new unit ones to freeze the current behaviour and detect future changes.