From 62c095b258fdc01326092016dc5f28d28ecbe679 Mon Sep 17 00:00:00 2001 From: Alexey Antonov Date: Wed, 23 Sep 2020 10:48:55 +0300 Subject: [PATCH] 'Auto' interval must be correctly calculated for natural numbers (#77995) * 'Auto' interval must be correctly calculated for natural numbers * fix ts error * fix PR comments Co-authored-by: Elastic Machine --- .../common/search/aggs/buckets/histogram.ts | 1 + .../lib/histogram_calculate_interval.test.ts | 93 ++++++++++++++++++- .../lib/histogram_calculate_interval.ts | 27 +++++- 3 files changed, 117 insertions(+), 4 deletions(-) diff --git a/src/plugins/data/common/search/aggs/buckets/histogram.ts b/src/plugins/data/common/search/aggs/buckets/histogram.ts index 4b631e1fd7cd7..c3d3f041dd0c7 100644 --- a/src/plugins/data/common/search/aggs/buckets/histogram.ts +++ b/src/plugins/data/common/search/aggs/buckets/histogram.ts @@ -158,6 +158,7 @@ export const getHistogramBucketAgg = ({ maxBucketsUiSettings: getConfig(UI_SETTINGS.HISTOGRAM_MAX_BARS), maxBucketsUserInput: aggConfig.params.maxBars, intervalBase: aggConfig.params.intervalBase, + esTypes: aggConfig.params.field?.spec?.esTypes || [], }); }, }, diff --git a/src/plugins/data/common/search/aggs/buckets/lib/histogram_calculate_interval.test.ts b/src/plugins/data/common/search/aggs/buckets/lib/histogram_calculate_interval.test.ts index d3a95b32cd425..7e5e20e5917aa 100644 --- a/src/plugins/data/common/search/aggs/buckets/lib/histogram_calculate_interval.test.ts +++ b/src/plugins/data/common/search/aggs/buckets/lib/histogram_calculate_interval.test.ts @@ -21,6 +21,7 @@ import { calculateHistogramInterval, CalculateHistogramIntervalParams, } from './histogram_calculate_interval'; +import { ES_FIELD_TYPES } from '../../../../types'; describe('calculateHistogramInterval', () => { describe('auto calculating mode', () => { @@ -36,10 +37,91 @@ describe('calculateHistogramInterval', () => { min: 0, max: 1, }, + esTypes: [], }; }); describe('maxBucketsUserInput is defined', () => { + test('should set 1 as an interval for integer numbers that are less than maxBuckets #1', () => { + const p = { + ...params, + maxBucketsUserInput: 100, + values: { + min: 1, + max: 50, + }, + esTypes: [ES_FIELD_TYPES.INTEGER], + }; + expect(calculateHistogramInterval(p)).toEqual(1); + }); + + test('should set 1 as an interval for integer numbers that are less than maxBuckets #2', () => { + const p = { + ...params, + maxBucketsUiSettings: 1000, + maxBucketsUserInput: 258, + values: { + min: 521, + max: 689, + }, + esTypes: [ES_FIELD_TYPES.INTEGER], + }; + expect(calculateHistogramInterval(p)).toEqual(1); + }); + + test('should set correct interval for integer numbers that are greater than maxBuckets #1', () => { + const p = { + ...params, + maxBucketsUserInput: 100, + values: { + min: 400, + max: 790, + }, + esTypes: [ES_FIELD_TYPES.INTEGER, ES_FIELD_TYPES.SHORT], + }; + expect(calculateHistogramInterval(p)).toEqual(5); + }); + + test('should set correct interval for integer numbers that are greater than maxBuckets #2', () => { + // diff === 3456211; interval === 50000; buckets === 69 + const p = { + ...params, + maxBucketsUserInput: 100, + values: { + min: 567, + max: 3456778, + }, + esTypes: [ES_FIELD_TYPES.LONG], + }; + expect(calculateHistogramInterval(p)).toEqual(50000); + }); + + test('should not set integer interval if the field type is float #1', () => { + const p = { + ...params, + maxBucketsUserInput: 100, + values: { + min: 0, + max: 1, + }, + esTypes: [ES_FIELD_TYPES.FLOAT], + }; + expect(calculateHistogramInterval(p)).toEqual(0.01); + }); + + test('should not set integer interval if the field type is float #2', () => { + const p = { + ...params, + maxBucketsUserInput: 100, + values: { + min: 0, + max: 1, + }, + esTypes: [ES_FIELD_TYPES.INTEGER, ES_FIELD_TYPES.FLOAT], + }; + expect(calculateHistogramInterval(p)).toEqual(0.01); + }); + test('should not set interval which more than largest possible', () => { const p = { ...params, @@ -48,6 +130,7 @@ describe('calculateHistogramInterval', () => { min: 150, max: 250, }, + esTypes: [ES_FIELD_TYPES.SHORT], }; expect(calculateHistogramInterval(p)).toEqual(1); }); @@ -61,6 +144,7 @@ describe('calculateHistogramInterval', () => { min: 0.1, max: 0.9, }, + esTypes: [ES_FIELD_TYPES.FLOAT], }) ).toBe(0.02); }); @@ -74,6 +158,7 @@ describe('calculateHistogramInterval', () => { min: 10.45, max: 1000.05, }, + esTypes: [ES_FIELD_TYPES.FLOAT], }) ).toBe(100); }); @@ -88,6 +173,7 @@ describe('calculateHistogramInterval', () => { min: 0, max: 100, }, + esTypes: [ES_FIELD_TYPES.BYTE], }) ).toEqual(1); }); @@ -100,8 +186,9 @@ describe('calculateHistogramInterval', () => { min: 1, max: 10, }, + esTypes: [ES_FIELD_TYPES.INTEGER], }) - ).toEqual(0.1); + ).toEqual(1); }); test('should set intervals for integer numbers (diff more than maxBucketsUiSettings)', () => { @@ -113,6 +200,7 @@ describe('calculateHistogramInterval', () => { min: 45678, max: 90123, }, + esTypes: [ES_FIELD_TYPES.INTEGER], }) ).toEqual(500); }); @@ -127,6 +215,7 @@ describe('calculateHistogramInterval', () => { min: 1.245, max: 2.9, }, + esTypes: [ES_FIELD_TYPES.FLOAT], }) ).toEqual(0.02); expect( @@ -136,6 +225,7 @@ describe('calculateHistogramInterval', () => { min: 0.5, max: 2.3, }, + esTypes: [ES_FIELD_TYPES.FLOAT], }) ).toEqual(0.02); }); @@ -149,6 +239,7 @@ describe('calculateHistogramInterval', () => { min: 0.1, max: 0.9, }, + esTypes: [ES_FIELD_TYPES.FLOAT], }) ).toBe(0.01); }); diff --git a/src/plugins/data/common/search/aggs/buckets/lib/histogram_calculate_interval.ts b/src/plugins/data/common/search/aggs/buckets/lib/histogram_calculate_interval.ts index 378340e876296..313ecf1000f41 100644 --- a/src/plugins/data/common/search/aggs/buckets/lib/histogram_calculate_interval.ts +++ b/src/plugins/data/common/search/aggs/buckets/lib/histogram_calculate_interval.ts @@ -18,6 +18,7 @@ */ import { isAutoInterval } from '../_interval_options'; +import { ES_FIELD_TYPES } from '../../../../types'; interface IntervalValuesRange { min: number; @@ -28,6 +29,7 @@ export interface CalculateHistogramIntervalParams { interval: string; maxBucketsUiSettings: number; maxBucketsUserInput?: number | ''; + esTypes: ES_FIELD_TYPES[]; intervalBase?: number; values?: IntervalValuesRange; } @@ -77,11 +79,27 @@ const calculateForGivenInterval = ( - The lower power of 10, times 2 - The lower power of 10, times 5 **/ -const calculateAutoInterval = (diff: number, maxBars: number) => { +const calculateAutoInterval = (diff: number, maxBars: number, esTypes: ES_FIELD_TYPES[]) => { const exactInterval = diff / maxBars; - const lowerPower = Math.pow(10, Math.floor(Math.log10(exactInterval))); + // For integer fields that are less than maxBars, we should use 1 as the value of interval + // Elastic has 4 integer data types: long, integer, short, byte + // see: https://www.elastic.co/guide/en/elasticsearch/reference/current/number.html + if ( + diff < maxBars && + esTypes.every((esType) => + [ + ES_FIELD_TYPES.INTEGER, + ES_FIELD_TYPES.LONG, + ES_FIELD_TYPES.SHORT, + ES_FIELD_TYPES.BYTE, + ].includes(esType) + ) + ) { + return 1; + } + const lowerPower = Math.pow(10, Math.floor(Math.log10(exactInterval))); const autoBuckets = diff / lowerPower; if (autoBuckets > maxBars) { @@ -103,6 +121,7 @@ export const calculateHistogramInterval = ({ maxBucketsUserInput, intervalBase, values, + esTypes, }: CalculateHistogramIntervalParams) => { const isAuto = isAutoInterval(interval); let calculatedInterval = isAuto ? 0 : parseFloat(interval); @@ -119,8 +138,10 @@ export const calculateHistogramInterval = ({ calculatedInterval = isAuto ? calculateAutoInterval( diff, + // Mind maxBucketsUserInput can be an empty string, hence we need to ensure it here - Math.min(maxBucketsUiSettings, maxBucketsUserInput || maxBucketsUiSettings) + Math.min(maxBucketsUiSettings, maxBucketsUserInput || maxBucketsUiSettings), + esTypes ) : calculateForGivenInterval(diff, calculatedInterval, maxBucketsUiSettings); }