Add an extra case to prevent insertion of duplicate column

elastic · May 18, 2021 · 468cfcf · 468cfcf
1 parent 5413d7a
commit 468cfcf
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 38 deletions.
diff --git a/src/plugins/expressions/common/expression_functions/specs/map_column.ts b/src/plugins/expressions/common/expression_functions/specs/map_column.ts
@@ -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,
  },
@@ -102,8 +102,11 @@ export const mapColumn: ExpressionFunctionDefinition<
 
  return Promise.all(rowPromises).then((rows) => {
  const existingColumnIndex = columns.findIndex(({ id, name }) => {
- // Columns that have IDs are allowed to have duplicate names, for example esaggs
  if (args.id) {
+ if (!id) {
+ return name === args.id;
+ }
+ // Columns that have IDs are allowed to have duplicate names, for example esaggs
  return id === args.id;
  }
  // If the column has an ID, but there is no ID argument to mapColumn

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
@@ -35,45 +35,66 @@ describe('mapColumn', () => {
  expect(result.rows[arbitraryRowIndex]).toHaveProperty('pricePlusTwo');
  });
 
- it('matches name to id when mapColumn is called without an id', async () => {
- const result = await runFn(testTable, { name: 'name', expression: pricePlusTwo });
- const nameColumnIndex = result.columns.findIndex(({ name }) => name === 'name');
- const arbitraryRowIndex = 4;
-
- expect(result.type).toBe('datatable');
- expect(result.columns).toHaveLength(sqlTable.columns.length);
- expect(result.columns[nameColumnIndex]).toHaveProperty('name', 'name');
- expect(result.columns[nameColumnIndex].meta).toHaveProperty('type', 'number');
- expect(result.rows[arbitraryRowIndex]).toHaveProperty('name', 202);
- });
-
- it('overwrites existing column with the new column if an existing column name is missing an id', async () => {
- const result = await runFn(sqlTable, { name: 'name', expression: pricePlusTwo });
- const nameColumnIndex = result.columns.findIndex(({ name }) => name === 'name');
- const arbitraryRowIndex = 4;
+ describe('when the table columns have id', () => {
+ it('does not require the id arg by using the name arg as column id', async () => {
+ const result = await runFn(testTable, { name: 'name', expression: pricePlusTwo });
+ const nameColumnIndex = result.columns.findIndex(({ name }) => name === 'name');
+ const arbitraryRowIndex = 4;
+
+ expect(result.type).toBe('datatable');
+ expect(result.columns).toHaveLength(sqlTable.columns.length);
+ expect(result.columns[nameColumnIndex]).toHaveProperty('name', 'name');
+ expect(result.columns[nameColumnIndex].meta).toHaveProperty('type', 'number');
+ expect(result.rows[arbitraryRowIndex]).toHaveProperty('name', 202);
+ });
 
- expect(result.type).toBe('datatable');
- expect(result.columns).toHaveLength(sqlTable.columns.length);
- expect(result.columns[nameColumnIndex]).toHaveProperty('name', 'name');
- expect(result.columns[nameColumnIndex].meta).toHaveProperty('type', 'number');
- expect(result.rows[arbitraryRowIndex]).toHaveProperty('name', 202);
+ 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('inserts a new column with a duplicate name if an id and name are provided', async () => {
- const result = await runFn(testTable, {
- id: 'new',
- name: 'name label',
- expression: pricePlusTwo,
+ describe('when the table columns do not have id', () => {
+ it('uses name as unique key when id arg is also missing', async () => {
+ const result = await runFn(sqlTable, { name: 'name', expression: pricePlusTwo });
+ const nameColumnIndex = result.columns.findIndex(({ name }) => name === 'name');
+ const arbitraryRowIndex = 4;
+
+ expect(result.type).toBe('datatable');
+ expect(result.columns).toHaveLength(sqlTable.columns.length);
+ expect(result.columns[nameColumnIndex]).toHaveProperty('name', 'name');
+ expect(result.columns[nameColumnIndex].meta).toHaveProperty('type', 'number');
+ expect(result.rows[arbitraryRowIndex]).toHaveProperty('name', 202);
  });
- 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('overwrites columns matching id === name when the column is missing an id', async () => {
+ const result = await runFn(sqlTable, {
+ id: 'name',
+ name: 'name is ignored',
+ expression: pricePlusTwo,
+ });
+ const nameColumnIndex = result.columns.findIndex(({ name }) => name === 'name is ignored');
+ const arbitraryRowIndex = 4;
+
+ expect(result.type).toBe('datatable');
+ expect(result.columns).toHaveLength(sqlTable.columns.length);
+ expect(result.columns[nameColumnIndex]).toHaveProperty('id', 'name');
+ expect(result.columns[nameColumnIndex]).toHaveProperty('name', 'name is ignored');
+ expect(result.columns[nameColumnIndex].meta).toHaveProperty('type', 'number');
+ expect(result.rows[arbitraryRowIndex]).toHaveProperty('name', 202);
+ });
  });
 
  it('adds a column to empty tables', async () => {

diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json
@@ -2022,7 +2022,6 @@
  "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
@@ -2034,7 +2034,6 @@
  "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'` 时，其将引发异常，从而终止表达式执行 (默认) 。",