[Lens] Fix overall_min, overall_max and overall_average functions (#105276)

* [Lens] Fix overall metrics

* Style comments

src/plugins/expressions/common/expression_functions/specs/overall_metric.ts

@@ -125,7 +125,7 @@ export const overallMetric: ExpressionFunctionOverallMetric = {
     const valueCounter: Partial<Record<string, number>> = {};
     input.rows.forEach((row) => {
       const bucketIdentifier = getBucketIdentifier(row, by);
-      const accumulatorValue = accumulators[bucketIdentifier] ?? 0;
+      const accumulatorValue = accumulators[bucketIdentifier];
 
       const currentValue = row[inputColumnId];
       if (currentValue != null) {
@@ -135,22 +135,35 @@ export const overallMetric: ExpressionFunctionOverallMetric = {
             valueCounter[bucketIdentifier] =
               (valueCounter[bucketIdentifier] ?? 0) + currentNumberValues.length;
           case 'sum':
-            accumulators[bucketIdentifier] =
-              accumulatorValue + currentNumberValues.reduce((a, b) => a + b, 0);
+            accumulators[bucketIdentifier] = currentNumberValues.reduce(
+              (a, b) => a + b,
+              accumulatorValue || 0
+            );
             break;
           case 'min':
-            accumulators[bucketIdentifier] = Math.min(accumulatorValue, ...currentNumberValues);
+            if (typeof accumulatorValue !== 'undefined') {
+              accumulators[bucketIdentifier] = Math.min(accumulatorValue, ...currentNumberValues);
+            } else {
+              accumulators[bucketIdentifier] = Math.min(...currentNumberValues);
+            }
             break;
           case 'max':
-            accumulators[bucketIdentifier] = Math.max(accumulatorValue, ...currentNumberValues);
+            if (typeof accumulatorValue !== 'undefined') {
+              accumulators[bucketIdentifier] = Math.max(accumulatorValue, ...currentNumberValues);
+            } else {
+              accumulators[bucketIdentifier] = Math.max(...currentNumberValues);
+            }
             break;
         }
       }
     });
     if (metric === 'average') {
       Object.keys(accumulators).forEach((bucketIdentifier) => {
-        accumulators[bucketIdentifier] =
-          accumulators[bucketIdentifier]! / valueCounter[bucketIdentifier]!;
+        const accumulatorValue = accumulators[bucketIdentifier];
+        const valueCount = valueCounter[bucketIdentifier];
+        if (typeof accumulatorValue !== 'undefined' && typeof valueCount !== 'undefined') {
+          accumulators[bucketIdentifier] = accumulatorValue / valueCount;
+        }
       });
     }
     return {

src/plugins/expressions/common/expression_functions/specs/tests/overall_metric.test.ts

@@ -16,12 +16,12 @@ describe('interpreter/functions#overall_metric', () => {
   const runFn = (input: Datatable, args: OverallMetricArgs) =>
     fn(input, args, {} as ExecutionContext) as Datatable;
 
-  it('calculates overall sum', () => {
+  it('ignores null or undefined with sum', () => {
     const result = runFn(
       {
         type: 'datatable',
         columns: [{ id: 'val', name: 'val', meta: { type: 'number' } }],
-        rows: [{ val: 5 }, { val: 7 }, { val: 3 }, { val: 2 }],
+        rows: [{ val: undefined }, { val: 7 }, { val: 3 }, { val: 2 }],
       },
       { inputColumnId: 'val', outputColumnId: 'output', metric: 'sum' }
     );
@@ -30,10 +30,10 @@ describe('interpreter/functions#overall_metric', () => {
       name: 'output',
       meta: { type: 'number' },
     });
-    expect(result.rows.map((row) => row.output)).toEqual([17, 17, 17, 17]);
+    expect(result.rows.map((row) => row.output)).toEqual([12, 12, 12, 12]);
   });
 
-  it('ignores null or undefined', () => {
+  it('ignores null or undefined with average', () => {
     const result = runFn(
       {
         type: 'datatable',
@@ -50,6 +50,40 @@ describe('interpreter/functions#overall_metric', () => {
     expect(result.rows.map((row) => row.output)).toEqual([3, 3, 3, 3, 3]);
   });
 
+  it('ignores null or undefined with min', () => {
+    const result = runFn(
+      {
+        type: 'datatable',
+        columns: [{ id: 'val', name: 'val', meta: { type: 'number' } }],
+        rows: [{}, { val: null }, { val: undefined }, { val: 1 }, { val: 5 }],
+      },
+      { inputColumnId: 'val', outputColumnId: 'output', metric: 'min' }
+    );
+    expect(result.columns).toContainEqual({
+      id: 'output',
+      name: 'output',
+      meta: { type: 'number' },
+    });
+    expect(result.rows.map((row) => row.output)).toEqual([1, 1, 1, 1, 1]);
+  });
+
+  it('ignores null or undefined with max', () => {
+    const result = runFn(
+      {
+        type: 'datatable',
+        columns: [{ id: 'val', name: 'val', meta: { type: 'number' } }],
+        rows: [{}, { val: null }, { val: undefined }, { val: -1 }, { val: -5 }],
+      },
+      { inputColumnId: 'val', outputColumnId: 'output', metric: 'max' }
+    );
+    expect(result.columns).toContainEqual({
+      id: 'output',
+      name: 'output',
+      meta: { type: 'number' },
+    });
+    expect(result.rows.map((row) => row.output)).toEqual([-1, -1, -1, -1, -1]);
+  });
+
   it('calculates overall sum for multiple series', () => {
     const result = runFn(
       {
@@ -103,18 +137,9 @@ describe('interpreter/functions#overall_metric', () => {
           { val: 8, split: 'B' },
         ],
       },
-      { inputColumnId: 'val', outputColumnId: 'output', by: ['split'], metric: 'sum' }
+      { inputColumnId: 'val', outputColumnId: 'output', by: ['split'], metric: 'min' }
     );
-    expect(result.rows.map((row) => row.output)).toEqual([
-      1 + 4 + 6,
-      2 + 7 + 8,
-      3 + 5,
-      1 + 4 + 6,
-      3 + 5,
-      1 + 4 + 6,
-      2 + 7 + 8,
-      2 + 7 + 8,
-    ]);
+    expect(result.rows.map((row) => row.output)).toEqual([1, 2, 3, 1, 3, 1, 2, 2]);
   });
 
   it('treats null like undefined and empty string for split columns', () => {
@@ -162,6 +187,24 @@ describe('interpreter/functions#overall_metric', () => {
       metric: 'max',
     });
     expect(result2.rows.map((row) => row.output)).toEqual([6, 8, 9, 6, 9, 6, 9, 8, 9]);
+
+    const result3 = runFn(table, {
+      inputColumnId: 'val',
+      outputColumnId: 'output',
+      by: ['split'],
+      metric: 'average',
+    });
+    expect(result3.rows.map((row) => row.output)).toEqual([
+      (1 + 4 + 6) / 3,
+      (2 + 8) / 2,
+      (3 + 5 + 7 + 9) / 4,
+      (1 + 4 + 6) / 3,
+      (3 + 5 + 7 + 9) / 4,
+      (1 + 4 + 6) / 3,
+      (3 + 5 + 7 + 9) / 4,
+      (2 + 8) / 2,
+      (3 + 5 + 7 + 9) / 4,
+    ]);
   });
 
   it('handles array values', () => {

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/overall_metric.test.ts

@@ -0,0 +1,50 @@
+/*
+ * 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; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import { createMockedIndexPattern } from '../../../mocks';
+import type { IndexPatternLayer } from '../../../types';
+import {
+  overallAverageOperation,
+  overallMaxOperation,
+  overallMinOperation,
+  overallSumOperation,
+} from '../index';
+
+describe('overall_metric', () => {
+  const indexPattern = createMockedIndexPattern();
+  let layer: IndexPatternLayer;
+
+  beforeEach(() => {
+    layer = {
+      indexPatternId: '1',
+      columnOrder: [],
+      columns: {},
+    };
+  });
+
+  describe('buildColumn', () => {
+    it('should assign the right operationType', () => {
+      const args = {
+        layer,
+        indexPattern,
+        referenceIds: ['a'],
+      };
+      expect(overallAverageOperation.buildColumn(args)).toEqual(
+        expect.objectContaining({ operationType: 'overall_average' })
+      );
+      expect(overallMaxOperation.buildColumn(args)).toEqual(
+        expect.objectContaining({ operationType: 'overall_max' })
+      );
+      expect(overallMinOperation.buildColumn(args)).toEqual(
+        expect.objectContaining({ operationType: 'overall_min' })
+      );
+      expect(overallSumOperation.buildColumn(args)).toEqual(
+        expect.objectContaining({ operationType: 'overall_sum' })
+      );
+    });
+  });
+});

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/overall_metric.tsx

@@ -6,9 +6,12 @@
  */
 
 import { i18n } from '@kbn/i18n';
-import { FormattedIndexPatternColumn, ReferenceBasedIndexPatternColumn } from '../column_types';
+import type {
+  FormattedIndexPatternColumn,
+  ReferenceBasedIndexPatternColumn,
+} from '../column_types';
 import { optionallHistogramBasedOperationToExpression } from './utils';
-import { OperationDefinition } from '..';
+import type { OperationDefinition } from '..';
 import { getFormatFromPreviousColumn } from '../helpers';
 
 type OverallMetricIndexPatternColumn<T extends string> = FormattedIndexPatternColumn &
@@ -75,7 +78,7 @@ function buildOverallMetricOperation<T extends OverallMetricIndexPatternColumn<s
             : undefined
         ),
         dataType: 'number',
-        operationType: 'overall_sum',
+        operationType: `overall_${metric}`,
         isBucketed: false,
         scale: 'ratio',
         references: referenceIds,
@@ -154,7 +157,7 @@ Other dimensions breaking down the data like top values or filter are treated as
 If no date histograms or interval functions are used in the current chart, \`overall_min\` is calculating the minimum over all dimensions no matter the used function
 
 Example: Percentage of range
-\`(sum(bytes) - overall_min(sum(bytes)) / (overall_max(bytes) - overall_min(bytes))\`
+\`(sum(bytes) - overall_min(sum(bytes)) / (overall_max(sum(bytes)) - overall_min(sum(bytes)))\`
       `,
   }),
 });
@@ -185,7 +188,7 @@ Other dimensions breaking down the data like top values or filter are treated as
 If no date histograms or interval functions are used in the current chart, \`overall_max\` is calculating the maximum over all dimensions no matter the used function
 
 Example: Percentage of range
-\`(sum(bytes) - overall_min(sum(bytes)) / (overall_max(bytes) - overall_min(bytes))\`
+\`(sum(bytes) - overall_min(sum(bytes))) / (overall_max(sum(bytes)) - overall_min(sum(bytes)))\`
       `,
   }),
 });