[Expressions] Use table column ID instead of name when set

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

@@ -101,7 +101,14 @@ export const mapColumn: ExpressionFunctionDefinition<
     });
 
     return Promise.all(rowPromises).then((rows) => {
-      const existingColumnIndex = columns.findIndex(({ name }) => name === args.name);
+      const existingColumnIndex = columns.findIndex(({ id, name }) => {
+        // Columns that have IDs are allowed to have duplicate names, for example esaggs
+        if (id) {
+          return id === args.id && name === args.name;
+        }
+        // If no ID, name is the unique key. For example, SQL output does not have IDs
+        return name === args.name;
+      });
       const type = rows.length ? getType(rows[0][columnId]) : 'null';
       const newColumn = {
         id: columnId,

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 ?? col.name)
         )
       : { value: input };
 

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

@@ -9,7 +9,7 @@
 import { of } from 'rxjs';
 import { Datatable } from '../../../expression_types';
 import { mapColumn, MapColumnArguments } from '../map_column';
-import { emptyTable, functionWrapper, testTable } from './utils';
+import { emptyTable, functionWrapper, testTable, sqlTable } from './utils';
 
 const pricePlusTwo = (datatable: Datatable) => of(datatable.rows[0].price + 2);
 
@@ -35,18 +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 });
+  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;
 
     expect(result.type).toBe('datatable');
-    expect(result.columns).toHaveLength(testTable.columns.length);
+    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('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,
+    });
+    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 () => {
     const result = await runFn(emptyTable, { name: 'name', expression: pricePlusTwo });
 
@@ -66,18 +83,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(
       {

src/plugins/expressions/common/expression_functions/specs/tests/math.test.ts

@@ -7,7 +7,7 @@
  */
 
 import { errors, math } from '../math';
-import { emptyTable, functionWrapper, testTable } from './utils';
+import { emptyTable, functionWrapper, testTable, sqlTable } from './utils';
 
 describe('math', () => {
   const fn = functionWrapper<unknown>(math);
@@ -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,15 @@ describe('math', () => {
     expect(fn(testTable, { expression: 'max(price)' })).toBe(605);
   });
 
+  it('evaluates math expressions with references to columns in a datatable', () => {
+    expect(fn(sqlTable, { expression: 'unique(in_stock)' })).toBe(2);
+    expect(fn(sqlTable, { expression: 'sum(quantity)' })).toBe(2508);
+    expect(fn(sqlTable, { expression: 'mean(price)' })).toBe(320);
+    expect(fn(sqlTable, { expression: 'min(price)' })).toBe(67);
+    expect(fn(sqlTable, { expression: 'median(quantity)' })).toBe(256);
+    expect(fn(sqlTable, { expression: 'max(price)' })).toBe(605);
+  });
+
   describe('args', () => {
     describe('expression', () => {
       it('sets the math expression to be evaluted', () => {

src/plugins/expressions/common/expression_functions/specs/tests/utils.ts

@@ -9,7 +9,7 @@
 import { mapValues } from 'lodash';
 import { AnyExpressionFunctionDefinition } from '../../types';
 import { ExecutionContext } from '../../../execution/types';
-import { Datatable } from '../../../expression_types';
+import { Datatable, DatatableColumn } from '../../../expression_types';
 
 /**
  * Takes a function spec and passes in default args,
@@ -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' },
     },
   ],
@@ -224,4 +224,32 @@ const stringTable: Datatable = {
   ],
 };
 
-export { emptyTable, testTable, stringTable };
+// Emulates a SQL table that doesn't have any IDs
+const sqlTable: Datatable = {
+  type: 'datatable',
+  columns: [
+    ({
+      name: 'name',
+      meta: { type: 'string' },
+    } as unknown) as DatatableColumn,
+    ({
+      name: 'time',
+      meta: { type: 'date' },
+    } as unknown) as DatatableColumn,
+    ({
+      name: 'price',
+      meta: { type: 'number' },
+    } as unknown) as DatatableColumn,
+    ({
+      name: 'quantity',
+      meta: { type: 'number' },
+    } as unknown) as DatatableColumn,
+    ({
+      name: 'in_stock',
+      meta: { type: 'boolean' },
+    } as unknown) as DatatableColumn,
+  ],
+  rows: [...testTable.rows],
+};
+
+export { emptyTable, testTable, stringTable, sqlTable };