Skip to content

Commit 468cfcf

Browse files
wylieconlonwylieconlon
committed
Add an extra case to prevent insertion of duplicate column
1 parent 5413d7a commit 468cfcf

4 files changed

Lines changed: 60 additions & 38 deletions

File tree

‎src/plugins/expressions/common/expression_functions/specs/map_column.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ export const mapColumn: ExpressionFunctionDefinition<
5353
types: ['string'],
5454
aliases: ['_', 'column'],
5555
help: i18n.translate('expressions.functions.mapColumn.args.nameHelpText', {
56-
defaultMessage: 'The name of the resulting column.',
56+
defaultMessage: 'The name of the resulting column. Names are not required to be unique.',
5757
}),
5858
required: true,
5959
},
@@ -102,8 +102,11 @@ export const mapColumn: ExpressionFunctionDefinition<
102102

103103
return Promise.all(rowPromises).then((rows) => {
104104
const existingColumnIndex = columns.findIndex(({ id, name }) => {
105-
// Columns that have IDs are allowed to have duplicate names, for example esaggs
106105
if (args.id) {
106+
if (!id) {
107+
return name === args.id;
108+
}
109+
// Columns that have IDs are allowed to have duplicate names, for example esaggs
107110
return id === args.id;
108111
}
109112
// If the column has an ID, but there is no ID argument to mapColumn

‎src/plugins/expressions/common/expression_functions/specs/tests/map_column.test.ts‎

Lines changed: 55 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -35,45 +35,66 @@ describe('mapColumn', () => {
3535
expect(result.rows[arbitraryRowIndex]).toHaveProperty('pricePlusTwo');
3636
});
3737

38-
it('matches name to id when mapColumn is called without an id', async () => {
39-
const result = await runFn(testTable, { name: 'name', expression: pricePlusTwo });
40-
const nameColumnIndex = result.columns.findIndex(({ name }) => name === 'name');
41-
const arbitraryRowIndex = 4;
42-
43-
expect(result.type).toBe('datatable');
44-
expect(result.columns).toHaveLength(sqlTable.columns.length);
45-
expect(result.columns[nameColumnIndex]).toHaveProperty('name', 'name');
46-
expect(result.columns[nameColumnIndex].meta).toHaveProperty('type', 'number');
47-
expect(result.rows[arbitraryRowIndex]).toHaveProperty('name', 202);
48-
});
49-
50-
it('overwrites existing column with the new column if an existing column name is missing an id', async () => {
51-
const result = await runFn(sqlTable, { name: 'name', expression: pricePlusTwo });
52-
const nameColumnIndex = result.columns.findIndex(({ name }) => name === 'name');
53-
const arbitraryRowIndex = 4;
38+
describe('when the table columns have id', () => {
39+
it('does not require the id arg by using the name arg as column id', async () => {
40+
const result = await runFn(testTable, { name: 'name', expression: pricePlusTwo });
41+
const nameColumnIndex = result.columns.findIndex(({ name }) => name === 'name');
42+
const arbitraryRowIndex = 4;
43+
44+
expect(result.type).toBe('datatable');
45+
expect(result.columns).toHaveLength(sqlTable.columns.length);
46+
expect(result.columns[nameColumnIndex]).toHaveProperty('name', 'name');
47+
expect(result.columns[nameColumnIndex].meta).toHaveProperty('type', 'number');
48+
expect(result.rows[arbitraryRowIndex]).toHaveProperty('name', 202);
49+
});
5450

55-
expect(result.type).toBe('datatable');
56-
expect(result.columns).toHaveLength(sqlTable.columns.length);
57-
expect(result.columns[nameColumnIndex]).toHaveProperty('name', 'name');
58-
expect(result.columns[nameColumnIndex].meta).toHaveProperty('type', 'number');
59-
expect(result.rows[arbitraryRowIndex]).toHaveProperty('name', 202);
51+
it('allows a duplicate name when the ids are different', async () => {
52+
const result = await runFn(testTable, {
53+
id: 'new',
54+
name: 'name label',
55+
expression: pricePlusTwo,
56+
});
57+
const nameColumnIndex = result.columns.findIndex(({ id }) => id === 'new');
58+
const arbitraryRowIndex = 4;
59+
60+
expect(result.type).toBe('datatable');
61+
expect(result.columns).toHaveLength(testTable.columns.length + 1);
62+
expect(result.columns[nameColumnIndex]).toHaveProperty('id', 'new');
63+
expect(result.columns[nameColumnIndex]).toHaveProperty('name', 'name label');
64+
expect(result.columns[nameColumnIndex].meta).toHaveProperty('type', 'number');
65+
expect(result.rows[arbitraryRowIndex]).toHaveProperty('new', 202);
66+
});
6067
});
6168

62-
it('inserts a new column with a duplicate name if an id and name are provided', async () => {
63-
const result = await runFn(testTable, {
64-
id: 'new',
65-
name: 'name label',
66-
expression: pricePlusTwo,
69+
describe('when the table columns do not have id', () => {
70+
it('uses name as unique key when id arg is also missing', async () => {
71+
const result = await runFn(sqlTable, { name: 'name', expression: pricePlusTwo });
72+
const nameColumnIndex = result.columns.findIndex(({ name }) => name === 'name');
73+
const arbitraryRowIndex = 4;
74+
75+
expect(result.type).toBe('datatable');
76+
expect(result.columns).toHaveLength(sqlTable.columns.length);
77+
expect(result.columns[nameColumnIndex]).toHaveProperty('name', 'name');
78+
expect(result.columns[nameColumnIndex].meta).toHaveProperty('type', 'number');
79+
expect(result.rows[arbitraryRowIndex]).toHaveProperty('name', 202);
6780
});
68-
const nameColumnIndex = result.columns.findIndex(({ id }) => id === 'new');
69-
const arbitraryRowIndex = 4;
7081

71-
expect(result.type).toBe('datatable');
72-
expect(result.columns).toHaveLength(testTable.columns.length + 1);
73-
expect(result.columns[nameColumnIndex]).toHaveProperty('id', 'new');
74-
expect(result.columns[nameColumnIndex]).toHaveProperty('name', 'name label');
75-
expect(result.columns[nameColumnIndex].meta).toHaveProperty('type', 'number');
76-
expect(result.rows[arbitraryRowIndex]).toHaveProperty('new', 202);
82+
it('overwrites columns matching id === name when the column is missing an id', async () => {
83+
const result = await runFn(sqlTable, {
84+
id: 'name',
85+
name: 'name is ignored',
86+
expression: pricePlusTwo,
87+
});
88+
const nameColumnIndex = result.columns.findIndex(({ name }) => name === 'name is ignored');
89+
const arbitraryRowIndex = 4;
90+
91+
expect(result.type).toBe('datatable');
92+
expect(result.columns).toHaveLength(sqlTable.columns.length);
93+
expect(result.columns[nameColumnIndex]).toHaveProperty('id', 'name');
94+
expect(result.columns[nameColumnIndex]).toHaveProperty('name', 'name is ignored');
95+
expect(result.columns[nameColumnIndex].meta).toHaveProperty('type', 'number');
96+
expect(result.rows[arbitraryRowIndex]).toHaveProperty('name', 202);
97+
});
7798
});
7899

79100
it('adds a column to empty tables', async () => {

‎x-pack/plugins/translations/translations/ja-JP.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2022,7 +2022,6 @@
20222022
"expressions.functions.mapColumn.args.copyMetaFromHelpText": "設定されている場合、指定した列IDのメタオブジェクトが指定したターゲット列にコピーされます。列が存在しない場合は失敗し、エラーは表示されません。",
20232023
"expressions.functions.mapColumn.args.expressionHelpText": "すべての行で実行される式。単一行の{DATATABLE}と一緒に指定され、セル値を返します。",
20242024
"expressions.functions.mapColumn.args.idHelpText": "結果列の任意のID。「null」の場合、name/column引数がIDとして使用されます。",
2025-
"expressions.functions.mapColumn.args.nameHelpText": "結果の列の名前です。",
20262025
"expressions.functions.mapColumnHelpText": "他の列の結果として計算された列を追加します。引数が指定された場合のみ変更が加えられます。{alterColumnFn}と{staticColumnFn}もご参照ください。",
20272026
"expressions.functions.math.args.expressionHelpText": "評価された {TINYMATH} 表現です。{TINYMATH_URL} をご覧ください。",
20282027
"expressions.functions.math.args.onErrorHelpText": "{TINYMATH}評価が失敗するか、NaNが返される場合、戻り値はonErrorで指定されます。「'throw'」の場合、例外が発生し、式の実行が終了します (デフォルト) 。",

‎x-pack/plugins/translations/translations/zh-CN.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2034,7 +2034,6 @@
20342034
"expressions.functions.mapColumn.args.copyMetaFromHelpText": "如果设置，指定列 ID 的元对象将复制到指定目标列。如果列不存在，复制将无提示失败。",
20352035
"expressions.functions.mapColumn.args.expressionHelpText": "在每行上执行的表达式，为其提供了单行 {DATATABLE} 上下文，其将返回单元格值。",
20362036
"expressions.functions.mapColumn.args.idHelpText": "结果列的可选 ID。如果为 `null`，名称/列参数将用作 ID。",
2037-
"expressions.functions.mapColumn.args.nameHelpText": "结果列的名称。",
20382037
"expressions.functions.mapColumnHelpText": "添加计算为其他列的结果的列。只有提供参数时，才会执行更改。另请参见 {alterColumnFn} 和 {staticColumnFn}。",
20392038
"expressions.functions.math.args.expressionHelpText": "已计算的 {TINYMATH} 表达式。请参阅 {TINYMATH_URL}。",
20402039
"expressions.functions.math.args.onErrorHelpText": "如果 {TINYMATH} 评估失败或返回 NaN，返回值将由 onError 指定。为 `'throw'` 时，其将引发异常，从而终止表达式执行 (默认) 。",

0 commit comments

Comments
 (0)