[Aggs] Force return 0 on empty buckets on count if null flag is disabled (#207308)

## Summary

Fixes #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>
This commit is contained in:
Marco Liberati 2025-02-05 12:04:09 +01:00 committed by GitHub
parent 86497d5e7f
commit 0d9ce86d0b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 91 additions and 0 deletions

View file

@ -0,0 +1,87 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/
import moment from 'moment';
import { IMetricAggConfig } from './metric_agg_type';
import { getCountMetricAgg } from './count';
function makeAgg(emptyAsNull: boolean = false, timeShift?: moment.Duration): IMetricAggConfig {
return {
getTimeShift() {
return timeShift;
},
params: {
emptyAsNull,
},
} as IMetricAggConfig;
}
function getBucket(value: number | undefined, timeShift?: moment.Duration) {
const suffix = timeShift ? `_${timeShift.asMilliseconds()}` : '';
return { ['doc_count' + suffix]: value };
}
describe('Count', () => {
it('should return the value for a non-shifted bucket', () => {
const agg = getCountMetricAgg();
expect(agg.getValue(makeAgg(), getBucket(1000))).toBe(1000);
});
it('should return the value for a shifted bucket', () => {
const agg = getCountMetricAgg();
const shift = moment.duration(1800000);
expect(agg.getValue(makeAgg(false, shift), getBucket(1000, shift))).toBe(1000);
});
describe('emptyAsNull', () => {
it('should return null if the value is 0 and the flag is enabled', () => {
const agg = getCountMetricAgg();
expect(agg.getValue(makeAgg(true), getBucket(0))).toBe(null);
});
it('should return null if the value is undefined and the flag is enabled', () => {
const agg = getCountMetricAgg();
expect(agg.getValue(makeAgg(true), getBucket(undefined))).toBe(null);
});
it('should return null if the value is 0 and the flag is enabled on a shifted count', () => {
const agg = getCountMetricAgg();
const shift = moment.duration(1800000);
expect(agg.getValue(makeAgg(true, shift), getBucket(0, shift))).toBe(null);
});
it('should return null if the value is undefined and the flag is enabled on a shifted count', () => {
const agg = getCountMetricAgg();
const shift = moment.duration(1800000);
expect(agg.getValue(makeAgg(true, shift), getBucket(undefined, shift))).toBe(null);
});
it('should return 0 if the value is 0 and the flag is disabled', () => {
const agg = getCountMetricAgg();
expect(agg.getValue(makeAgg(false), getBucket(0))).toBe(0);
});
it('should return 0 if the value is undefined and the flag is disabled', () => {
const agg = getCountMetricAgg();
expect(agg.getValue(makeAgg(false), getBucket(undefined))).toBe(0);
});
it('should return 0 if the value is 0 and the flag is disabled on a shifted count', () => {
const agg = getCountMetricAgg();
const shift = moment.duration(1800000);
expect(agg.getValue(makeAgg(false, shift), getBucket(undefined, shift))).toBe(0);
});
it('should return 0 if the value is undefined and the flag is disabled on a shifted count', () => {
const agg = getCountMetricAgg();
const shift = moment.duration(1800000);
expect(agg.getValue(makeAgg(false, shift), getBucket(undefined, shift))).toBe(0);
});
});
});

View file

@ -48,6 +48,10 @@ export const getCountMetricAgg = () =>
if (value === 0 && agg.params.emptyAsNull) {
return null;
}
if (value == null) {
// if the value is undefined, respect the emptyAsNull flag
return agg.params.emptyAsNull ? null : 0;
}
return value;
},
isScalable() {