From 9c2a41da2f91987730bf0c2cb07837856e41e472 Mon Sep 17 00:00:00 2001 From: Andrew Tate Date: Mon, 9 May 2022 14:09:21 -0500 Subject: [PATCH 01/16] first attempt at grouping percentage aggs --- .../rename_columns/rename_columns.ts | 2 +- .../expressions/rename_columns/types.ts | 2 +- .../indexpattern_datasource/to_expression.ts | 68 ++++++++++++++++++- 3 files changed, 67 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.ts b/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.ts index d425d5c80d18d..e5440c2a09675 100644 --- a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.ts +++ b/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.ts @@ -9,7 +9,7 @@ import { i18n } from '@kbn/i18n'; import type { RenameColumnsExpressionFunction } from './types'; export const renameColumns: RenameColumnsExpressionFunction = { - name: 'lens_rename_columns', + name: 'lens_restore_column_ids', type: 'datatable', help: i18n.translate('xpack.lens.functions.renameColumns.help', { defaultMessage: 'A helper to rename the columns of a datatable', diff --git a/x-pack/plugins/lens/common/expressions/rename_columns/types.ts b/x-pack/plugins/lens/common/expressions/rename_columns/types.ts index 6edfda41cc62f..7c53ade527c1a 100644 --- a/x-pack/plugins/lens/common/expressions/rename_columns/types.ts +++ b/x-pack/plugins/lens/common/expressions/rename_columns/types.ts @@ -13,7 +13,7 @@ export type OriginalColumn = { id: string; label: string } & ( ); export type RenameColumnsExpressionFunction = ExpressionFunctionDefinition< - 'lens_rename_columns', + 'lens_restore_column_ids', Datatable, { idMap: string; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts index aee1abd34e7cb..55f47c97bc4e5 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts @@ -11,14 +11,16 @@ import { AggFunctionsMapping, EsaggsExpressionFunctionDefinition, IndexPatternLoadExpressionFunctionDefinition, + METRIC_TYPES, } from '@kbn/data-plugin/public'; -import { queryToAst } from '@kbn/data-plugin/common'; +import { AggExpressionFunctionArgs, queryToAst } from '@kbn/data-plugin/common'; import { buildExpression, buildExpressionFunction, ExpressionAstExpression, ExpressionAstExpressionBuilder, ExpressionAstFunction, + ExpressionAstFunctionBuilder, } from '@kbn/expressions-plugin/public'; import { GenericIndexPatternColumn } from './indexpattern'; import { operationDefinitionMap } from './operations'; @@ -95,7 +97,7 @@ function getExpressionForLayer( ); if (referenceEntries.length || esAggEntries.length) { - const aggs: ExpressionAstExpressionBuilder[] = []; + let aggs: ExpressionAstExpressionBuilder[] = []; const expressions: ExpressionAstFunction[] = []; sortedReferences(referenceEntries).forEach((colId) => { @@ -177,6 +179,66 @@ function getExpressionForLayer( }, }; }, {} as Record); + + const percentileExpressionsByArgs: Record = {}; + + aggs.forEach((expressionBuilder) => { + const { + functions: [fnBuilder], + } = expressionBuilder; + if (fnBuilder.name === 'aggSinglePercentile') { + const field = fnBuilder.getArgument('field')?.[0]; + if (!(field in percentileExpressionsByArgs)) { + percentileExpressionsByArgs[field] = []; + } + + percentileExpressionsByArgs[field].push(expressionBuilder); + } + }); + + Object.values(percentileExpressionsByArgs).forEach(([first, ...rest]) => { + if (!rest.length) { + // don't need to optimize if there's only one + return; + } + + const { + functions: [firstFnBuilder], + } = first; + + const aggPercentilesConfig: AggExpressionFunctionArgs = { + id: 'foo-foo', + enabled: firstFnBuilder.getArgument('enabled')?.[0], + schema: firstFnBuilder.getArgument('schema')?.[0], + field: firstFnBuilder.getArgument('field')?.[0], + percents: firstFnBuilder.getArgument('percentile'), + // time shift is added to wrapping aggFilteredMetric if filter is set + timeShift: firstFnBuilder.getArgument('timeShift')?.[0], + }; + + const siblingPercentiles = rest.map( + ({ functions: [fnBuilder] }) => fnBuilder.getArgument('percentile')?.[0] + ) as number[]; + + aggPercentilesConfig.percents = [ + ...(aggPercentilesConfig.percents ?? []), + ...siblingPercentiles, + ]; + + const multiPercentilesAst = buildExpressionFunction( + 'aggPercentiles', + aggPercentilesConfig + ).toAst(); + + aggs = [ + ...aggs.filter((agg) => ![first, ...rest].includes(agg)), + buildExpression({ + type: 'expression', + chain: [multiPercentilesAst], + }), + ]; + }); + const columnsWithFormatters = columnEntries.filter( ([, col]) => (isColumnOfType('range', col) && col.params?.parentFormat) || @@ -291,7 +353,7 @@ function getExpressionForLayer( }).toAst(), { type: 'function', - function: 'lens_rename_columns', + function: 'lens_restore_column_ids', arguments: { idMap: [JSON.stringify(idMap)], }, From 7b264feed5637256db07580c9515bd439c34afc6 Mon Sep 17 00:00:00 2001 From: Andrew Tate Date: Mon, 9 May 2022 14:24:57 -0500 Subject: [PATCH 02/16] include only columns with corresponding esAggs fns in idMap rename id map and restoration function for greater clarity --- .../rename_columns/rename_columns.ts | 2 +- .../expressions/rename_columns/types.ts | 2 +- .../indexpattern_datasource/to_expression.ts | 30 ++++++++----------- 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.ts b/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.ts index e5440c2a09675..e03ae52a04f4a 100644 --- a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.ts +++ b/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.ts @@ -9,7 +9,7 @@ import { i18n } from '@kbn/i18n'; import type { RenameColumnsExpressionFunction } from './types'; export const renameColumns: RenameColumnsExpressionFunction = { - name: 'lens_restore_column_ids', + name: 'lens_restore_original_column_ids', type: 'datatable', help: i18n.translate('xpack.lens.functions.renameColumns.help', { defaultMessage: 'A helper to rename the columns of a datatable', diff --git a/x-pack/plugins/lens/common/expressions/rename_columns/types.ts b/x-pack/plugins/lens/common/expressions/rename_columns/types.ts index 7c53ade527c1a..243662b65842a 100644 --- a/x-pack/plugins/lens/common/expressions/rename_columns/types.ts +++ b/x-pack/plugins/lens/common/expressions/rename_columns/types.ts @@ -13,7 +13,7 @@ export type OriginalColumn = { id: string; label: string } & ( ); export type RenameColumnsExpressionFunction = ExpressionFunctionDefinition< - 'lens_restore_column_ids', + 'lens_restore_original_column_ids', Datatable, { idMap: string; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts index 55f47c97bc4e5..319f2cc649529 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts @@ -109,6 +109,7 @@ function getExpressionForLayer( }); const orderedColumnIds = esAggEntries.map(([colId]) => colId); + const esAggsToOriginalColumnIdMap: Record = {}; esAggEntries.forEach(([colId, col], index) => { const def = operationDefinitionMap[col.operationType]; if (def.input !== 'fullReference' && def.input !== 'managedReference') { @@ -147,6 +148,15 @@ function getExpressionForLayer( chain: [aggAst], }) ); + + const esAggsId = window.ELASTIC_LENS_DELAY_SECONDS + ? `col-${index + (col.isBucketed ? 0 : 1)}-${index}` + : `col-${index}-${index}`; + + esAggsToOriginalColumnIdMap[esAggsId] = { + ...col, + id: colId, + }; } }); @@ -166,20 +176,6 @@ function getExpressionForLayer( ); } - const idMap = esAggEntries.reduce((currentIdMap, [colId, column], index) => { - const esAggsId = window.ELASTIC_LENS_DELAY_SECONDS - ? `col-${index + (column.isBucketed ? 0 : 1)}-${index}` - : `col-${index}-${index}`; - - return { - ...currentIdMap, - [esAggsId]: { - ...column, - id: colId, - }, - }; - }, {} as Record); - const percentileExpressionsByArgs: Record = {}; aggs.forEach((expressionBuilder) => { @@ -217,7 +213,7 @@ function getExpressionForLayer( }; const siblingPercentiles = rest.map( - ({ functions: [fnBuilder] }) => fnBuilder.getArgument('percentile')?.[0] + ({ functions: [fnBuilder] }) => fnBuilder.getArgument('percentile')![0] ) as number[]; aggPercentilesConfig.percents = [ @@ -353,9 +349,9 @@ function getExpressionForLayer( }).toAst(), { type: 'function', - function: 'lens_restore_column_ids', + function: 'lens_restore_original_column_ids', arguments: { - idMap: [JSON.stringify(idMap)], + idMap: [JSON.stringify(esAggsToOriginalColumnIdMap)], }, }, ...expressions, From 6fa190ae1d32550f002419910d8bf7e7a902f175 Mon Sep 17 00:00:00 2001 From: Andrew Tate Date: Mon, 9 May 2022 16:46:39 -0500 Subject: [PATCH 03/16] make restore column id algo independent of agg order (introduces infinite loop that need troubleshooting) --- .../rename_columns/rename_columns_fn.ts | 21 +++++-- .../indexpattern_datasource/to_expression.ts | 57 ++++++++++++++----- 2 files changed, 60 insertions(+), 18 deletions(-) diff --git a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns_fn.ts b/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns_fn.ts index 3cf4293ffa9f2..2e510c6b0b110 100644 --- a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns_fn.ts +++ b/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns_fn.ts @@ -27,17 +27,30 @@ export const renameColumnFn: RenameColumnsExpressionFunction['fn'] = ( ) => { const idMap = JSON.parse(encodedIdMap) as Record; + const columnIdMap = Object.keys(idMap).reduce((acc, esAggsId) => { + const columnId = data.columns.map((column) => column.id).find((id) => id.includes(esAggsId)); + + if (columnId === undefined) { + throw new Error(`Could not find column corresponding to aggregation with id ${esAggsId}`); + } + + return { + ...acc, + [columnId]: idMap[esAggsId], + }; + }, {} as Record); + return { ...data, rows: data.rows.map((row) => { const mappedRow: Record = {}; - Object.entries(idMap).forEach(([fromId, toId]) => { + Object.entries(columnIdMap).forEach(([fromId, toId]) => { mappedRow[toId.id] = row[fromId]; }); Object.entries(row).forEach(([id, value]) => { - if (id in idMap) { - mappedRow[idMap[id].id] = value; + if (id in columnIdMap) { + mappedRow[columnIdMap[id].id] = value; } else { mappedRow[id] = value; } @@ -46,7 +59,7 @@ export const renameColumnFn: RenameColumnsExpressionFunction['fn'] = ( return mappedRow; }), columns: data.columns.map((column) => { - const mappedItem = idMap[column.id]; + const mappedItem = columnIdMap[column.id]; if (!mappedItem) { return column; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts index 319f2cc649529..8633accc59408 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts @@ -20,7 +20,6 @@ import { ExpressionAstExpression, ExpressionAstExpressionBuilder, ExpressionAstFunction, - ExpressionAstFunctionBuilder, } from '@kbn/expressions-plugin/public'; import { GenericIndexPatternColumn } from './indexpattern'; import { operationDefinitionMap } from './operations'; @@ -108,15 +107,28 @@ function getExpressionForLayer( } }); + function makeid(length = 10) { + let result = ''; + const characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'; + const charactersLength = characters.length; + for (let i = 0; i < length; i++) { + result += characters.charAt(Math.floor(Math.random() * charactersLength)); + } + return result; + } + const orderedColumnIds = esAggEntries.map(([colId]) => colId); const esAggsToOriginalColumnIdMap: Record = {}; + const expressionBuilderToEsAggsIdMap: Map = new Map(); esAggEntries.forEach(([colId, col], index) => { const def = operationDefinitionMap[col.operationType]; if (def.input !== 'fullReference' && def.input !== 'managedReference') { + const esAggsId = makeid(); + const wrapInFilter = Boolean(def.filterable && col.filter); let aggAst = def.toEsAggsFn( col, - wrapInFilter ? `${index}-metric` : String(index), + wrapInFilter ? `${esAggsId}-metric` : esAggsId, indexPattern, layer, uiSettings, @@ -142,21 +154,19 @@ function getExpressionForLayer( } ).toAst(); } - aggs.push( - buildExpression({ - type: 'expression', - chain: [aggAst], - }) - ); - const esAggsId = window.ELASTIC_LENS_DELAY_SECONDS - ? `col-${index + (col.isBucketed ? 0 : 1)}-${index}` - : `col-${index}-${index}`; + const expressionBuilder = buildExpression({ + type: 'expression', + chain: [aggAst], + }); + aggs.push(expressionBuilder); esAggsToOriginalColumnIdMap[esAggsId] = { ...col, id: colId, }; + + expressionBuilderToEsAggsIdMap.set(expressionBuilder, esAggsId); } }); @@ -192,7 +202,9 @@ function getExpressionForLayer( } }); - Object.values(percentileExpressionsByArgs).forEach(([first, ...rest]) => { + Object.values(percentileExpressionsByArgs).forEach((expressionBuilders) => { + const [first, ...rest] = expressionBuilders; + if (!rest.length) { // don't need to optimize if there's only one return; @@ -202,8 +214,10 @@ function getExpressionForLayer( functions: [firstFnBuilder], } = first; + const esAggsColumnId = makeid(); + const aggPercentilesConfig: AggExpressionFunctionArgs = { - id: 'foo-foo', + id: esAggsColumnId, enabled: firstFnBuilder.getArgument('enabled')?.[0], schema: firstFnBuilder.getArgument('schema')?.[0], field: firstFnBuilder.getArgument('field')?.[0], @@ -227,12 +241,27 @@ function getExpressionForLayer( ).toAst(); aggs = [ - ...aggs.filter((agg) => ![first, ...rest].includes(agg)), + ...aggs.filter((aggBuilder) => ![first, ...rest].includes(aggBuilder)), buildExpression({ type: 'expression', chain: [multiPercentilesAst], }), ]; + + expressionBuilders.forEach((expressionBuilder) => { + const currentEsAggsId = expressionBuilderToEsAggsIdMap.get(expressionBuilder); + if (currentEsAggsId === undefined) { + throw new Error('Could not find current column ID for percentile agg expression builder'); + } + // esAggs appends the percent number to the agg id. We're anticipating that here. + const newEsAggsId = `${esAggsColumnId}.${ + expressionBuilder.functions[0].getArgument('percentile')![0] + }`; + + esAggsToOriginalColumnIdMap[newEsAggsId] = esAggsToOriginalColumnIdMap[currentEsAggsId]; + + delete esAggsToOriginalColumnIdMap[currentEsAggsId]; + }); }); const columnsWithFormatters = columnEntries.filter( From 51b4cce31d49ad6b4fcfa40e32654c39d67b4659 Mon Sep 17 00:00:00 2001 From: Andrew Tate Date: Tue, 10 May 2022 13:11:33 -0500 Subject: [PATCH 04/16] update ID map deterministically following optimization --- .../rename_columns/rename_columns_fn.ts | 21 +---- .../indexpattern_datasource/to_expression.ts | 85 ++++++++++++++----- 2 files changed, 68 insertions(+), 38 deletions(-) diff --git a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns_fn.ts b/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns_fn.ts index 2e510c6b0b110..3cf4293ffa9f2 100644 --- a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns_fn.ts +++ b/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns_fn.ts @@ -27,30 +27,17 @@ export const renameColumnFn: RenameColumnsExpressionFunction['fn'] = ( ) => { const idMap = JSON.parse(encodedIdMap) as Record; - const columnIdMap = Object.keys(idMap).reduce((acc, esAggsId) => { - const columnId = data.columns.map((column) => column.id).find((id) => id.includes(esAggsId)); - - if (columnId === undefined) { - throw new Error(`Could not find column corresponding to aggregation with id ${esAggsId}`); - } - - return { - ...acc, - [columnId]: idMap[esAggsId], - }; - }, {} as Record); - return { ...data, rows: data.rows.map((row) => { const mappedRow: Record = {}; - Object.entries(columnIdMap).forEach(([fromId, toId]) => { + Object.entries(idMap).forEach(([fromId, toId]) => { mappedRow[toId.id] = row[fromId]; }); Object.entries(row).forEach(([id, value]) => { - if (id in columnIdMap) { - mappedRow[columnIdMap[id].id] = value; + if (id in idMap) { + mappedRow[idMap[id].id] = value; } else { mappedRow[id] = value; } @@ -59,7 +46,7 @@ export const renameColumnFn: RenameColumnsExpressionFunction['fn'] = ( return mappedRow; }), columns: data.columns.map((column) => { - const mappedItem = columnIdMap[column.id]; + const mappedItem = idMap[column.id]; if (!mappedItem) { return column; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts index 8633accc59408..4ab3f9e66c037 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts @@ -107,28 +107,18 @@ function getExpressionForLayer( } }); - function makeid(length = 10) { - let result = ''; - const characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'; - const charactersLength = characters.length; - for (let i = 0; i < length; i++) { - result += characters.charAt(Math.floor(Math.random() * charactersLength)); - } - return result; - } - const orderedColumnIds = esAggEntries.map(([colId]) => colId); - const esAggsToOriginalColumnIdMap: Record = {}; + const esAggsIdToOriginalColumn: Record = {}; const expressionBuilderToEsAggsIdMap: Map = new Map(); esAggEntries.forEach(([colId, col], index) => { const def = operationDefinitionMap[col.operationType]; if (def.input !== 'fullReference' && def.input !== 'managedReference') { - const esAggsId = makeid(); + const aggId = String(index); const wrapInFilter = Boolean(def.filterable && col.filter); let aggAst = def.toEsAggsFn( col, - wrapInFilter ? `${esAggsId}-metric` : esAggsId, + wrapInFilter ? `${aggId}-metric` : aggId, indexPattern, layer, uiSettings, @@ -161,7 +151,11 @@ function getExpressionForLayer( }); aggs.push(expressionBuilder); - esAggsToOriginalColumnIdMap[esAggsId] = { + const esAggsId = window.ELASTIC_LENS_DELAY_SECONDS + ? `col-${index + (col.isBucketed ? 0 : 1)}-${aggId}` + : `col-${index}-${aggId}`; + + esAggsIdToOriginalColumn[esAggsId] = { ...col, id: colId, }; @@ -214,8 +208,7 @@ function getExpressionForLayer( functions: [firstFnBuilder], } = first; - const esAggsColumnId = makeid(); - + const esAggsColumnId = firstFnBuilder.getArgument('id')![0]; const aggPercentilesConfig: AggExpressionFunctionArgs = { id: esAggsColumnId, enabled: firstFnBuilder.getArgument('enabled')?.[0], @@ -253,14 +246,64 @@ function getExpressionForLayer( if (currentEsAggsId === undefined) { throw new Error('Could not find current column ID for percentile agg expression builder'); } - // esAggs appends the percent number to the agg id. We're anticipating that here. - const newEsAggsId = `${esAggsColumnId}.${ + // esAggs appends the percent number to the agg id to make distinct column IDs in the resulting datatable. + // We're anticipating that here by adding the `.`. + // The agg index ('?') will be assigned when we update all the indices in the ID map based on the agg order. + const newEsAggsId = `col-?-${esAggsColumnId}.${ expressionBuilder.functions[0].getArgument('percentile')![0] }`; - esAggsToOriginalColumnIdMap[newEsAggsId] = esAggsToOriginalColumnIdMap[currentEsAggsId]; + esAggsIdToOriginalColumn[newEsAggsId] = esAggsIdToOriginalColumn[currentEsAggsId]; + + delete esAggsIdToOriginalColumn[currentEsAggsId]; + }); + }); + + /* + Update ID mappings. + + Given this esAggs-ID-to-original-column map after percentile optimization: + col-0-0: column1 + col-?-1.34: column2 (34th percentile) + col-2-2: column3 + col-?-1.98: column4 (98th percentile) + + and this array of aggs + 0: { id: 0 } + 1: { id: 2 } + 2: { id: 1 } + + We need to update the anticipated agg indicies to match the aggs array: + col-0-0: column1 + col-1-2: column3 + col-2-1.34: column2 (34th percentile) + col-3-3.98: column4 (98th percentile) + */ + const newEsAggsIdToOriginalColumn: Record = {}; + let counter = 0; + aggs.forEach((builder) => { + const extractEsAggId = (id: string) => id.split('.')[0].split('-')[2]; + const updatePositionIndex = (currentId: string, newIndex: number) => { + const [fullId, percentile] = currentId.split('.'); + const idParts = fullId.split('-'); + idParts[1] = String(newIndex); + return idParts.join('-') + (percentile ? `.${percentile}` : ''); + }; + + const esAggId = builder.functions[0].getArgument('id')?.[0]; + const matchingEsAggColumnIds = Object.keys(esAggsIdToOriginalColumn).filter( + (id) => extractEsAggId(id) === esAggId + ); + + matchingEsAggColumnIds.forEach((currentId) => { + const currentColumn = esAggsIdToOriginalColumn[currentId]; + const aggIndex = window.ELASTIC_LENS_DELAY_SECONDS + ? counter + (currentColumn.isBucketed ? 0 : 1) + : counter; + const newId = updatePositionIndex(currentId, aggIndex); + newEsAggsIdToOriginalColumn[newId] = esAggsIdToOriginalColumn[currentId]; - delete esAggsToOriginalColumnIdMap[currentEsAggsId]; + counter++; }); }); @@ -380,7 +423,7 @@ function getExpressionForLayer( type: 'function', function: 'lens_restore_original_column_ids', arguments: { - idMap: [JSON.stringify(esAggsToOriginalColumnIdMap)], + idMap: [JSON.stringify(newEsAggsIdToOriginalColumn)], }, }, ...expressions, From eba315691de3c90bd1c9f9b967cecdb7ced25bd6 Mon Sep 17 00:00:00 2001 From: Andrew Tate Date: Wed, 11 May 2022 09:39:37 -0500 Subject: [PATCH 05/16] move optimization logic to operation class --- .../operations/definitions/index.ts | 17 ++- .../operations/definitions/percentile.tsx | 95 ++++++++++++++- .../indexpattern_datasource/to_expression.ts | 112 ++++-------------- 3 files changed, 133 insertions(+), 91 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts index 36039a058908b..1243cbca56d71 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts @@ -12,7 +12,10 @@ import { CoreStart, } from '@kbn/core/public'; import { IStorageWrapper } from '@kbn/kibana-utils-plugin/public'; -import { ExpressionAstFunction } from '@kbn/expressions-plugin/public'; +import { + ExpressionAstExpressionBuilder, + ExpressionAstFunction, +} from '@kbn/expressions-plugin/public'; import { DataPublicPluginStart } from '@kbn/data-plugin/public'; import { UnifiedSearchPublicPluginStart } from '@kbn/unified-search-plugin/public'; import { termsOperation } from './terms'; @@ -53,6 +56,7 @@ import { IndexPattern, IndexPatternField, IndexPatternLayer } from '../../types' import { DateRange, LayerType } from '../../../../common'; import { rangeOperation } from './ranges'; import { IndexPatternDimensionEditorProps, OperationSupportMatrix } from '../../dimension_panel'; +import type { OriginalColumn } from '../../to_expression'; export type { IncompleteColumn, @@ -372,6 +376,17 @@ interface BaseOperationDefinitionProps * Title for the help component */ helpComponentTitle?: string; + /** + * Optimizes EsAggs expression. Invoked only once per operation type. May mutate arguments. + */ + optimizeEsAggs?: ( + aggs: ExpressionAstExpressionBuilder[], + esAggsIdMap: Record, + aggExpressionToEsAggsIdMap: Map + ) => { + aggs: ExpressionAstExpressionBuilder[]; + esAggsIdMap: Record; + }; } interface BaseBuildColumnArgs { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx index 0699ad5f88405..bf3f0adb18e6a 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx @@ -8,8 +8,13 @@ import { EuiFormRow, EuiRange, EuiRangeProps } from '@elastic/eui'; import React, { useCallback } from 'react'; import { i18n } from '@kbn/i18n'; -import { AggFunctionsMapping } from '@kbn/data-plugin/public'; -import { buildExpressionFunction } from '@kbn/expressions-plugin/public'; +import { AggFunctionsMapping, METRIC_TYPES } from '@kbn/data-plugin/public'; +import { + buildExpression, + buildExpressionFunction, + ExpressionAstExpressionBuilder, +} from '@kbn/expressions-plugin/public'; +import { AggExpressionFunctionArgs } from '@kbn/data-plugin/common'; import { OperationDefinition } from '.'; import { getFormatFromPreviousColumn, @@ -143,6 +148,92 @@ export const percentileOperation: OperationDefinition< } ).toAst(); }, + // TODO - there may be a way to get around including aggExpressionToEsAggsIdMap + optimizeEsAggs: (aggs, esAggsIdMap, aggExpressionToEsAggsIdMap) => { + const percentileExpressionsByArgs: Record = {}; + + aggs.forEach((expressionBuilder) => { + const { + functions: [fnBuilder], + } = expressionBuilder; + if (fnBuilder.name === 'aggSinglePercentile') { + const field = fnBuilder.getArgument('field')?.[0]; + if (!(field in percentileExpressionsByArgs)) { + percentileExpressionsByArgs[field] = []; + } + + percentileExpressionsByArgs[field].push(expressionBuilder); + } + }); + + Object.values(percentileExpressionsByArgs).forEach((expressionBuilders) => { + const [first, ...rest] = expressionBuilders; + + if (!rest.length) { + // don't need to optimize if there's only one + return; + } + + const { + functions: [firstFnBuilder], + } = first; + + const esAggsColumnId = firstFnBuilder.getArgument('id')![0]; + const aggPercentilesConfig: AggExpressionFunctionArgs = { + id: esAggsColumnId, + enabled: firstFnBuilder.getArgument('enabled')?.[0], + schema: firstFnBuilder.getArgument('schema')?.[0], + field: firstFnBuilder.getArgument('field')?.[0], + percents: firstFnBuilder.getArgument('percentile'), + // time shift is added to wrapping aggFilteredMetric if filter is set + timeShift: firstFnBuilder.getArgument('timeShift')?.[0], + }; + + const siblingPercentiles = rest.map( + ({ functions: [fnBuilder] }) => fnBuilder.getArgument('percentile')![0] + ) as number[]; + + aggPercentilesConfig.percents = [ + ...(aggPercentilesConfig.percents ?? []), + ...siblingPercentiles, + ]; + + const multiPercentilesAst = buildExpressionFunction( + 'aggPercentiles', + aggPercentilesConfig + ).toAst(); + + aggs = [ + ...aggs.filter((aggBuilder) => ![first, ...rest].includes(aggBuilder)), + buildExpression({ + type: 'expression', + chain: [multiPercentilesAst], + }), + ]; + + expressionBuilders.forEach((expressionBuilder) => { + const currentEsAggsId = aggExpressionToEsAggsIdMap.get(expressionBuilder); + if (currentEsAggsId === undefined) { + throw new Error('Could not find current column ID for percentile agg expression builder'); + } + // esAggs appends the percent number to the agg id to make distinct column IDs in the resulting datatable. + // We're anticipating that here by adding the `.`. + // The agg index ('?') will be assigned when we update all the indices in the ID map based on the agg order. + const newEsAggsId = `col-?-${esAggsColumnId}.${ + expressionBuilder.functions[0].getArgument('percentile')![0] + }`; + + esAggsIdMap[newEsAggsId] = esAggsIdMap[currentEsAggsId]; + + delete esAggsIdMap[currentEsAggsId]; + }); + }); + + return { + esAggsIdMap, + aggs, + }; + }, getErrorMessage: (layer, columnId, indexPattern) => combineErrorMessages([ getInvalidFieldMessage(layer.columns[columnId] as FieldBasedIndexPatternColumn, indexPattern), diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts index 4ab3f9e66c037..f9c35844d7853 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts @@ -6,14 +6,13 @@ */ import type { IUiSettingsClient } from '@kbn/core/public'; -import { partition } from 'lodash'; +import { partition, uniq } from 'lodash'; import { AggFunctionsMapping, EsaggsExpressionFunctionDefinition, IndexPatternLoadExpressionFunctionDefinition, - METRIC_TYPES, } from '@kbn/data-plugin/public'; -import { AggExpressionFunctionArgs, queryToAst } from '@kbn/data-plugin/common'; +import { queryToAst } from '@kbn/data-plugin/common'; import { buildExpression, buildExpressionFunction, @@ -28,7 +27,7 @@ import { DateHistogramIndexPatternColumn, RangeIndexPatternColumn } from './oper import { FormattedIndexPatternColumn } from './operations/definitions/column_types'; import { isColumnFormatted, isColumnOfType } from './operations/definitions/helpers'; -type OriginalColumn = { id: string } & GenericIndexPatternColumn; +export type OriginalColumn = { id: string } & GenericIndexPatternColumn; declare global { interface Window { @@ -108,8 +107,8 @@ function getExpressionForLayer( }); const orderedColumnIds = esAggEntries.map(([colId]) => colId); - const esAggsIdToOriginalColumn: Record = {}; - const expressionBuilderToEsAggsIdMap: Map = new Map(); + let esAggsIdMap: Record = {}; + const aggExpressionToEsAggsIdMap: Map = new Map(); esAggEntries.forEach(([colId, col], index) => { const def = operationDefinitionMap[col.operationType]; if (def.input !== 'fullReference' && def.input !== 'managedReference') { @@ -155,12 +154,12 @@ function getExpressionForLayer( ? `col-${index + (col.isBucketed ? 0 : 1)}-${aggId}` : `col-${index}-${aggId}`; - esAggsIdToOriginalColumn[esAggsId] = { + esAggsIdMap[esAggsId] = { ...col, id: colId, }; - expressionBuilderToEsAggsIdMap.set(expressionBuilder, esAggsId); + aggExpressionToEsAggsIdMap.set(expressionBuilder, esAggsId); } }); @@ -180,87 +179,24 @@ function getExpressionForLayer( ); } - const percentileExpressionsByArgs: Record = {}; - - aggs.forEach((expressionBuilder) => { - const { - functions: [fnBuilder], - } = expressionBuilder; - if (fnBuilder.name === 'aggSinglePercentile') { - const field = fnBuilder.getArgument('field')?.[0]; - if (!(field in percentileExpressionsByArgs)) { - percentileExpressionsByArgs[field] = []; - } + uniq(esAggEntries.map(([_, column]) => column.operationType)).forEach((type) => { + const optimizeAggs = operationDefinitionMap[type].optimizeEsAggs?.bind( + operationDefinitionMap[type] + ); + if (optimizeAggs) { + const { aggs: newAggs, esAggsIdMap: newIdMap } = optimizeAggs( + aggs, + esAggsIdMap, + aggExpressionToEsAggsIdMap + ); - percentileExpressionsByArgs[field].push(expressionBuilder); + aggs = newAggs; + esAggsIdMap = newIdMap; } }); - Object.values(percentileExpressionsByArgs).forEach((expressionBuilders) => { - const [first, ...rest] = expressionBuilders; - - if (!rest.length) { - // don't need to optimize if there's only one - return; - } - - const { - functions: [firstFnBuilder], - } = first; - - const esAggsColumnId = firstFnBuilder.getArgument('id')![0]; - const aggPercentilesConfig: AggExpressionFunctionArgs = { - id: esAggsColumnId, - enabled: firstFnBuilder.getArgument('enabled')?.[0], - schema: firstFnBuilder.getArgument('schema')?.[0], - field: firstFnBuilder.getArgument('field')?.[0], - percents: firstFnBuilder.getArgument('percentile'), - // time shift is added to wrapping aggFilteredMetric if filter is set - timeShift: firstFnBuilder.getArgument('timeShift')?.[0], - }; - - const siblingPercentiles = rest.map( - ({ functions: [fnBuilder] }) => fnBuilder.getArgument('percentile')![0] - ) as number[]; - - aggPercentilesConfig.percents = [ - ...(aggPercentilesConfig.percents ?? []), - ...siblingPercentiles, - ]; - - const multiPercentilesAst = buildExpressionFunction( - 'aggPercentiles', - aggPercentilesConfig - ).toAst(); - - aggs = [ - ...aggs.filter((aggBuilder) => ![first, ...rest].includes(aggBuilder)), - buildExpression({ - type: 'expression', - chain: [multiPercentilesAst], - }), - ]; - - expressionBuilders.forEach((expressionBuilder) => { - const currentEsAggsId = expressionBuilderToEsAggsIdMap.get(expressionBuilder); - if (currentEsAggsId === undefined) { - throw new Error('Could not find current column ID for percentile agg expression builder'); - } - // esAggs appends the percent number to the agg id to make distinct column IDs in the resulting datatable. - // We're anticipating that here by adding the `.`. - // The agg index ('?') will be assigned when we update all the indices in the ID map based on the agg order. - const newEsAggsId = `col-?-${esAggsColumnId}.${ - expressionBuilder.functions[0].getArgument('percentile')![0] - }`; - - esAggsIdToOriginalColumn[newEsAggsId] = esAggsIdToOriginalColumn[currentEsAggsId]; - - delete esAggsIdToOriginalColumn[currentEsAggsId]; - }); - }); - /* - Update ID mappings. + Update ID mappings with new agg array positions. Given this esAggs-ID-to-original-column map after percentile optimization: col-0-0: column1 @@ -275,8 +211,8 @@ function getExpressionForLayer( We need to update the anticipated agg indicies to match the aggs array: col-0-0: column1 - col-1-2: column3 col-2-1.34: column2 (34th percentile) + col-1-2: column3 col-3-3.98: column4 (98th percentile) */ const newEsAggsIdToOriginalColumn: Record = {}; @@ -291,17 +227,17 @@ function getExpressionForLayer( }; const esAggId = builder.functions[0].getArgument('id')?.[0]; - const matchingEsAggColumnIds = Object.keys(esAggsIdToOriginalColumn).filter( + const matchingEsAggColumnIds = Object.keys(esAggsIdMap).filter( (id) => extractEsAggId(id) === esAggId ); matchingEsAggColumnIds.forEach((currentId) => { - const currentColumn = esAggsIdToOriginalColumn[currentId]; + const currentColumn = esAggsIdMap[currentId]; const aggIndex = window.ELASTIC_LENS_DELAY_SECONDS ? counter + (currentColumn.isBucketed ? 0 : 1) : counter; const newId = updatePositionIndex(currentId, aggIndex); - newEsAggsIdToOriginalColumn[newId] = esAggsIdToOriginalColumn[currentId]; + newEsAggsIdToOriginalColumn[newId] = esAggsIdMap[currentId]; counter++; }); From 5d65cad43917718dbe964e5ad9b8df43f0be63cc Mon Sep 17 00:00:00 2001 From: Andrew Tate Date: Wed, 11 May 2022 09:45:42 -0500 Subject: [PATCH 06/16] group percentiles by timeshift as well as field --- .../operations/definitions/percentile.tsx | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx index bf3f0adb18e6a..489357732f85d 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx @@ -157,12 +157,14 @@ export const percentileOperation: OperationDefinition< functions: [fnBuilder], } = expressionBuilder; if (fnBuilder.name === 'aggSinglePercentile') { - const field = fnBuilder.getArgument('field')?.[0]; - if (!(field in percentileExpressionsByArgs)) { - percentileExpressionsByArgs[field] = []; + const groupByKey = `${fnBuilder.getArgument('field')?.[0]}-${ + fnBuilder.getArgument('timeShift')?.[0] + }`; + if (!(groupByKey in percentileExpressionsByArgs)) { + percentileExpressionsByArgs[groupByKey] = []; } - percentileExpressionsByArgs[field].push(expressionBuilder); + percentileExpressionsByArgs[groupByKey].push(expressionBuilder); } }); From 7610415c6359e10477c8639527dff2d26441bb38 Mon Sep 17 00:00:00 2001 From: Andrew Tate Date: Mon, 13 Jun 2022 10:00:21 -0500 Subject: [PATCH 07/16] support one-to-many for remapping columns --- .../rename_columns/rename_columns.test.ts | 85 ++++++++++++++++--- .../rename_columns/rename_columns_fn.ts | 37 ++++---- 2 files changed, 94 insertions(+), 28 deletions(-) diff --git a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.test.ts b/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.test.ts index 2047e4647cb4c..59b06a6980435 100644 --- a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.test.ts +++ b/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.test.ts @@ -26,14 +26,18 @@ describe('rename_columns', () => { }; const idMap = { - a: { - id: 'b', - label: 'Austrailia', - }, - b: { - id: 'c', - label: 'Boomerang', - }, + a: [ + { + id: 'b', + label: 'Austrailia', + }, + ], + b: [ + { + id: 'c', + label: 'Boomerang', + }, + ], }; const result = await renameColumns.fn( @@ -99,7 +103,7 @@ describe('rename_columns', () => { }; const idMap = { - b: { id: 'c', label: 'Catamaran' }, + b: [{ id: 'c', label: 'Catamaran' }], }; const result = await renameColumns.fn( @@ -149,6 +153,67 @@ describe('rename_columns', () => { `); }); + it('should map to multiple original columns', async () => { + const input: Datatable = { + type: 'datatable', + columns: [{ id: 'b', name: 'B', meta: { type: 'number' } }], + rows: [{ b: 2 }, { b: 4 }, { b: 6 }, { b: 8 }], + }; + + const idMap = { + b: [ + { id: 'c', label: 'Catamaran' }, + { id: 'd', label: 'Dinghy' }, + ], + }; + + const result = await renameColumns.fn( + input, + { idMap: JSON.stringify(idMap) }, + createMockExecutionContext() + ); + + expect(result).toMatchInlineSnapshot(` + Object { + "columns": Array [ + Object { + "id": "c", + "meta": Object { + "type": "number", + }, + "name": "Catamaran", + }, + Object { + "id": "d", + "meta": Object { + "type": "number", + }, + "name": "Dinghy", + }, + ], + "rows": Array [ + Object { + "c": 2, + "d": 2, + }, + Object { + "c": 4, + "d": 4, + }, + Object { + "c": 6, + "d": 6, + }, + Object { + "c": 8, + "d": 8, + }, + ], + "type": "datatable", + } + `); + }); + it('should rename date histograms', async () => { const input: Datatable = { type: 'datatable', @@ -165,7 +230,7 @@ describe('rename_columns', () => { }; const idMap = { - b: { id: 'c', label: 'Apple', operationType: 'date_histogram', sourceField: 'banana' }, + b: [{ id: 'c', label: 'Apple', operationType: 'date_histogram', sourceField: 'banana' }], }; const result = await renameColumns.fn( diff --git a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns_fn.ts b/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns_fn.ts index 3cf4293ffa9f2..2018e3128733a 100644 --- a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns_fn.ts +++ b/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns_fn.ts @@ -25,19 +25,18 @@ export const renameColumnFn: RenameColumnsExpressionFunction['fn'] = ( data, { idMap: encodedIdMap } ) => { - const idMap = JSON.parse(encodedIdMap) as Record; + const idMap = JSON.parse(encodedIdMap) as Record; return { ...data, rows: data.rows.map((row) => { const mappedRow: Record = {}; - Object.entries(idMap).forEach(([fromId, toId]) => { - mappedRow[toId.id] = row[fromId]; - }); Object.entries(row).forEach(([id, value]) => { if (id in idMap) { - mappedRow[idMap[id].id] = value; + idMap[id].forEach(({ id: originalId }) => { + mappedRow[originalId] = value; + }); } else { mappedRow[id] = value; } @@ -45,18 +44,20 @@ export const renameColumnFn: RenameColumnsExpressionFunction['fn'] = ( return mappedRow; }), - columns: data.columns.map((column) => { - const mappedItem = idMap[column.id]; - - if (!mappedItem) { - return column; - } - - return { - ...column, - id: mappedItem.id, - name: getColumnName(mappedItem, column), - }; - }), + columns: data.columns + .map((column) => { + const originalColumns = idMap[column.id]; + + if (!originalColumns) { + return column; + } + + return originalColumns.map((originalColumn) => ({ + ...column, + id: originalColumn.id, + name: getColumnName(originalColumn, column), + })); + }) + .flat(), }; }; From 4ea17ed3bbcb324db2ccf297bffcf0f36e60b11b Mon Sep 17 00:00:00 2001 From: Andrew Tate Date: Mon, 13 Jun 2022 11:55:05 -0500 Subject: [PATCH 08/16] handle duplicate percentile dimensions --- .../operations/definitions/index.ts | 4 +- .../operations/definitions/percentile.tsx | 61 +++++++++++++------ .../indexpattern_datasource/to_expression.ts | 17 +++--- 3 files changed, 56 insertions(+), 26 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts index 22289a412d3a7..58d2508365829 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts @@ -387,11 +387,11 @@ interface BaseOperationDefinitionProps */ optimizeEsAggs?: ( aggs: ExpressionAstExpressionBuilder[], - esAggsIdMap: Record, + esAggsIdMap: Record, aggExpressionToEsAggsIdMap: Map ) => { aggs: ExpressionAstExpressionBuilder[]; - esAggsIdMap: Record; + esAggsIdMap: Record; }; } diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx index 489357732f85d..12859035ed139 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx @@ -152,6 +152,7 @@ export const percentileOperation: OperationDefinition< optimizeEsAggs: (aggs, esAggsIdMap, aggExpressionToEsAggsIdMap) => { const percentileExpressionsByArgs: Record = {}; + // group percentile dimensions by differentiating parameters aggs.forEach((expressionBuilder) => { const { functions: [fnBuilder], @@ -168,17 +169,20 @@ export const percentileOperation: OperationDefinition< } }); + // collapse them into a single esAggs expression builder Object.values(percentileExpressionsByArgs).forEach((expressionBuilders) => { - const [first, ...rest] = expressionBuilders; - - if (!rest.length) { + if (!(expressionBuilders.length > 1)) { // don't need to optimize if there's only one return; } + // we're going to merge these percentile builders into a single builder, so + // remove them from the aggs array + aggs = aggs.filter((aggBuilder) => !expressionBuilders.includes(aggBuilder)); + const { functions: [firstFnBuilder], - } = first; + } = expressionBuilders[0]; const esAggsColumnId = firstFnBuilder.getArgument('id')![0]; const aggPercentilesConfig: AggExpressionFunctionArgs = { @@ -186,32 +190,54 @@ export const percentileOperation: OperationDefinition< enabled: firstFnBuilder.getArgument('enabled')?.[0], schema: firstFnBuilder.getArgument('schema')?.[0], field: firstFnBuilder.getArgument('field')?.[0], - percents: firstFnBuilder.getArgument('percentile'), + percents: [], // time shift is added to wrapping aggFilteredMetric if filter is set timeShift: firstFnBuilder.getArgument('timeShift')?.[0], }; - const siblingPercentiles = rest.map( - ({ functions: [fnBuilder] }) => fnBuilder.getArgument('percentile')![0] - ) as number[]; + const percentileToBuilder: Record = {}; + for (const builder of expressionBuilders) { + const percentile = builder.functions[0].getArgument('percentile')![0] as number; + if (percentile in percentileToBuilder) { + // found a duplicate percentile so let's optimize + + const duplicateExpressionBuilder = percentileToBuilder[percentile]; + + if ( + !aggExpressionToEsAggsIdMap.has(builder) || + !aggExpressionToEsAggsIdMap.has(duplicateExpressionBuilder) + ) { + throw new Error( + "Couldn't find esAggs ID for percentile expression builder... this should never happen." + ); + } + + const idForDuplicate = aggExpressionToEsAggsIdMap.get(duplicateExpressionBuilder)!; + const idForThisOne = aggExpressionToEsAggsIdMap.get(builder)!; - aggPercentilesConfig.percents = [ - ...(aggPercentilesConfig.percents ?? []), - ...siblingPercentiles, - ]; + esAggsIdMap[idForDuplicate].push(...esAggsIdMap[idForThisOne]); + + delete esAggsIdMap[idForThisOne]; + + // remove current builder + expressionBuilders = expressionBuilders.filter((b) => b !== builder); + } else { + percentileToBuilder[percentile] = builder; + aggPercentilesConfig.percents!.push(percentile); + } + } const multiPercentilesAst = buildExpressionFunction( 'aggPercentiles', aggPercentilesConfig ).toAst(); - aggs = [ - ...aggs.filter((aggBuilder) => ![first, ...rest].includes(aggBuilder)), + aggs.push( buildExpression({ type: 'expression', chain: [multiPercentilesAst], - }), - ]; + }) + ); expressionBuilders.forEach((expressionBuilder) => { const currentEsAggsId = aggExpressionToEsAggsIdMap.get(expressionBuilder); @@ -220,7 +246,8 @@ export const percentileOperation: OperationDefinition< } // esAggs appends the percent number to the agg id to make distinct column IDs in the resulting datatable. // We're anticipating that here by adding the `.`. - // The agg index ('?') will be assigned when we update all the indices in the ID map based on the agg order. + // The agg index will be assigned when we update all the indices in the ID map based on the agg order in the + // datasource's toExpression fn so we mark it as '?' for now. const newEsAggsId = `col-?-${esAggsColumnId}.${ expressionBuilder.functions[0].getArgument('percentile')![0] }`; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts index da75b905a22a4..88081719ba297 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts @@ -107,7 +107,7 @@ function getExpressionForLayer( }); const orderedColumnIds = esAggEntries.map(([colId]) => colId); - let esAggsIdMap: Record = {}; + let esAggsIdMap: Record = {}; const aggExpressionToEsAggsIdMap: Map = new Map(); esAggEntries.forEach(([colId, col], index) => { const def = operationDefinitionMap[col.operationType]; @@ -154,10 +154,12 @@ function getExpressionForLayer( ? `col-${index + (col.isBucketed ? 0 : 1)}-${aggId}` : `col-${index}-${aggId}`; - esAggsIdMap[esAggsId] = { - ...col, - id: colId, - }; + esAggsIdMap[esAggsId] = [ + { + ...col, + id: colId, + }, + ]; aggExpressionToEsAggsIdMap.set(expressionBuilder, esAggsId); } @@ -215,7 +217,7 @@ function getExpressionForLayer( col-1-2: column3 col-3-3.98: column4 (98th percentile) */ - const newEsAggsIdToOriginalColumn: Record = {}; + const newEsAggsIdToOriginalColumn: Record = {}; let counter = 0; aggs.forEach((builder) => { const extractEsAggId = (id: string) => id.split('.')[0].split('-')[2]; @@ -232,7 +234,8 @@ function getExpressionForLayer( ); matchingEsAggColumnIds.forEach((currentId) => { - const currentColumn = esAggsIdMap[currentId]; + const currentColumn = esAggsIdMap[currentId][0]; + // TODO what if multiple columns are mapped to, and only one is bucketed? const aggIndex = window.ELASTIC_LENS_DELAY_SECONDS ? counter + (currentColumn.isBucketed ? 0 : 1) : counter; From aee9e26923146049ab6c4c9f7414bf1cf4ef7cd5 Mon Sep 17 00:00:00 2001 From: Andrew Tate Date: Mon, 13 Jun 2022 14:21:38 -0500 Subject: [PATCH 09/16] basic collapse-by-parameter unit test --- .../definitions/percentile.test.tsx | 331 ++++++++++++++++++ 1 file changed, 331 insertions(+) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx index ae8ba7d965ea7..0d93796c89285 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx @@ -20,6 +20,8 @@ import { percentileOperation } from '.'; import { IndexPattern, IndexPatternLayer } from '../../types'; import { PercentileIndexPatternColumn } from './percentile'; import { TermsIndexPatternColumn } from './terms'; +import { buildExpressionFunction, buildExpression } from '@kbn/expressions-plugin/public'; +import type { OriginalColumn } from '../../to_expression'; jest.mock('lodash', () => { const original = jest.requireActual('lodash'); @@ -187,6 +189,335 @@ describe('percentile', () => { }); }); + describe('optimizeEsAggs', () => { + it('should collapse percentile dimensions with matching parameters', () => { + const field1 = 'foo'; + const field2 = 'bar'; + const timeShift1 = '1d'; + const timeShift2 = '2d'; + + const makeEsAggBuilder = (name: string, params: object) => + buildExpression({ + type: 'expression', + chain: [buildExpressionFunction(name, params).toAst()], + }); + + const aggs = [ + // group 1 + makeEsAggBuilder('aggSinglePercentile', { + id: 1, + enabled: true, + schema: 'metric', + field: field1, + percentile: 10, + timeShift: undefined, + }), + makeEsAggBuilder('aggSinglePercentile', { + id: 2, + enabled: true, + schema: 'metric', + field: field1, + percentile: 20, + timeShift: undefined, + }), + makeEsAggBuilder('aggSinglePercentile', { + id: 3, + enabled: true, + schema: 'metric', + field: field1, + percentile: 30, + timeShift: undefined, + }), + // group 2 + makeEsAggBuilder('aggSinglePercentile', { + id: 4, + enabled: true, + schema: 'metric', + field: field2, + percentile: 10, + timeShift: undefined, + }), + makeEsAggBuilder('aggSinglePercentile', { + id: 5, + enabled: true, + schema: 'metric', + field: field2, + percentile: 40, + timeShift: undefined, + }), + // group 3 + makeEsAggBuilder('aggSinglePercentile', { + id: 6, + enabled: true, + schema: 'metric', + field: field2, + percentile: 50, + timeShift: timeShift1, + }), + makeEsAggBuilder('aggSinglePercentile', { + id: 7, + enabled: true, + schema: 'metric', + field: field2, + percentile: 60, + timeShift: timeShift1, + }), + // group 4 + makeEsAggBuilder('aggSinglePercentile', { + id: 8, + enabled: true, + schema: 'metric', + field: field2, + percentile: 70, + timeShift: timeShift2, + }), + makeEsAggBuilder('aggSinglePercentile', { + id: 9, + enabled: true, + schema: 'metric', + field: field2, + percentile: 80, + timeShift: timeShift2, + }), + ]; + + const idMap: Record = {}; + const aggsToIdsMap = new Map(); + aggs.forEach((builder, i) => { + const esAggsId = `col-${i}-${i}`; + idMap[esAggsId] = [{ id: `original-${i}` } as OriginalColumn]; + aggsToIdsMap.set(builder, esAggsId); + }); + + const { esAggsIdMap, aggs: newAggs } = percentileOperation.optimizeEsAggs!( + aggs, + idMap, + aggsToIdsMap + ); + + expect(newAggs.length).toBe(4); + + expect(newAggs[0].functions[0].getArgument('field')![0]).toBe(field1); + expect(newAggs[0].functions[0].getArgument('timeShift')).toBeUndefined(); + expect(newAggs[1].functions[0].getArgument('field')![0]).toBe(field2); + expect(newAggs[1].functions[0].getArgument('timeShift')).toBeUndefined(); + expect(newAggs[2].functions[0].getArgument('field')![0]).toBe(field2); + expect(newAggs[2].functions[0].getArgument('timeShift')![0]).toBe(timeShift1); + expect(newAggs[3].functions[0].getArgument('field')![0]).toBe(field2); + expect(newAggs[3].functions[0].getArgument('timeShift')![0]).toBe(timeShift2); + + expect(newAggs).toMatchInlineSnapshot( + ` + Array [ + Object { + "findFunction": [Function], + "functions": Array [ + Object { + "addArgument": [Function], + "arguments": Object { + "enabled": Array [ + true, + ], + "field": Array [ + "foo", + ], + "id": Array [ + 1, + ], + "percents": Array [ + 10, + 20, + 30, + ], + "schema": Array [ + "metric", + ], + }, + "getArgument": [Function], + "name": "aggPercentiles", + "removeArgument": [Function], + "replaceArgument": [Function], + "toAst": [Function], + "toString": [Function], + "type": "expression_function_builder", + }, + ], + "toAst": [Function], + "toString": [Function], + "type": "expression_builder", + }, + Object { + "findFunction": [Function], + "functions": Array [ + Object { + "addArgument": [Function], + "arguments": Object { + "enabled": Array [ + true, + ], + "field": Array [ + "bar", + ], + "id": Array [ + 4, + ], + "percents": Array [ + 10, + 40, + ], + "schema": Array [ + "metric", + ], + }, + "getArgument": [Function], + "name": "aggPercentiles", + "removeArgument": [Function], + "replaceArgument": [Function], + "toAst": [Function], + "toString": [Function], + "type": "expression_function_builder", + }, + ], + "toAst": [Function], + "toString": [Function], + "type": "expression_builder", + }, + Object { + "findFunction": [Function], + "functions": Array [ + Object { + "addArgument": [Function], + "arguments": Object { + "enabled": Array [ + true, + ], + "field": Array [ + "bar", + ], + "id": Array [ + 6, + ], + "percents": Array [ + 50, + 60, + ], + "schema": Array [ + "metric", + ], + "timeShift": Array [ + "1d", + ], + }, + "getArgument": [Function], + "name": "aggPercentiles", + "removeArgument": [Function], + "replaceArgument": [Function], + "toAst": [Function], + "toString": [Function], + "type": "expression_function_builder", + }, + ], + "toAst": [Function], + "toString": [Function], + "type": "expression_builder", + }, + Object { + "findFunction": [Function], + "functions": Array [ + Object { + "addArgument": [Function], + "arguments": Object { + "enabled": Array [ + true, + ], + "field": Array [ + "bar", + ], + "id": Array [ + 8, + ], + "percents": Array [ + 70, + 80, + ], + "schema": Array [ + "metric", + ], + "timeShift": Array [ + "2d", + ], + }, + "getArgument": [Function], + "name": "aggPercentiles", + "removeArgument": [Function], + "replaceArgument": [Function], + "toAst": [Function], + "toString": [Function], + "type": "expression_function_builder", + }, + ], + "toAst": [Function], + "toString": [Function], + "type": "expression_builder", + }, + ] + ` + ); + + expect(esAggsIdMap).toMatchInlineSnapshot( + ` + Object { + "col-?-1.10": Array [ + Object { + "id": "original-0", + }, + ], + "col-?-1.20": Array [ + Object { + "id": "original-1", + }, + ], + "col-?-1.30": Array [ + Object { + "id": "original-2", + }, + ], + "col-?-4.10": Array [ + Object { + "id": "original-3", + }, + ], + "col-?-4.40": Array [ + Object { + "id": "original-4", + }, + ], + "col-?-6.50": Array [ + Object { + "id": "original-5", + }, + ], + "col-?-6.60": Array [ + Object { + "id": "original-6", + }, + ], + "col-?-8.70": Array [ + Object { + "id": "original-7", + }, + ], + "col-?-8.80": Array [ + Object { + "id": "original-8", + }, + ], + } + ` + ); + }); + }); + describe('buildColumn', () => { it('should set default percentile', () => { const indexPattern = createMockedIndexPattern(); From a6cdca5de054d4d963d26fbc4bcd60582a77caaf Mon Sep 17 00:00:00 2001 From: Andrew Tate Date: Mon, 13 Jun 2022 17:00:20 -0500 Subject: [PATCH 10/16] more tests --- .../definitions/percentile.test.tsx | 156 +++++++++++++++--- .../indexpattern_datasource/to_expression.ts | 2 +- 2 files changed, 134 insertions(+), 24 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx index 0d93796c89285..08afcc447eec6 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx @@ -20,7 +20,11 @@ import { percentileOperation } from '.'; import { IndexPattern, IndexPatternLayer } from '../../types'; import { PercentileIndexPatternColumn } from './percentile'; import { TermsIndexPatternColumn } from './terms'; -import { buildExpressionFunction, buildExpression } from '@kbn/expressions-plugin/public'; +import { + buildExpressionFunction, + buildExpression, + ExpressionAstExpressionBuilder, +} from '@kbn/expressions-plugin/public'; import type { OriginalColumn } from '../../to_expression'; jest.mock('lodash', () => { @@ -190,18 +194,32 @@ describe('percentile', () => { }); describe('optimizeEsAggs', () => { + const makeEsAggBuilder = (name: string, params: object) => + buildExpression({ + type: 'expression', + chain: [buildExpressionFunction(name, params).toAst()], + }); + + const buildMapsFromAggBuilders = (aggs: ExpressionAstExpressionBuilder[]) => { + const esAggsIdMap: Record = {}; + const aggsToIdsMap = new Map(); + aggs.forEach((builder, i) => { + const esAggsId = `col-${i}-${i}`; + esAggsIdMap[esAggsId] = [{ id: `original-${i}` } as OriginalColumn]; + aggsToIdsMap.set(builder, esAggsId); + }); + return { + esAggsIdMap, + aggsToIdsMap, + }; + }; + it('should collapse percentile dimensions with matching parameters', () => { const field1 = 'foo'; const field2 = 'bar'; const timeShift1 = '1d'; const timeShift2 = '2d'; - const makeEsAggBuilder = (name: string, params: object) => - buildExpression({ - type: 'expression', - chain: [buildExpressionFunction(name, params).toAst()], - }); - const aggs = [ // group 1 makeEsAggBuilder('aggSinglePercentile', { @@ -281,17 +299,11 @@ describe('percentile', () => { }), ]; - const idMap: Record = {}; - const aggsToIdsMap = new Map(); - aggs.forEach((builder, i) => { - const esAggsId = `col-${i}-${i}`; - idMap[esAggsId] = [{ id: `original-${i}` } as OriginalColumn]; - aggsToIdsMap.set(builder, esAggsId); - }); + const { esAggsIdMap, aggsToIdsMap } = buildMapsFromAggBuilders(aggs); - const { esAggsIdMap, aggs: newAggs } = percentileOperation.optimizeEsAggs!( + const { esAggsIdMap: newIdMap, aggs: newAggs } = percentileOperation.optimizeEsAggs!( aggs, - idMap, + esAggsIdMap, aggsToIdsMap ); @@ -306,8 +318,7 @@ describe('percentile', () => { expect(newAggs[3].functions[0].getArgument('field')![0]).toBe(field2); expect(newAggs[3].functions[0].getArgument('timeShift')![0]).toBe(timeShift2); - expect(newAggs).toMatchInlineSnapshot( - ` + expect(newAggs).toMatchInlineSnapshot(` Array [ Object { "findFunction": [Function], @@ -461,11 +472,9 @@ describe('percentile', () => { "type": "expression_builder", }, ] - ` - ); + `); - expect(esAggsIdMap).toMatchInlineSnapshot( - ` + expect(newIdMap).toMatchInlineSnapshot(` Object { "col-?-1.10": Array [ Object { @@ -513,8 +522,109 @@ describe('percentile', () => { }, ], } - ` + `); + }); + + it('should handle multiple identical percentiles', () => { + const field1 = 'foo'; + const field2 = 'bar'; + const samePercentile = 90; + + const aggs = [ + // group 1 + makeEsAggBuilder('aggSinglePercentile', { + id: 1, + enabled: true, + schema: 'metric', + field: field1, + percentile: samePercentile, + timeShift: undefined, + }), + makeEsAggBuilder('aggSinglePercentile', { + id: 2, + enabled: true, + schema: 'metric', + field: field1, + percentile: samePercentile, + timeShift: undefined, + }), + makeEsAggBuilder('aggSinglePercentile', { + id: 4, + enabled: true, + schema: 'metric', + field: field2, + percentile: 10, + timeShift: undefined, + }), + makeEsAggBuilder('aggSinglePercentile', { + id: 3, + enabled: true, + schema: 'metric', + field: field1, + percentile: samePercentile, + timeShift: undefined, + }), + ]; + + const { esAggsIdMap, aggsToIdsMap } = buildMapsFromAggBuilders(aggs); + + const { esAggsIdMap: newIdMap, aggs: newAggs } = percentileOperation.optimizeEsAggs!( + aggs, + esAggsIdMap, + aggsToIdsMap ); + + expect(newAggs.length).toBe(2); + expect(newIdMap[`col-?-1.${samePercentile}`].length).toBe(3); + expect(newIdMap).toMatchInlineSnapshot(` + Object { + "col-2-2": Array [ + Object { + "id": "original-2", + }, + ], + "col-?-1.90": Array [ + Object { + "id": "original-0", + }, + Object { + "id": "original-1", + }, + Object { + "id": "original-3", + }, + ], + } + `); + }); + + it("shouldn't touch non-percentile aggs or single percentiles with no siblings", () => { + const aggs = [ + makeEsAggBuilder('aggSinglePercentile', { + id: 1, + enabled: true, + schema: 'metric', + field: 'foo', + percentile: 30, + }), + makeEsAggBuilder('aggMax', { + id: 1, + enabled: true, + schema: 'metric', + field: 'bar', + }), + ]; + + const { esAggsIdMap, aggsToIdsMap } = buildMapsFromAggBuilders(aggs); + + const { esAggsIdMap: newIdMap, aggs: newAggs } = percentileOperation.optimizeEsAggs!( + aggs, + esAggsIdMap, + aggsToIdsMap + ); + + expect(newAggs).toEqual(aggs); + expect(newIdMap).toEqual(esAggsIdMap); }); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts index 88081719ba297..a88046b508ebc 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts @@ -200,7 +200,7 @@ function getExpressionForLayer( /* Update ID mappings with new agg array positions. - Given this esAggs-ID-to-original-column map after percentile optimization: + Given this esAggs-ID-to-original-column map after percentile (for example) optimization: col-0-0: column1 col-?-1.34: column2 (34th percentile) col-2-2: column3 From 970a78bc664a97904444c6e23dd97d98135466c0 Mon Sep 17 00:00:00 2001 From: Andrew Tate Date: Mon, 13 Jun 2022 17:33:19 -0500 Subject: [PATCH 11/16] update indexpattern toExpression test --- .../rename_columns/rename_columns.ts | 10 +++++----- .../rename_columns/rename_columns_fn.ts | 4 ++-- .../expressions/rename_columns/types.ts | 4 ++-- .../indexpattern.test.ts | 19 +++++++++++-------- .../indexpattern_datasource/to_expression.ts | 2 +- 5 files changed, 21 insertions(+), 18 deletions(-) diff --git a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.ts b/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.ts index e03ae52a04f4a..373390d51d2cf 100644 --- a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.ts +++ b/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.ts @@ -6,10 +6,10 @@ */ import { i18n } from '@kbn/i18n'; -import type { RenameColumnsExpressionFunction } from './types'; +import type { MapToOriginalColumnsExpressionFunction } from './types'; -export const renameColumns: RenameColumnsExpressionFunction = { - name: 'lens_restore_original_column_ids', +export const renameColumns: MapToOriginalColumnsExpressionFunction = { + name: 'lens_map_to_original_columns', type: 'datatable', help: i18n.translate('xpack.lens.functions.renameColumns.help', { defaultMessage: 'A helper to rename the columns of a datatable', @@ -26,7 +26,7 @@ export const renameColumns: RenameColumnsExpressionFunction = { inputTypes: ['datatable'], async fn(...args) { /** Build optimization: prevent adding extra code into initial bundle **/ - const { renameColumnFn } = await import('./rename_columns_fn'); - return renameColumnFn(...args); + const { mapToOriginalColumns } = await import('./rename_columns_fn'); + return mapToOriginalColumns(...args); }, }; diff --git a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns_fn.ts b/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns_fn.ts index 2018e3128733a..793e6183d8157 100644 --- a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns_fn.ts +++ b/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns_fn.ts @@ -6,7 +6,7 @@ */ import type { DatatableColumn } from '@kbn/expressions-plugin/common'; -import type { OriginalColumn, RenameColumnsExpressionFunction } from './types'; +import type { OriginalColumn, MapToOriginalColumnsExpressionFunction } from './types'; function getColumnName(originalColumn: OriginalColumn, newColumn: DatatableColumn) { if (originalColumn?.operationType === 'date_histogram') { @@ -21,7 +21,7 @@ function getColumnName(originalColumn: OriginalColumn, newColumn: DatatableColum return originalColumn.label; } -export const renameColumnFn: RenameColumnsExpressionFunction['fn'] = ( +export const mapToOriginalColumns: MapToOriginalColumnsExpressionFunction['fn'] = ( data, { idMap: encodedIdMap } ) => { diff --git a/x-pack/plugins/lens/common/expressions/rename_columns/types.ts b/x-pack/plugins/lens/common/expressions/rename_columns/types.ts index 243662b65842a..f10e6bda35a3b 100644 --- a/x-pack/plugins/lens/common/expressions/rename_columns/types.ts +++ b/x-pack/plugins/lens/common/expressions/rename_columns/types.ts @@ -12,8 +12,8 @@ export type OriginalColumn = { id: string; label: string } & ( | { operationType: string; sourceField: never } ); -export type RenameColumnsExpressionFunction = ExpressionFunctionDefinition< - 'lens_restore_original_column_ids', +export type MapToOriginalColumnsExpressionFunction = ExpressionFunctionDefinition< + 'lens_map_to_original_columns', Datatable, { idMap: string; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts index 6806b1ce47795..03cb620468203 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts @@ -491,10 +491,10 @@ describe('IndexPattern Data Source', () => { Object { "arguments": Object { "idMap": Array [ - "{\\"col-0-0\\":{\\"label\\":\\"Count of records\\",\\"dataType\\":\\"number\\",\\"isBucketed\\":false,\\"sourceField\\":\\"___records___\\",\\"operationType\\":\\"count\\",\\"id\\":\\"col1\\"},\\"col-1-1\\":{\\"label\\":\\"Date\\",\\"dataType\\":\\"date\\",\\"isBucketed\\":true,\\"operationType\\":\\"date_histogram\\",\\"sourceField\\":\\"timestamp\\",\\"params\\":{\\"interval\\":\\"1d\\"},\\"id\\":\\"col2\\"}}", + "{\\"col-0-0\\":[{\\"label\\":\\"Count of records\\",\\"dataType\\":\\"number\\",\\"isBucketed\\":false,\\"sourceField\\":\\"___records___\\",\\"operationType\\":\\"count\\",\\"id\\":\\"col1\\"}],\\"col-1-1\\":[{\\"label\\":\\"Date\\",\\"dataType\\":\\"date\\",\\"isBucketed\\":true,\\"operationType\\":\\"date_histogram\\",\\"sourceField\\":\\"timestamp\\",\\"params\\":{\\"interval\\":\\"1d\\"},\\"id\\":\\"col2\\"}]}", ], }, - "function": "lens_rename_columns", + "function": "lens_map_to_original_columns", "type": "function", }, ], @@ -905,9 +905,9 @@ describe('IndexPattern Data Source', () => { const ast = indexPatternDatasource.toExpression(state, 'first') as Ast; expect(ast.chain[1].arguments.metricsAtAllLevels).toEqual([false]); expect(JSON.parse(ast.chain[2].arguments.idMap[0] as string)).toEqual({ - 'col-0-0': expect.objectContaining({ id: 'bucket1' }), - 'col-1-1': expect.objectContaining({ id: 'bucket2' }), - 'col-2-2': expect.objectContaining({ id: 'metric' }), + 'col-0-0': [expect.objectContaining({ id: 'bucket1' })], + 'col-1-1': [expect.objectContaining({ id: 'bucket2' })], + 'col-2-2': [expect.objectContaining({ id: 'metric' })], }); }); @@ -1026,10 +1026,13 @@ describe('IndexPattern Data Source', () => { const state = enrichBaseState(queryBaseState); const ast = indexPatternDatasource.toExpression(state, 'first') as Ast; + expect(JSON.parse(ast.chain[2].arguments.idMap[0] as string)).toEqual({ - 'col-0-0': expect.objectContaining({ - id: 'col1', - }), + 'col-0-0': [ + expect.objectContaining({ + id: 'col1', + }), + ], }); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts index a88046b508ebc..d21c0498d0712 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts @@ -361,7 +361,7 @@ function getExpressionForLayer( }).toAst(), { type: 'function', - function: 'lens_restore_original_column_ids', + function: 'lens_map_to_original_columns', arguments: { idMap: [JSON.stringify(newEsAggsIdToOriginalColumn)], }, From 5d6174253d163ce6090066faeb50272957e5a16d Mon Sep 17 00:00:00 2001 From: Andrew Tate Date: Tue, 14 Jun 2022 11:13:39 -0500 Subject: [PATCH 12/16] test optimizeEsAggs contract --- .../indexpattern.test.ts | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts index 03cb620468203..c37d238582047 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts @@ -33,6 +33,7 @@ import { FormulaIndexPatternColumn, RangeIndexPatternColumn, FiltersIndexPatternColumn, + PercentileIndexPatternColumn, } from './operations'; import { createMockedFullReference } from './operations/mocks'; import { cloneDeep } from 'lodash'; @@ -948,6 +949,66 @@ describe('IndexPattern Data Source', () => { expect(ast.chain[1].arguments.timeFields).not.toContain('timefield'); }); + it('should call optimizeEsAggs once per operation for which it is available', () => { + const queryBaseState: DataViewBaseState = { + currentIndexPatternId: '1', + layers: { + first: { + indexPatternId: '1', + columns: { + col1: { + label: 'timestamp', + dataType: 'date', + operationType: 'date_histogram', + sourceField: 'timestamp', + isBucketed: true, + scale: 'interval', + params: { + interval: 'auto', + includeEmptyRows: true, + dropPartials: false, + }, + } as DateHistogramIndexPatternColumn, + col2: { + label: '95th percentile of bytes', + dataType: 'number', + operationType: 'percentile', + sourceField: 'bytes', + isBucketed: false, + scale: 'ratio', + params: { + percentile: 95, + }, + } as PercentileIndexPatternColumn, + col3: { + label: '95th percentile of bytes', + dataType: 'number', + operationType: 'percentile', + sourceField: 'bytes', + isBucketed: false, + scale: 'ratio', + params: { + percentile: 95, + }, + } as PercentileIndexPatternColumn, + }, + columnOrder: ['col1', 'col2', 'col3'], + incompleteColumns: {}, + }, + }, + }; + + const state = enrichBaseState(queryBaseState); + + const optimizeMock = jest.spyOn(operationDefinitionMap.percentile, 'optimizeEsAggs'); + + indexPatternDatasource.toExpression(state, 'first'); + + expect(operationDefinitionMap.percentile.optimizeEsAggs).toHaveBeenCalledTimes(1); + + optimizeMock.mockRestore(); + }); + describe('references', () => { beforeEach(() => { // @ts-expect-error we are inserting an invalid type From 78bfdce9b80e393de20dc88276994102155e952b Mon Sep 17 00:00:00 2001 From: Andrew Tate Date: Tue, 14 Jun 2022 13:11:37 -0500 Subject: [PATCH 13/16] test updating idMap based on new agg order --- .../indexpattern.test.ts | 74 +++++++++++++++++++ .../indexpattern_datasource/to_expression.ts | 23 +++--- 2 files changed, 85 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts index c37d238582047..1a6ed804cf21b 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts @@ -1009,6 +1009,80 @@ describe('IndexPattern Data Source', () => { optimizeMock.mockRestore(); }); + it('should update anticipated esAggs column IDs based on the order of the optimized agg expression builders', () => { + const queryBaseState: DataViewBaseState = { + currentIndexPatternId: '1', + layers: { + first: { + indexPatternId: '1', + columns: { + col1: { + label: 'timestamp', + dataType: 'date', + operationType: 'date_histogram', + sourceField: 'timestamp', + isBucketed: true, + scale: 'interval', + params: { + interval: 'auto', + includeEmptyRows: true, + dropPartials: false, + }, + } as DateHistogramIndexPatternColumn, + col2: { + label: '95th percentile of bytes', + dataType: 'number', + operationType: 'percentile', + sourceField: 'bytes', + isBucketed: false, + scale: 'ratio', + params: { + percentile: 95, + }, + } as PercentileIndexPatternColumn, + col3: { + label: 'Count of records', + dataType: 'number', + isBucketed: false, + sourceField: '___records___', + operationType: 'count', + timeScale: 'h', + }, + col4: { + label: 'Count of records2', + dataType: 'number', + isBucketed: false, + sourceField: '___records___', + operationType: 'count', + timeScale: 'h', + }, + }, + columnOrder: ['col1', 'col2', 'col3', 'col4'], + incompleteColumns: {}, + }, + }, + }; + + const state = enrichBaseState(queryBaseState); + + const optimizeMock = jest + .spyOn(operationDefinitionMap.percentile, 'optimizeEsAggs') + .mockImplementation((aggs, esAggsIdMap) => { + // change the order of the aggregations + return { aggs: aggs.reverse(), esAggsIdMap }; + }); + + const ast = indexPatternDatasource.toExpression(state, 'first') as Ast; + + expect(operationDefinitionMap.percentile.optimizeEsAggs).toHaveBeenCalledTimes(1); + + const idMap = JSON.parse(ast.chain[2].arguments.idMap as unknown as string); + + expect(Object.keys(idMap)).toEqual(['col-0-3', 'col-1-2', 'col-2-1', 'col-3-0']); + + optimizeMock.mockRestore(); + }); + describe('references', () => { beforeEach(() => { // @ts-expect-error we are inserting an invalid type diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts index d21c0498d0712..b61bfce76e4c9 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts @@ -217,17 +217,17 @@ function getExpressionForLayer( col-1-2: column3 col-3-3.98: column4 (98th percentile) */ - const newEsAggsIdToOriginalColumn: Record = {}; + + const updatedEsAggsIdMap: Record = {}; + const extractEsAggId = (id: string) => id.split('.')[0].split('-')[2]; + const updatePositionIndex = (currentId: string, newIndex: number) => { + const [fullId, percentile] = currentId.split('.'); + const idParts = fullId.split('-'); + idParts[1] = String(newIndex); + return idParts.join('-') + (percentile ? `.${percentile}` : ''); + }; let counter = 0; aggs.forEach((builder) => { - const extractEsAggId = (id: string) => id.split('.')[0].split('-')[2]; - const updatePositionIndex = (currentId: string, newIndex: number) => { - const [fullId, percentile] = currentId.split('.'); - const idParts = fullId.split('-'); - idParts[1] = String(newIndex); - return idParts.join('-') + (percentile ? `.${percentile}` : ''); - }; - const esAggId = builder.functions[0].getArgument('id')?.[0]; const matchingEsAggColumnIds = Object.keys(esAggsIdMap).filter( (id) => extractEsAggId(id) === esAggId @@ -235,12 +235,11 @@ function getExpressionForLayer( matchingEsAggColumnIds.forEach((currentId) => { const currentColumn = esAggsIdMap[currentId][0]; - // TODO what if multiple columns are mapped to, and only one is bucketed? const aggIndex = window.ELASTIC_LENS_DELAY_SECONDS ? counter + (currentColumn.isBucketed ? 0 : 1) : counter; const newId = updatePositionIndex(currentId, aggIndex); - newEsAggsIdToOriginalColumn[newId] = esAggsIdMap[currentId]; + updatedEsAggsIdMap[newId] = esAggsIdMap[currentId]; counter++; }); @@ -363,7 +362,7 @@ function getExpressionForLayer( type: 'function', function: 'lens_map_to_original_columns', arguments: { - idMap: [JSON.stringify(newEsAggsIdToOriginalColumn)], + idMap: [JSON.stringify(updatedEsAggsIdMap)], }, }, ...expressions, From 3ca5e4f317dd0ab3a817ada5e2fa1fe201d18d7b Mon Sep 17 00:00:00 2001 From: Andrew Tate Date: Thu, 16 Jun 2022 09:01:38 -0500 Subject: [PATCH 14/16] code improvements --- .../operations/definitions/index.ts | 2 +- .../operations/definitions/percentile.tsx | 21 +++++++++--------- .../indexpattern_datasource/to_expression.ts | 22 ++++++++++--------- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts index 58d2508365829..74d635cac02dc 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts @@ -383,7 +383,7 @@ interface BaseOperationDefinitionProps */ helpComponentTitle?: string; /** - * Optimizes EsAggs expression. Invoked only once per operation type. May mutate arguments. + * Optimizes EsAggs expression. Invoked only once per operation type. */ optimizeEsAggs?: ( aggs: ExpressionAstExpressionBuilder[], diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx index 12859035ed139..a313b03d34e1b 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx @@ -148,8 +148,10 @@ export const percentileOperation: OperationDefinition< } ).toAst(); }, - // TODO - there may be a way to get around including aggExpressionToEsAggsIdMap - optimizeEsAggs: (aggs, esAggsIdMap, aggExpressionToEsAggsIdMap) => { + optimizeEsAggs: (_aggs, _esAggsIdMap, aggExpressionToEsAggsIdMap) => { + let aggs = [..._aggs]; + const esAggsIdMap = { ..._esAggsIdMap }; + const percentileExpressionsByArgs: Record = {}; // group percentile dimensions by differentiating parameters @@ -171,8 +173,8 @@ export const percentileOperation: OperationDefinition< // collapse them into a single esAggs expression builder Object.values(percentileExpressionsByArgs).forEach((expressionBuilders) => { - if (!(expressionBuilders.length > 1)) { - // don't need to optimize if there's only one + if (expressionBuilders.length <= 1) { + // don't need to optimize if there aren't more than one return; } @@ -203,18 +205,15 @@ export const percentileOperation: OperationDefinition< const duplicateExpressionBuilder = percentileToBuilder[percentile]; - if ( - !aggExpressionToEsAggsIdMap.has(builder) || - !aggExpressionToEsAggsIdMap.has(duplicateExpressionBuilder) - ) { + const idForDuplicate = aggExpressionToEsAggsIdMap.get(duplicateExpressionBuilder); + const idForThisOne = aggExpressionToEsAggsIdMap.get(builder); + + if (!idForDuplicate || !idForThisOne) { throw new Error( "Couldn't find esAggs ID for percentile expression builder... this should never happen." ); } - const idForDuplicate = aggExpressionToEsAggsIdMap.get(duplicateExpressionBuilder)!; - const idForThisOne = aggExpressionToEsAggsIdMap.get(builder)!; - esAggsIdMap[idForDuplicate].push(...esAggsIdMap[idForThisOne]); delete esAggsIdMap[idForThisOne]; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts index b61bfce76e4c9..5ae053a64b123 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts @@ -38,6 +38,15 @@ declare global { } } +// esAggs column ID manipulation functions +const extractEsAggId = (id: string) => id.split('.')[0].split('-')[2]; +const updatePositionIndex = (currentId: string, newIndex: number) => { + const [fullId, percentile] = currentId.split('.'); + const idParts = fullId.split('-'); + idParts[1] = String(newIndex); + return idParts.join('-') + (percentile ? `.${percentile}` : ''); +}; + function getExpressionForLayer( layer: IndexPatternLayer, indexPattern: IndexPattern, @@ -219,19 +228,12 @@ function getExpressionForLayer( */ const updatedEsAggsIdMap: Record = {}; - const extractEsAggId = (id: string) => id.split('.')[0].split('-')[2]; - const updatePositionIndex = (currentId: string, newIndex: number) => { - const [fullId, percentile] = currentId.split('.'); - const idParts = fullId.split('-'); - idParts[1] = String(newIndex); - return idParts.join('-') + (percentile ? `.${percentile}` : ''); - }; let counter = 0; + + const esAggsIds = Object.keys(esAggsIdMap); aggs.forEach((builder) => { const esAggId = builder.functions[0].getArgument('id')?.[0]; - const matchingEsAggColumnIds = Object.keys(esAggsIdMap).filter( - (id) => extractEsAggId(id) === esAggId - ); + const matchingEsAggColumnIds = esAggsIds.filter((id) => extractEsAggId(id) === esAggId); matchingEsAggColumnIds.forEach((currentId) => { const currentColumn = esAggsIdMap[currentId][0]; From ff96f556fc46b3ec77fc4231d6654b69e8013b24 Mon Sep 17 00:00:00 2001 From: Andrew Tate Date: Mon, 20 Jun 2022 12:16:33 -0500 Subject: [PATCH 15/16] renameColumns -> mapToColumns --- .../plugins/lens/common/expressions/index.ts | 2 +- .../index.ts | 2 +- .../map_to_columns.test.ts} | 12 +++---- .../map_to_columns/map_to_columns.ts | 32 +++++++++++++++++++ .../map_to_columns_fn.ts} | 4 +-- .../types.ts | 4 +-- .../rename_columns/rename_columns.ts | 32 ------------------- x-pack/plugins/lens/public/expressions.ts | 4 +-- .../indexpattern.test.ts | 2 +- .../indexpattern_datasource/to_expression.ts | 2 +- .../lens/server/expressions/expressions.ts | 4 +-- 11 files changed, 50 insertions(+), 50 deletions(-) rename x-pack/plugins/lens/common/expressions/{rename_columns => map_to_columns}/index.ts (83%) rename x-pack/plugins/lens/common/expressions/{rename_columns/rename_columns.test.ts => map_to_columns/map_to_columns.test.ts} (95%) create mode 100644 x-pack/plugins/lens/common/expressions/map_to_columns/map_to_columns.ts rename x-pack/plugins/lens/common/expressions/{rename_columns/rename_columns_fn.ts => map_to_columns/map_to_columns_fn.ts} (90%) rename x-pack/plugins/lens/common/expressions/{rename_columns => map_to_columns}/types.ts (83%) delete mode 100644 x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.ts diff --git a/x-pack/plugins/lens/common/expressions/index.ts b/x-pack/plugins/lens/common/expressions/index.ts index 924141da6074a..ccb6343334d62 100644 --- a/x-pack/plugins/lens/common/expressions/index.ts +++ b/x-pack/plugins/lens/common/expressions/index.ts @@ -8,6 +8,6 @@ export * from './counter_rate'; export * from './collapse'; export * from './format_column'; -export * from './rename_columns'; +export * from './map_to_columns'; export * from './time_scale'; export * from './datatable'; diff --git a/x-pack/plugins/lens/common/expressions/rename_columns/index.ts b/x-pack/plugins/lens/common/expressions/map_to_columns/index.ts similarity index 83% rename from x-pack/plugins/lens/common/expressions/rename_columns/index.ts rename to x-pack/plugins/lens/common/expressions/map_to_columns/index.ts index 86ab16e06ec01..8ce71d06f6579 100644 --- a/x-pack/plugins/lens/common/expressions/rename_columns/index.ts +++ b/x-pack/plugins/lens/common/expressions/map_to_columns/index.ts @@ -5,4 +5,4 @@ * 2.0. */ -export { renameColumns } from './rename_columns'; +export { mapToColumns } from './map_to_columns'; diff --git a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.test.ts b/x-pack/plugins/lens/common/expressions/map_to_columns/map_to_columns.test.ts similarity index 95% rename from x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.test.ts rename to x-pack/plugins/lens/common/expressions/map_to_columns/map_to_columns.test.ts index 59b06a6980435..e5d678b88e5a5 100644 --- a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.test.ts +++ b/x-pack/plugins/lens/common/expressions/map_to_columns/map_to_columns.test.ts @@ -5,11 +5,11 @@ * 2.0. */ -import { renameColumns } from './rename_columns'; +import { mapToColumns } from './map_to_columns'; import { Datatable } from '@kbn/expressions-plugin/common'; import { createMockExecutionContext } from '@kbn/expressions-plugin/common/mocks'; -describe('rename_columns', () => { +describe('map_to_columns', () => { it('should rename columns of a given datatable', async () => { const input: Datatable = { type: 'datatable', @@ -40,7 +40,7 @@ describe('rename_columns', () => { ], }; - const result = await renameColumns.fn( + const result = await mapToColumns.fn( input, { idMap: JSON.stringify(idMap) }, createMockExecutionContext() @@ -106,7 +106,7 @@ describe('rename_columns', () => { b: [{ id: 'c', label: 'Catamaran' }], }; - const result = await renameColumns.fn( + const result = await mapToColumns.fn( input, { idMap: JSON.stringify(idMap) }, createMockExecutionContext() @@ -167,7 +167,7 @@ describe('rename_columns', () => { ], }; - const result = await renameColumns.fn( + const result = await mapToColumns.fn( input, { idMap: JSON.stringify(idMap) }, createMockExecutionContext() @@ -233,7 +233,7 @@ describe('rename_columns', () => { b: [{ id: 'c', label: 'Apple', operationType: 'date_histogram', sourceField: 'banana' }], }; - const result = await renameColumns.fn( + const result = await mapToColumns.fn( input, { idMap: JSON.stringify(idMap) }, createMockExecutionContext() diff --git a/x-pack/plugins/lens/common/expressions/map_to_columns/map_to_columns.ts b/x-pack/plugins/lens/common/expressions/map_to_columns/map_to_columns.ts new file mode 100644 index 0000000000000..3315cd4170dd9 --- /dev/null +++ b/x-pack/plugins/lens/common/expressions/map_to_columns/map_to_columns.ts @@ -0,0 +1,32 @@ +/* + * 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 { i18n } from '@kbn/i18n'; +import type { MapToColumnsExpressionFunction } from './types'; + +export const mapToColumns: MapToColumnsExpressionFunction = { + name: 'lens_map_to_columns', + type: 'datatable', + help: i18n.translate('xpack.lens.functions.mapToColumns.help', { + defaultMessage: 'A helper to transform a datatable to match Lens column definitions', + }), + args: { + idMap: { + types: ['string'], + help: i18n.translate('xpack.lens.functions.mapToColumns.idMap.help', { + defaultMessage: + 'A JSON encoded object in which keys are the datatable column ids and values are the Lens column definitions. Any datatable columns not mentioned within the ID map will be kept unmapped.', + }), + }, + }, + inputTypes: ['datatable'], + async fn(...args) { + /** Build optimization: prevent adding extra code into initial bundle **/ + const { mapToOriginalColumns } = await import('./map_to_columns_fn'); + return mapToOriginalColumns(...args); + }, +}; diff --git a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns_fn.ts b/x-pack/plugins/lens/common/expressions/map_to_columns/map_to_columns_fn.ts similarity index 90% rename from x-pack/plugins/lens/common/expressions/rename_columns/rename_columns_fn.ts rename to x-pack/plugins/lens/common/expressions/map_to_columns/map_to_columns_fn.ts index 793e6183d8157..401051db71065 100644 --- a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns_fn.ts +++ b/x-pack/plugins/lens/common/expressions/map_to_columns/map_to_columns_fn.ts @@ -6,7 +6,7 @@ */ import type { DatatableColumn } from '@kbn/expressions-plugin/common'; -import type { OriginalColumn, MapToOriginalColumnsExpressionFunction } from './types'; +import type { OriginalColumn, MapToColumnsExpressionFunction } from './types'; function getColumnName(originalColumn: OriginalColumn, newColumn: DatatableColumn) { if (originalColumn?.operationType === 'date_histogram') { @@ -21,7 +21,7 @@ function getColumnName(originalColumn: OriginalColumn, newColumn: DatatableColum return originalColumn.label; } -export const mapToOriginalColumns: MapToOriginalColumnsExpressionFunction['fn'] = ( +export const mapToOriginalColumns: MapToColumnsExpressionFunction['fn'] = ( data, { idMap: encodedIdMap } ) => { diff --git a/x-pack/plugins/lens/common/expressions/rename_columns/types.ts b/x-pack/plugins/lens/common/expressions/map_to_columns/types.ts similarity index 83% rename from x-pack/plugins/lens/common/expressions/rename_columns/types.ts rename to x-pack/plugins/lens/common/expressions/map_to_columns/types.ts index f10e6bda35a3b..0c99260b704b1 100644 --- a/x-pack/plugins/lens/common/expressions/rename_columns/types.ts +++ b/x-pack/plugins/lens/common/expressions/map_to_columns/types.ts @@ -12,8 +12,8 @@ export type OriginalColumn = { id: string; label: string } & ( | { operationType: string; sourceField: never } ); -export type MapToOriginalColumnsExpressionFunction = ExpressionFunctionDefinition< - 'lens_map_to_original_columns', +export type MapToColumnsExpressionFunction = ExpressionFunctionDefinition< + 'lens_map_to_columns', Datatable, { idMap: string; diff --git a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.ts b/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.ts deleted file mode 100644 index 373390d51d2cf..0000000000000 --- a/x-pack/plugins/lens/common/expressions/rename_columns/rename_columns.ts +++ /dev/null @@ -1,32 +0,0 @@ -/* - * 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 { i18n } from '@kbn/i18n'; -import type { MapToOriginalColumnsExpressionFunction } from './types'; - -export const renameColumns: MapToOriginalColumnsExpressionFunction = { - name: 'lens_map_to_original_columns', - type: 'datatable', - help: i18n.translate('xpack.lens.functions.renameColumns.help', { - defaultMessage: 'A helper to rename the columns of a datatable', - }), - args: { - idMap: { - types: ['string'], - help: i18n.translate('xpack.lens.functions.renameColumns.idMap.help', { - defaultMessage: - 'A JSON encoded object in which keys are the old column ids and values are the corresponding new ones. All other columns ids are kept.', - }), - }, - }, - inputTypes: ['datatable'], - async fn(...args) { - /** Build optimization: prevent adding extra code into initial bundle **/ - const { mapToOriginalColumns } = await import('./rename_columns_fn'); - return mapToOriginalColumns(...args); - }, -}; diff --git a/x-pack/plugins/lens/public/expressions.ts b/x-pack/plugins/lens/public/expressions.ts index 868d947464e5f..a5f193d63e4f3 100644 --- a/x-pack/plugins/lens/public/expressions.ts +++ b/x-pack/plugins/lens/public/expressions.ts @@ -8,7 +8,7 @@ import type { ExpressionsSetup } from '@kbn/expressions-plugin/public'; import { getDatatable } from '../common/expressions/datatable/datatable'; import { datatableColumn } from '../common/expressions/datatable/datatable_column'; -import { renameColumns } from '../common/expressions/rename_columns/rename_columns'; +import { mapToColumns } from '../common/expressions/map_to_columns/map_to_columns'; import { formatColumn } from '../common/expressions/format_column'; import { counterRate } from '../common/expressions/counter_rate'; import { getTimeScale } from '../common/expressions/time_scale/time_scale'; @@ -24,7 +24,7 @@ export const setupExpressions = ( collapse, counterRate, formatColumn, - renameColumns, + mapToColumns, datatableColumn, getDatatable(formatFactory), getTimeScale(getDatatableUtilities, getTimeZone), diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts index 1a6ed804cf21b..a37976f6d8069 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts @@ -495,7 +495,7 @@ describe('IndexPattern Data Source', () => { "{\\"col-0-0\\":[{\\"label\\":\\"Count of records\\",\\"dataType\\":\\"number\\",\\"isBucketed\\":false,\\"sourceField\\":\\"___records___\\",\\"operationType\\":\\"count\\",\\"id\\":\\"col1\\"}],\\"col-1-1\\":[{\\"label\\":\\"Date\\",\\"dataType\\":\\"date\\",\\"isBucketed\\":true,\\"operationType\\":\\"date_histogram\\",\\"sourceField\\":\\"timestamp\\",\\"params\\":{\\"interval\\":\\"1d\\"},\\"id\\":\\"col2\\"}]}", ], }, - "function": "lens_map_to_original_columns", + "function": "lens_map_to_columns", "type": "function", }, ], diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts index 5ae053a64b123..6cfab965f36a9 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts @@ -362,7 +362,7 @@ function getExpressionForLayer( }).toAst(), { type: 'function', - function: 'lens_map_to_original_columns', + function: 'lens_map_to_columns', arguments: { idMap: [JSON.stringify(updatedEsAggsIdMap)], }, diff --git a/x-pack/plugins/lens/server/expressions/expressions.ts b/x-pack/plugins/lens/server/expressions/expressions.ts index bd516c756df15..1e80fc5bb49a3 100644 --- a/x-pack/plugins/lens/server/expressions/expressions.ts +++ b/x-pack/plugins/lens/server/expressions/expressions.ts @@ -10,7 +10,7 @@ import type { ExpressionsServerSetup } from '@kbn/expressions-plugin/server'; import { counterRate, formatColumn, - renameColumns, + mapToColumns, getTimeScale, getDatatable, } from '../../common/expressions'; @@ -25,7 +25,7 @@ export const setupExpressions = ( [ counterRate, formatColumn, - renameColumns, + mapToColumns, getDatatable(getFormatFactory(core)), getTimeScale(getDatatableUtilitiesFactory(core), getTimeZoneFactory(core)), ].forEach((expressionFn) => expressions.registerFunction(expressionFn)); From c67dd72e9fef613ce5af259cc0c9eee8aabfac54 Mon Sep 17 00:00:00 2001 From: Andrew Tate Date: Mon, 20 Jun 2022 13:58:04 -0500 Subject: [PATCH 16/16] remove unused translations --- x-pack/plugins/translations/translations/fr-FR.json | 2 -- x-pack/plugins/translations/translations/ja-JP.json | 2 -- x-pack/plugins/translations/translations/zh-CN.json | 2 -- 3 files changed, 6 deletions(-) diff --git a/x-pack/plugins/translations/translations/fr-FR.json b/x-pack/plugins/translations/translations/fr-FR.json index b2f095bd838cb..9ea2b89fb6ab8 100644 --- a/x-pack/plugins/translations/translations/fr-FR.json +++ b/x-pack/plugins/translations/translations/fr-FR.json @@ -365,8 +365,6 @@ "xpack.lens.functions.counterRate.args.outputColumnNameHelpText": "Nom de la colonne dans laquelle le taux de compteur résultant sera stocké", "xpack.lens.functions.counterRate.help": "Calcule le taux de compteur d'une colonne dans un tableau de données", "xpack.lens.functions.lastValue.missingSortField": "Cette vue de données ne contient aucun champ de date.", - "xpack.lens.functions.renameColumns.help": "Aide pour renommer les colonnes d'un tableau de données", - "xpack.lens.functions.renameColumns.idMap.help": "Un objet encodé JSON dans lequel les clés sont les anciens ID de colonne et les valeurs sont les nouveaux ID correspondants. Tous les autres ID de colonne sont conservés.", "xpack.lens.functions.timeScale.dateColumnMissingMessage": "L'ID de colonne de date {columnId} n'existe pas.", "xpack.lens.functions.timeScale.timeInfoMissingMessage": "Impossible de récupérer les informations d'histogramme des dates", "xpack.lens.gauge.addLayer": "Visualisation", diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index fecfabdb7b16a..fd5158ef608ae 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -367,8 +367,6 @@ "xpack.lens.functions.counterRate.args.outputColumnNameHelpText": "結果のカウンターレートを格納する列の名前", "xpack.lens.functions.counterRate.help": "データテーブルの列のカウンターレートを計算します", "xpack.lens.functions.lastValue.missingSortField": "このデータビューには日付フィールドが含まれていません", - "xpack.lens.functions.renameColumns.help": "データベースの列の名前の変更をアシストします", - "xpack.lens.functions.renameColumns.idMap.help": "キーが古い列 ID で値が対応する新しい列 ID となるように JSON エンコーディングされたオブジェクトです。他の列 ID はすべてのそのままです。", "xpack.lens.functions.timeScale.dateColumnMissingMessage": "指定した dateColumnId {columnId} は存在しません。", "xpack.lens.functions.timeScale.timeInfoMissingMessage": "日付ヒストグラム情報を取得できませんでした", "xpack.lens.gauge.addLayer": "ビジュアライゼーション", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 1cc41035906d5..d1b184244e9fb 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -372,8 +372,6 @@ "xpack.lens.functions.counterRate.args.outputColumnNameHelpText": "要存储结果计数率的列名称", "xpack.lens.functions.counterRate.help": "在数据表中计算列的计数率", "xpack.lens.functions.lastValue.missingSortField": "此数据视图不包含任何日期字段", - "xpack.lens.functions.renameColumns.help": "用于重命名数据表列的助手", - "xpack.lens.functions.renameColumns.idMap.help": "旧列 ID 为键且相应新列 ID 为值的 JSON 编码对象。所有其他列 ID 都将保留。", "xpack.lens.functions.timeScale.dateColumnMissingMessage": "指定的 dateColumnId {columnId} 不存在。", "xpack.lens.functions.timeScale.timeInfoMissingMessage": "无法获取日期直方图信息", "xpack.lens.gauge.addLayer": "可视化",