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

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

@@ -101,7 +101,18 @@ 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 (args.id) {
+ return id === args.id;
+ }
+ // If the column has an ID, but there is no ID argument to mapColumn
+ if (id) {
+ return id === args.name;
+ }
+ // Columns without ID use name as 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,47 @@ describe('mapColumn', () => {
  expect(result.rows[arbitraryRowIndex]).toHaveProperty('pricePlusTwo');
  });
 
- it('overwrites existing column with the new column if an existing column name is provided', async () => {
+ 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(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('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(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 +95,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 };