[Lens] Do not add math columns for pass-through operations (#102656)

This commit is contained in:
Joe Reuter 2021-06-23 13:48:48 +02:00 committed by GitHub
parent 2ab5d6bc46
commit b652ef677f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 85 additions and 73 deletions

View file

@ -7,11 +7,12 @@
import { i18n } from '@kbn/i18n';
import type { ExpressionFunctionAST } from '@kbn/interpreter/common';
import memoizeOne from 'memoize-one';
import type { TimeScaleUnit } from '../../../time_scale';
import type { IndexPattern, IndexPatternLayer } from '../../../types';
import { adjustTimeScaleLabelSuffix } from '../../time_scale_utils';
import type { ReferenceBasedIndexPatternColumn } from '../column_types';
import { isColumnValidAsReference } from '../../layer_helpers';
import { getManagedColumnsFrom, isColumnValidAsReference } from '../../layer_helpers';
import { operationDefinitionMap } from '..';
export const buildLabelFunction = (ofName: (name?: string) => string) => (
@ -45,6 +46,23 @@ export function checkForDateHistogram(layer: IndexPatternLayer, name: string) {
];
}
const getFullyManagedColumnIds = memoizeOne((layer: IndexPatternLayer) => {
const managedColumnIds = new Set<string>();
Object.entries(layer.columns).forEach(([id, column]) => {
if (
'references' in column &&
operationDefinitionMap[column.operationType].input === 'managedReference'
) {
managedColumnIds.add(id);
const managedColumns = getManagedColumnsFrom(id, layer.columns);
managedColumns.map(([managedId]) => {
managedColumnIds.add(managedId);
});
}
});
return managedColumnIds;
});
export function checkReferences(layer: IndexPatternLayer, columnId: string) {
const column = layer.columns[columnId] as ReferenceBasedIndexPatternColumn;
@ -72,7 +90,8 @@ export function checkReferences(layer: IndexPatternLayer, columnId: string) {
column: referenceColumn,
});
if (!isValid) {
// do not enforce column validity if current column is part of managed subtree
if (!isValid && !getFullyManagedColumnIds(layer).has(columnId)) {
errors.push(
i18n.translate('xpack.lens.indexPattern.invalidReferenceConfiguration', {
defaultMessage: 'Dimension "{dimensionLabel}" is configured incorrectly',

View file

@ -413,13 +413,13 @@ describe('formula', () => {
).newLayer
).toEqual({
...layer,
columnOrder: ['col1X0', 'col1X1', 'col1'],
columnOrder: ['col1X0', 'col1'],
columns: {
...layer.columns,
col1: {
...currentColumn,
label: 'average(bytes)',
references: ['col1X1'],
references: ['col1X0'],
params: {
...currentColumn.params,
formula: 'average(bytes)',
@ -436,18 +436,6 @@ describe('formula', () => {
sourceField: 'bytes',
timeScale: false,
},
col1X1: {
customLabel: true,
dataType: 'number',
isBucketed: false,
label: 'Part of average(bytes)',
operationType: 'math',
params: {
tinymathAst: 'col1X0',
},
references: ['col1X0'],
scale: 'ratio',
},
},
});
});
@ -568,8 +556,8 @@ describe('formula', () => {
).locations
).toEqual({
col1X0: { min: 15, max: 29 },
col1X2: { min: 0, max: 41 },
col1X3: { min: 42, max: 50 },
col1X1: { min: 0, max: 41 },
col1X2: { min: 42, max: 50 },
});
});
});

View file

@ -123,17 +123,20 @@ function extractColumns(
if (nodeOperation.input === 'fullReference') {
const [referencedOp] = functions;
const consumedParam = parseNode(referencedOp);
const hasActualMathContent = typeof consumedParam !== 'string';
const subNodeVariables = consumedParam ? findVariables(consumedParam) : [];
const mathColumn = mathOperation.buildColumn({
layer,
indexPattern,
});
mathColumn.references = subNodeVariables.map(({ value }) => value);
mathColumn.params.tinymathAst = consumedParam!;
columns.push({ column: mathColumn });
mathColumn.customLabel = true;
mathColumn.label = label;
if (hasActualMathContent) {
const subNodeVariables = consumedParam ? findVariables(consumedParam) : [];
const mathColumn = mathOperation.buildColumn({
layer,
indexPattern,
});
mathColumn.references = subNodeVariables.map(({ value }) => value);
mathColumn.params.tinymathAst = consumedParam!;
columns.push({ column: mathColumn });
mathColumn.customLabel = true;
mathColumn.label = label;
}
const mappedParams = getOperationParams(nodeOperation, namedArguments || []);
const newCol = (nodeOperation as OperationDefinition<
@ -143,7 +146,11 @@ function extractColumns(
{
layer,
indexPattern,
referenceIds: [getManagedId(idPrefix, columns.length - 1)],
referenceIds: [
hasActualMathContent
? getManagedId(idPrefix, columns.length - 1)
: (consumedParam as string),
],
},
mappedParams
);
@ -160,16 +167,19 @@ function extractColumns(
if (root === undefined) {
return [];
}
const variables = findVariables(root);
const mathColumn = mathOperation.buildColumn({
layer,
indexPattern,
});
mathColumn.references = variables.map(({ value }) => value);
mathColumn.params.tinymathAst = root!;
mathColumn.customLabel = true;
mathColumn.label = label;
columns.push({ column: mathColumn });
const topLevelMath = typeof root !== 'string';
if (topLevelMath) {
const variables = findVariables(root);
const mathColumn = mathOperation.buildColumn({
layer,
indexPattern,
});
mathColumn.references = variables.map(({ value }) => value);
mathColumn.params.tinymathAst = root!;
mathColumn.customLabel = true;
mathColumn.label = label;
columns.push({ column: mathColumn });
}
return columns;
}

View file

@ -25,6 +25,7 @@ import { documentField } from '../document_field';
import { getFieldByNameFactory } from '../pure_helpers';
import { generateId } from '../../id_generator';
import { createMockedFullReference, createMockedManagedReference } from './mocks';
import { TinymathAST } from 'packages/kbn-tinymath';
jest.mock('../operations');
jest.mock('../../id_generator');
@ -105,28 +106,34 @@ describe('state_helpers', () => {
const source = {
dataType: 'number' as const,
isBucketed: false,
label: 'moving_average(sum(bytes), window=5)',
label: '5 + moving_average(sum(bytes), window=5)',
operationType: 'formula' as const,
params: {
formula: 'moving_average(sum(bytes), window=5)',
formula: '5 + moving_average(sum(bytes), window=5)',
isFormulaBroken: false,
},
references: ['formulaX1'],
references: ['formulaX2'],
};
const math = {
customLabel: true,
dataType: 'number' as const,
isBucketed: false,
label: 'Part of moving_average(sum(bytes), window=5)',
operationType: 'math' as const,
params: { tinymathAst: 'formulaX2' },
references: ['formulaX2'],
label: 'Part of 5 + moving_average(sum(bytes), window=5)',
references: ['formulaX1'],
params: {
tinymathAst: {
type: 'function',
name: 'add',
args: [5, 'formulaX1'],
} as TinymathAST,
},
};
const sum = {
customLabel: true,
dataType: 'number' as const,
isBucketed: false,
label: 'Part of moving_average(sum(bytes), window=5)',
label: 'Part of 5 + moving_average(sum(bytes), window=5)',
operationType: 'sum' as const,
scale: 'ratio' as const,
sourceField: 'bytes',
@ -135,7 +142,7 @@ describe('state_helpers', () => {
customLabel: true,
dataType: 'number' as const,
isBucketed: false,
label: 'Part of moving_average(sum(bytes), window=5)',
label: 'Part of 5 + moving_average(sum(bytes), window=5)',
operationType: 'moving_average' as const,
params: { window: 5 },
references: ['formulaX0'],
@ -148,14 +155,8 @@ describe('state_helpers', () => {
columns: {
source,
formulaX0: sum,
formulaX1: math,
formulaX2: movingAvg,
formulaX3: {
...math,
label: 'Part of moving_average(sum(bytes), window=5)',
references: ['formulaX2'],
params: { tinymathAst: 'formulaX2' },
},
formulaX1: movingAvg,
formulaX2: math,
},
},
targetId: 'copy',
@ -171,40 +172,34 @@ describe('state_helpers', () => {
'formulaX0',
'formulaX1',
'formulaX2',
'formulaX3',
'copyX0',
'copyX1',
'copyX2',
'copyX3',
'copy',
],
columns: {
source,
formulaX0: sum,
formulaX1: math,
formulaX2: movingAvg,
formulaX3: {
...math,
references: ['formulaX2'],
params: { tinymathAst: 'formulaX2' },
},
copy: expect.objectContaining({ ...source, references: ['copyX3'] }),
formulaX1: movingAvg,
formulaX2: math,
copy: expect.objectContaining({ ...source, references: ['copyX2'] }),
copyX0: expect.objectContaining({
...sum,
}),
copyX1: expect.objectContaining({
...math,
...movingAvg,
references: ['copyX0'],
params: { tinymathAst: 'copyX0' },
}),
copyX2: expect.objectContaining({
...movingAvg,
references: ['copyX1'],
}),
copyX3: expect.objectContaining({
...math,
references: ['copyX2'],
params: { tinymathAst: 'copyX2' },
references: ['copyX1'],
params: {
tinymathAst: expect.objectContaining({
type: 'function',
name: 'add',
args: [5, 'copyX1'],
} as TinymathAST),
},
}),
},
});