diff --git a/src/plugins/expressions/common/expression_functions/specs/map_column.ts b/src/plugins/expressions/common/expression_functions/specs/map_column.ts index c570206670dde5..7939441ff0d602 100644 --- a/src/plugins/expressions/common/expression_functions/specs/map_column.ts +++ b/src/plugins/expressions/common/expression_functions/specs/map_column.ts @@ -44,7 +44,7 @@ export const mapColumn: ExpressionFunctionDefinition< types: ['string', 'null'], help: i18n.translate('expressions.functions.mapColumn.args.idHelpText', { defaultMessage: - 'An optional id of the resulting column. When `null` the name/column argument is used as id.', + 'An optional id of the resulting column. When no id is provided, the id will be looked up from the existing column by the provided name argument. If no column with this name exists yet, a new column with this name and an identical id will be added to the table.', }), required: false, default: null, @@ -53,7 +53,7 @@ export const mapColumn: ExpressionFunctionDefinition< types: ['string'], aliases: ['_', 'column'], help: i18n.translate('expressions.functions.mapColumn.args.nameHelpText', { - defaultMessage: 'The name of the resulting column.', + defaultMessage: 'The name of the resulting column. Names are not required to be unique.', }), required: true, }, @@ -86,9 +86,17 @@ export const mapColumn: ExpressionFunctionDefinition< .expression?.(...params) .pipe(take(1)) .toPromise() ?? Promise.resolve(null); - const columnId = args.id != null ? args.id : args.name; const columns = [...input.columns]; + const existingColumnIndex = columns.findIndex(({ id, name }) => { + if (args.id) { + return id === args.id; + } + return name === args.name; + }); + const columnId = + existingColumnIndex === -1 ? args.id ?? args.name : columns[existingColumnIndex].id; + const rowPromises = input.rows.map((row) => { return expression({ type: 'datatable', @@ -101,7 +109,6 @@ export const mapColumn: ExpressionFunctionDefinition< }); return Promise.all(rowPromises).then((rows) => { - const existingColumnIndex = columns.findIndex(({ name }) => name === args.name); const type = rows.length ? getType(rows[0][columnId]) : 'null'; const newColumn = { id: columnId, diff --git a/src/plugins/expressions/common/expression_functions/specs/math.ts b/src/plugins/expressions/common/expression_functions/specs/math.ts index a70c032769b570..92a10976428a36 100644 --- a/src/plugins/expressions/common/expression_functions/specs/math.ts +++ b/src/plugins/expressions/common/expression_functions/specs/math.ts @@ -130,10 +130,11 @@ export const math: ExpressionFunctionDefinition< throw errors.emptyExpression(); } + // Use unique ID if available, otherwise fall back to names const mathContext = isDatatable(input) ? pivotObjectArray( input.rows, - input.columns.map((col) => col.name) + input.columns.map((col) => col.id) ) : { value: input }; diff --git a/src/plugins/expressions/common/expression_functions/specs/tests/map_column.test.ts b/src/plugins/expressions/common/expression_functions/specs/tests/map_column.test.ts index b2966b010b4790..f5c1f3838f66c7 100644 --- a/src/plugins/expressions/common/expression_functions/specs/tests/map_column.test.ts +++ b/src/plugins/expressions/common/expression_functions/specs/tests/map_column.test.ts @@ -35,16 +35,35 @@ describe('mapColumn', () => { expect(result.rows[arbitraryRowIndex]).toHaveProperty('pricePlusTwo'); }); - it('overwrites existing column with the new column if an existing column name is provided', async () => { - const result = await runFn(testTable, { name: 'name', expression: pricePlusTwo }); - const nameColumnIndex = result.columns.findIndex(({ name }) => name === 'name'); + it('allows the id arg to be optional, looking up by name instead', async () => { + const result = await runFn(testTable, { name: 'name label', expression: pricePlusTwo }); + const nameColumnIndex = result.columns.findIndex(({ name }) => name === 'name label'); const arbitraryRowIndex = 4; expect(result.type).toBe('datatable'); expect(result.columns).toHaveLength(testTable.columns.length); - expect(result.columns[nameColumnIndex]).toHaveProperty('name', 'name'); + expect(result.columns[nameColumnIndex]).toHaveProperty('id', 'name'); + expect(result.columns[nameColumnIndex]).toHaveProperty('name', 'name label'); expect(result.columns[nameColumnIndex].meta).toHaveProperty('type', 'number'); expect(result.rows[arbitraryRowIndex]).toHaveProperty('name', 202); + expect(result.rows[arbitraryRowIndex]).not.toHaveProperty('name label'); + }); + + it('allows a duplicate name when the ids are different', async () => { + const result = await runFn(testTable, { + id: 'new', + name: 'name label', + expression: pricePlusTwo, + }); + const nameColumnIndex = result.columns.findIndex(({ id }) => id === 'new'); + const arbitraryRowIndex = 4; + + expect(result.type).toBe('datatable'); + expect(result.columns).toHaveLength(testTable.columns.length + 1); + expect(result.columns[nameColumnIndex]).toHaveProperty('id', 'new'); + expect(result.columns[nameColumnIndex]).toHaveProperty('name', 'name label'); + expect(result.columns[nameColumnIndex].meta).toHaveProperty('type', 'number'); + expect(result.rows[arbitraryRowIndex]).toHaveProperty('new', 202); }); it('adds a column to empty tables', async () => { @@ -66,18 +85,6 @@ describe('mapColumn', () => { expect(result.columns[0].meta).toHaveProperty('type', 'null'); }); - it('should assign specific id, different from name, when id arg is passed for copied column', async () => { - const result = await runFn(testTable, { name: 'name', id: 'myid', expression: pricePlusTwo }); - const nameColumnIndex = result.columns.findIndex(({ name }) => name === 'name'); - - expect(result.type).toBe('datatable'); - expect(result.columns[nameColumnIndex]).toEqual({ - id: 'myid', - name: 'name', - meta: { type: 'number' }, - }); - }); - it('should copy over the meta information from the specified column', async () => { const result = await runFn( { diff --git a/src/plugins/expressions/common/expression_functions/specs/tests/math.test.ts b/src/plugins/expressions/common/expression_functions/specs/tests/math.test.ts index 7541852cdbdaff..6da00061244da8 100644 --- a/src/plugins/expressions/common/expression_functions/specs/tests/math.test.ts +++ b/src/plugins/expressions/common/expression_functions/specs/tests/math.test.ts @@ -27,7 +27,7 @@ describe('math', () => { expect(fn(-103, { expression: 'abs(value)' })).toBe(103); }); - it('evaluates math expressions with references to columns in a datatable', () => { + it('evaluates math expressions with references to columns by id in a datatable', () => { expect(fn(testTable, { expression: 'unique(in_stock)' })).toBe(2); expect(fn(testTable, { expression: 'sum(quantity)' })).toBe(2508); expect(fn(testTable, { expression: 'mean(price)' })).toBe(320); @@ -36,6 +36,21 @@ describe('math', () => { expect(fn(testTable, { expression: 'max(price)' })).toBe(605); }); + it('does not use the name for math', () => { + expect(() => fn(testTable, { expression: 'unique("in_stock label")' })).toThrow( + 'Unknown variable' + ); + expect(() => fn(testTable, { expression: 'sum("quantity label")' })).toThrow( + 'Unknown variable' + ); + expect(() => fn(testTable, { expression: 'mean("price label")' })).toThrow('Unknown variable'); + expect(() => fn(testTable, { expression: 'min("price label")' })).toThrow('Unknown variable'); + expect(() => fn(testTable, { expression: 'median("quantity label")' })).toThrow( + 'Unknown variable' + ); + expect(() => fn(testTable, { expression: 'max("price label")' })).toThrow('Unknown variable'); + }); + describe('args', () => { describe('expression', () => { it('sets the math expression to be evaluted', () => { diff --git a/src/plugins/expressions/common/expression_functions/specs/tests/utils.ts b/src/plugins/expressions/common/expression_functions/specs/tests/utils.ts index 7369570cf2c4bc..60d22d2b8575cb 100644 --- a/src/plugins/expressions/common/expression_functions/specs/tests/utils.ts +++ b/src/plugins/expressions/common/expression_functions/specs/tests/utils.ts @@ -37,27 +37,27 @@ const testTable: Datatable = { columns: [ { id: 'name', - name: 'name', + name: 'name label', meta: { type: 'string' }, }, { id: 'time', - name: 'time', + name: 'time label', meta: { type: 'date' }, }, { id: 'price', - name: 'price', + name: 'price label', meta: { type: 'number' }, }, { id: 'quantity', - name: 'quantity', + name: 'quantity label', meta: { type: 'number' }, }, { id: 'in_stock', - name: 'in_stock', + name: 'in_stock label', meta: { type: 'boolean' }, }, ], diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index 56548ea7602a73..8933d2daddbf4e 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -1891,8 +1891,6 @@ "expressions.functions.fontHelpText": "フォントスタイルを作成します。", "expressions.functions.mapColumn.args.copyMetaFromHelpText": "設定されている場合、指定した列IDのメタオブジェクトが指定したターゲット列にコピーされます。列が存在しない場合は失敗し、エラーは表示されません。", "expressions.functions.mapColumn.args.expressionHelpText": "すべての行で実行される式。単一行の{DATATABLE}と一緒に指定され、セル値を返します。", - "expressions.functions.mapColumn.args.idHelpText": "結果列の任意のID。「null」の場合、name/column引数がIDとして使用されます。", - "expressions.functions.mapColumn.args.nameHelpText": "結果の列の名前です。", "expressions.functions.mapColumnHelpText": "他の列の結果として計算された列を追加します。引数が指定された場合のみ変更が加えられます。{alterColumnFn}と{staticColumnFn}もご参照ください。", "expressions.functions.math.args.expressionHelpText": "評価された {TINYMATH} 表現です。{TINYMATH_URL} をご覧ください。", "expressions.functions.math.args.onErrorHelpText": "{TINYMATH}評価が失敗するか、NaNが返される場合、戻り値はonErrorで指定されます。「'throw'」の場合、例外が発生し、式の実行が終了します (デフォルト) 。", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index c8802633a5556d..db41cc2e2d2842 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -1903,8 +1903,6 @@ "expressions.functions.fontHelpText": "创建字体样式。", "expressions.functions.mapColumn.args.copyMetaFromHelpText": "如果设置,指定列 ID 的元对象将复制到指定目标列。如果列不存在,复制将无提示失败。", "expressions.functions.mapColumn.args.expressionHelpText": "在每行上执行的表达式,为其提供了单行 {DATATABLE} 上下文,其将返回单元格值。", - "expressions.functions.mapColumn.args.idHelpText": "结果列的可选 ID。如果为 `null`,名称/列参数将用作 ID。", - "expressions.functions.mapColumn.args.nameHelpText": "结果列的名称。", "expressions.functions.mapColumnHelpText": "添加计算为其他列的结果的列。只有提供参数时,才会执行更改。另请参见 {alterColumnFn} 和 {staticColumnFn}。", "expressions.functions.math.args.expressionHelpText": "已计算的 {TINYMATH} 表达式。请参阅 {TINYMATH_URL}。", "expressions.functions.math.args.onErrorHelpText": "如果 {TINYMATH} 评估失败或返回 NaN,返回值将由 onError 指定。为 `'throw'` 时,其将引发异常,从而终止表达式执行 (默认) 。",