'Auto' interval must be correctly calculated for natural numbers (ela…

…stic#77995) * 'Auto' interval must be correctly calculated for natural numbers * fix ts error * fix PR comments Co-authored-by: Elastic Machine <[email protected]>
alexwizp · Sep 23, 2020 · 63603ae · 63603ae
1 parent 99bc56d
commit 63603ae
Show file tree

Hide file tree

Showing 3 changed files with 117 additions and 4 deletions.
diff --git 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
@@ -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
@@ -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);
     }