From 08121d9f837676de533769780980ca71ea21b3d4 Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Thu, 10 Jun 2021 10:40:25 -0400 Subject: [PATCH 1/3] [Lens] Create mathColumn function to improve performance --- .../canvas/canvas-function-reference.asciidoc | 57 +++++++-- .../expression_functions/specs/index.ts | 1 + .../expression_functions/specs/math_column.ts | 111 ++++++++++++++++++ .../specs/tests/math_column.test.ts | 74 ++++++++++++ .../common/service/expressions_services.ts | 2 + .../definitions/formula/formula.tsx | 21 +--- .../operations/definitions/formula/math.tsx | 19 +-- 7 files changed, 242 insertions(+), 43 deletions(-) create mode 100644 src/plugins/expressions/common/expression_functions/specs/math_column.ts create mode 100644 src/plugins/expressions/common/expression_functions/specs/tests/math_column.test.ts diff --git a/docs/canvas/canvas-function-reference.asciidoc b/docs/canvas/canvas-function-reference.asciidoc index 272cd524c2c20..ac7cbba6e9933 100644 --- a/docs/canvas/canvas-function-reference.asciidoc +++ b/docs/canvas/canvas-function-reference.asciidoc @@ -71,7 +71,7 @@ Alias: `condition` [[alterColumn_fn]] === `alterColumn` -Converts between core types, including `string`, `number`, `null`, `boolean`, and `date`, and renames columns. See also <> and <>. +Converts between core types, including `string`, `number`, `null`, `boolean`, and `date`, and renames columns. See also <>, <>, and <>. *Expression syntax* [source,js] @@ -1717,11 +1717,16 @@ Adds a column calculated as the result of other columns. Changes are made only w |=== |Argument |Type |Description +|`id` + +|`string`, `null` +|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. + |_Unnamed_ *** Aliases: `column`, `name` |`string` -|The name of the resulting column. +|The name of the resulting column. Names are not required to be unique. |`expression` *** @@ -1729,11 +1734,6 @@ Aliases: `exp`, `fn`, `function` |`boolean`, `number`, `string`, `null` |A Canvas expression that is passed to each row as a single row `datatable`. -|`id` - -|`string`, `null` -|An optional id of the resulting column. When not specified or `null` the name argument is used as id. - |`copyMetaFrom` |`string`, `null` @@ -1808,6 +1808,47 @@ Default: `"throw"` *Returns:* `number` | `boolean` | `null` +[float] +[[mathColumn_fn]] +=== `mathColumn` + +Adds a column by evaluating `TinyMath` on each row. This function is optimized for math, so it performs better than the <> with a <>. +*Accepts:* `datatable` + +[cols="3*^<"] +|=== +|Argument |Type |Description + +|id *** +|`string` +|id of the resulting column. Must be unique. + +|name *** +|`string` +|The name of the resulting column. Names are not required to be unique. + +|_Unnamed_ + +Alias: `expression` +|`string` +|A `TinyMath` expression evaluated on each row. See https://www.elastic.co/guide/en/kibana/current/canvas-tinymath-functions.html. + +|`onError` + +|`string` +|In case the `TinyMath` evaluation fails or returns NaN, the return value is specified by onError. For example, `"null"`, `"zero"`, `"false"`, `"throw"`. When `"throw"`, it will throw an exception, terminating expression execution. + +Default: `"throw"` + +|`copyMetaFrom` + +|`string`, `null` +|If set, the meta object from the specified column id is copied over to the specified target column. Throws an exception if the column doesn't exist +|=== + +*Returns:* `datatable` + + [float] [[metric_fn]] === `metric` @@ -2581,7 +2622,7 @@ Default: `false` [[staticColumn_fn]] === `staticColumn` -Adds a column with the same static value in every row. See also <> and <>. +Adds a column with the same static value in every row. See also <>, <>, and <>. *Accepts:* `datatable` diff --git a/src/plugins/expressions/common/expression_functions/specs/index.ts b/src/plugins/expressions/common/expression_functions/specs/index.ts index 20a6f9aac4567..e78e864b9f43b 100644 --- a/src/plugins/expressions/common/expression_functions/specs/index.ts +++ b/src/plugins/expressions/common/expression_functions/specs/index.ts @@ -17,3 +17,4 @@ export * from './moving_average'; export * from './ui_setting'; export { mapColumn, MapColumnArguments } from './map_column'; export { math, MathArguments, MathInput } from './math'; +export { mathColumn, MathColumnArguments } from './math_column'; diff --git a/src/plugins/expressions/common/expression_functions/specs/math_column.ts b/src/plugins/expressions/common/expression_functions/specs/math_column.ts new file mode 100644 index 0000000000000..0ff8faf3ce55a --- /dev/null +++ b/src/plugins/expressions/common/expression_functions/specs/math_column.ts @@ -0,0 +1,111 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { i18n } from '@kbn/i18n'; +import { ExpressionFunctionDefinition } from '../types'; +import { math, MathArguments } from './math'; +import { Datatable, DatatableColumn, getType } from '../../expression_types'; + +export type MathColumnArguments = MathArguments & { + id: string; + name?: string; + copyMetaFrom?: string | null; +}; + +export const mathColumn: ExpressionFunctionDefinition< + 'mathColumn', + Datatable, + MathColumnArguments, + Datatable +> = { + name: 'mathColumn', + type: 'datatable', + inputTypes: ['datatable'], + help: i18n.translate('expressions.functions.mathColumnHelpText', { + defaultMessage: + 'Adds a column calculated as the result of other columns. ' + + 'Changes are made only when you provide arguments.' + + 'See also {alterColumnFn} and {staticColumnFn}.', + values: { + alterColumnFn: '`alterColumn`', + staticColumnFn: '`staticColumn`', + }, + }), + args: { + ...math.args, + id: { + types: ['string'], + help: i18n.translate('expressions.functions.mathColumn.args.idHelpText', { + defaultMessage: 'id of the resulting column. Must be unique.', + }), + required: true, + }, + name: { + types: ['string'], + aliases: ['_', 'column'], + help: i18n.translate('expressions.functions.mathColumn.args.nameHelpText', { + defaultMessage: 'The name of the resulting column. Names are not required to be unique.', + }), + required: true, + }, + copyMetaFrom: { + types: ['string', 'null'], + help: i18n.translate('expressions.functions.mathColumn.args.copyMetaFromHelpText', { + defaultMessage: + "If set, the meta object from the specified column id is copied over to the specified target column. If the column doesn't exist it silently fails.", + }), + required: false, + default: null, + }, + }, + fn: (input, args, context) => { + const columns = [...input.columns]; + const existingColumnIndex = columns.findIndex(({ id }) => { + return id === args.id; + }); + if (existingColumnIndex > -1) { + throw new Error('ID must be unique'); + } + + const newRows = input.rows.map((row) => { + return { + ...row, + [args.id]: math.fn( + { + type: 'datatable', + columns: input.columns, + rows: [row], + }, + { + expression: args.expression, + onError: args.onError, + }, + context + ), + }; + }); + const type = newRows.length ? getType(newRows[0][args.id]) : 'null'; + const newColumn: DatatableColumn = { + id: args.id, + name: args.name ?? args.id, + meta: { type, params: { id: type } }, + }; + if (args.copyMetaFrom) { + const metaSourceFrom = columns.find(({ id }) => id === args.copyMetaFrom); + newColumn.meta = { ...newColumn.meta, ...(metaSourceFrom?.meta || {}) }; + } + + columns.push(newColumn); + + return { + type: 'datatable', + columns, + rows: newRows, + } as Datatable; + }, +}; diff --git a/src/plugins/expressions/common/expression_functions/specs/tests/math_column.test.ts b/src/plugins/expressions/common/expression_functions/specs/tests/math_column.test.ts new file mode 100644 index 0000000000000..bc6699a2b689b --- /dev/null +++ b/src/plugins/expressions/common/expression_functions/specs/tests/math_column.test.ts @@ -0,0 +1,74 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { mathColumn } from '../math_column'; +import { functionWrapper, testTable } from './utils'; + +describe('mathColumn', () => { + const fn = functionWrapper(mathColumn); + + it('throws if the id is used', () => { + expect(() => fn(testTable, { id: 'price', name: 'price', expression: 'price * 2' })).toThrow( + `ID must be unique` + ); + }); + + it('applies math to each row by id', () => { + const result = fn(testTable, { id: 'output', name: 'output', expression: 'quantity * price' }); + expect(result.columns).toEqual([ + ...testTable.columns, + { id: 'output', name: 'output', meta: { params: { id: 'number' }, type: 'number' } }, + ]); + expect(result.rows[0]).toEqual({ + in_stock: true, + name: 'product1', + output: 60500, + price: 605, + quantity: 100, + time: 1517842800950, + }); + }); + + it('handles onError', () => { + const args = { + id: 'output', + name: 'output', + expression: 'quantity / 0', + }; + expect(() => fn(testTable, args)).toThrowError(`Cannot divide by 0`); + expect(() => fn(testTable, { ...args, onError: 'throw' })).toThrow(); + expect(fn(testTable, { ...args, onError: 'zero' }).rows[0].output).toEqual(0); + expect(fn(testTable, { ...args, onError: 'false' }).rows[0].output).toEqual(false); + expect(fn(testTable, { ...args, onError: 'null' }).rows[0].output).toEqual(null); + }); + + it('should copy over the meta information from the specified column', async () => { + const result = await fn( + { + ...testTable, + columns: [ + ...testTable.columns, + { + id: 'myId', + name: 'myName', + meta: { type: 'date', params: { id: 'number', params: { digits: 2 } } }, + }, + ], + rows: testTable.rows.map((row) => ({ ...row, myId: Date.now() })), + }, + { id: 'output', name: 'name', copyMetaFrom: 'myId', expression: 'price + 2' } + ); + + expect(result.type).toBe('datatable'); + expect(result.columns[result.columns.length - 1]).toEqual({ + id: 'output', + name: 'name', + meta: { type: 'date', params: { id: 'number', params: { digits: 2 } } }, + }); + }); +}); diff --git a/src/plugins/expressions/common/service/expressions_services.ts b/src/plugins/expressions/common/service/expressions_services.ts index a8839c9b0d71e..6e43a9c9f767c 100644 --- a/src/plugins/expressions/common/service/expressions_services.ts +++ b/src/plugins/expressions/common/service/expressions_services.ts @@ -30,6 +30,7 @@ import { movingAverage, mapColumn, math, + mathColumn, } from '../expression_functions'; /** @@ -342,6 +343,7 @@ export class ExpressionsService implements PersistableStateService Date: Mon, 14 Jun 2021 13:03:24 -0400 Subject: [PATCH 2/3] Fix empty formula case --- packages/kbn-tinymath/src/index.js | 3 ++- .../operations/definitions/formula/formula.tsx | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/kbn-tinymath/src/index.js b/packages/kbn-tinymath/src/index.js index 9f1bb7b851463..b5807ce79e9eb 100644 --- a/packages/kbn-tinymath/src/index.js +++ b/packages/kbn-tinymath/src/index.js @@ -7,6 +7,7 @@ */ const { get } = require('lodash'); +const memoizeOne = require('memoize-one'); // eslint-disable-next-line import/no-unresolved const { parse: parseFn } = require('../grammar'); const { functions: includedFunctions } = require('./functions'); @@ -23,7 +24,7 @@ function parse(input, options) { } try { - return parseFn(input, options); + return memoizeOne(parseFn)(input, options); } catch (e) { throw new Error(`Failed to parse expression. ${e.message}`); } diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.tsx index ec0ce461c0422..6d3ab6a7f3082 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.tsx @@ -100,11 +100,11 @@ export const formulaOperation: OperationDefinition< return [ { type: 'function', - function: 'mathColumn', + function: currentColumn.references.length ? 'mathColumn' : 'mapColumn', arguments: { id: [columnId], name: [label || defaultLabel], - expression: [currentColumn.references.length ? `"${currentColumn.references[0]}"` : ``], + expression: [currentColumn.references.length ? `"${currentColumn.references[0]}"` : ''], }, }, ]; From 9d1b40debe8d9ce53672bce85c0b373524e009eb Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Tue, 15 Jun 2021 13:33:57 -0400 Subject: [PATCH 3/3] Fix tinymath memoization --- packages/kbn-tinymath/src/index.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/kbn-tinymath/src/index.js b/packages/kbn-tinymath/src/index.js index b5807ce79e9eb..6fde4c202e2a7 100644 --- a/packages/kbn-tinymath/src/index.js +++ b/packages/kbn-tinymath/src/index.js @@ -12,8 +12,6 @@ const memoizeOne = require('memoize-one'); const { parse: parseFn } = require('../grammar'); const { functions: includedFunctions } = require('./functions'); -module.exports = { parse, evaluate, interpret }; - function parse(input, options) { if (input == null) { throw new Error('Missing expression'); @@ -24,15 +22,17 @@ function parse(input, options) { } try { - return memoizeOne(parseFn)(input, options); + return parseFn(input, options); } catch (e) { throw new Error(`Failed to parse expression. ${e.message}`); } } +const memoizedParse = memoizeOne(parse); + function evaluate(expression, scope = {}, injectedFunctions = {}) { scope = scope || {}; - return interpret(parse(expression), scope, injectedFunctions); + return interpret(memoizedParse(expression), scope, injectedFunctions); } function interpret(node, scope, injectedFunctions) { @@ -80,3 +80,5 @@ function isOperable(args) { return typeof arg === 'number' && !isNaN(arg); }); } + +module.exports = { parse: memoizedParse, evaluate, interpret };