Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Expressions] [Lens] Add id and copyMetaFrom arg to mapColumn fn + add configurable onError argument to math fn #90481

Merged
merged 23 commits into from
Feb 25, 2021
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
2fc8e67
:truck: Move mapColumn to expressions plugin
dej611 Feb 5, 2021
2f1e324
Merge branch 'master' into expression/mapColumn-ext
kibanamachine Feb 9, 2021
744f39a
Merge branch 'master' into expression/mapColumn-ext
kibanamachine Feb 11, 2021
b00b9dc
Merge branch 'master' into expression/mapColumn-ext
kibanamachine Feb 15, 2021
96c3aae
:globe_with_meridians: Fix i18n for mapColumns
dej611 Feb 15, 2021
241aa03
:truck: Move over the math function to shared expressions plugin
dej611 Feb 15, 2021
b89934f
:sparkles: Add new onError argument
dej611 Feb 15, 2021
4c8fd8c
:globe_with_meridians: Fix i18n for math
dej611 Feb 15, 2021
75fecc8
:memo: Update expressions doc with new args
dej611 Feb 16, 2021
abbdbbc
Merge branch 'master' into expression/mapColumn-ext
kibanamachine Feb 18, 2021
7eb7ebd
:sparkles: Add the divide by zero handling
dej611 Feb 18, 2021
804507a
:sparkles: Add options for autocomplete
dej611 Feb 18, 2021
88a7d36
Merge branch 'master' into expression/mapColumn-ext
kibanamachine Feb 19, 2021
0ba2ce9
Merge branch 'master' into expression/mapColumn-ext
kibanamachine Feb 22, 2021
0ac1f27
:bug: Fix bug wth type picker
dej611 Feb 22, 2021
eb841bc
Merge branch 'expression/mapColumn-ext' of github.com:dej611/kibana i…
dej611 Feb 22, 2021
2579d0e
Merge branch 'master' into expression/mapColumn-ext
kibanamachine Feb 23, 2021
a5c8bdf
:ok_hand: Integrated feedback
dej611 Feb 23, 2021
6bfa823
:bug: Fix unused value
dej611 Feb 23, 2021
a6cb2d0
:white_check_mark: Add more tests
dej611 Feb 24, 2021
7cdf96f
:bug: Fix type issue
dej611 Feb 24, 2021
0f601c0
:white_check_mark: Fix unit test
dej611 Feb 24, 2021
b8aff19
Merge branch 'master' into expression/mapColumn-ext
kibanamachine Feb 24, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion docs/canvas/canvas-function-reference.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -1697,6 +1697,16 @@ Aliases: `column`, `name`
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`
|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`
Expand Down Expand Up @@ -1755,9 +1765,16 @@ Interprets a `TinyMath` math expression using a `number` or `datatable` as _cont
Alias: `expression`
|`string`
|An evaluated `TinyMath` expression. 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"`
|===

*Returns:* `number`
*Returns:* `number` | `boolean` | `null`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It needs to extends the return type to cover the onError returning values



[float]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import { theme } from './theme';
import { cumulativeSum } from './cumulative_sum';
import { derivative } from './derivative';
import { movingAverage } from './moving_average';
import { mapColumn } from './map_column';
import { math } from './math';

export const functionSpecs: AnyExpressionFunctionDefinition[] = [
clog,
Expand All @@ -25,6 +27,8 @@ export const functionSpecs: AnyExpressionFunctionDefinition[] = [
cumulativeSum,
derivative,
movingAverage,
mapColumn,
math,
];

export * from './clog';
Expand All @@ -35,3 +39,5 @@ export * from './theme';
export * from './cumulative_sum';
export * from './derivative';
export * from './moving_average';
export { mapColumn, MapColumnArguments } from './map_column';
export { math, MathArguments, MathInput } from './math';
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
/*
* 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 { Datatable, getType } from '../../expression_types';

export interface MapColumnArguments {
id?: string | null;
name: string;
expression?: (datatable: Datatable) => Promise<boolean | number | string | null>;
copyMetaFrom?: string | null;
}

export const mapColumn: ExpressionFunctionDefinition<
'mapColumn',
Datatable,
MapColumnArguments,
Promise<Datatable>
> = {
name: 'mapColumn',
aliases: ['mc'], // midnight commander. So many times I've launched midnight commander instead of moving a file.
type: 'datatable',
inputTypes: ['datatable'],
help: i18n.translate('expressions.functions.mapColumnHelpText', {
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: {
id: {
types: ['string', 'null'],
help: i18n.translate('expressions.functions.mapColumn.args.idHelpText', {
defaultMessage:
'An optional id of the resulting column. When `null` the name/column argument is used as id.',
}),
required: false,
default: null,
},
name: {
types: ['string'],
aliases: ['_', 'column'],
help: i18n.translate('expressions.functions.mapColumn.args.nameHelpText', {
defaultMessage: 'The name of the resulting column.',
}),
required: true,
},
expression: {
types: ['boolean', 'number', 'string', 'null'],
resolve: false,
aliases: ['exp', 'fn', 'function'],
help: i18n.translate('expressions.functions.mapColumn.args.expressionHelpText', {
defaultMessage:
'An expression that is executed on every row, provided with a single-row {DATATABLE} context and returning the cell value.',
values: {
DATATABLE: '`datatable`',
},
}),
required: true,
},
copyMetaFrom: {
types: ['string', 'null'],
help: i18n.translate('expressions.functions.mapColumn.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) => {
const expression = args.expression || (() => Promise.resolve(null));
const columnId = args.id != null ? args.id : args.name;

const columns = [...input.columns];
const rowPromises = input.rows.map((row) => {
return expression({
type: 'datatable',
columns,
rows: [row],
}).then((val) => ({
...row,
[columnId]: val,
}));
});

return Promise.all(rowPromises).then((rows) => {
const existingColumnIndex = columns.findIndex(({ name }) => name === args.name);
const type = rows.length ? getType(rows[0][columnId]) : 'null';
const newColumn = {
id: columnId,
name: args.name,
meta: { type },
};
if (args.copyMetaFrom) {
const metaSourceFrom = columns.find(({ id }) => id === args.copyMetaFrom);
newColumn.meta = { ...newColumn.meta, ...(metaSourceFrom?.meta || {}) };
}

if (existingColumnIndex === -1) {
columns.push(newColumn);
} else {
columns[existingColumnIndex] = newColumn;
}

return {
type: 'datatable',
columns,
rows,
} as Datatable;
});
},
};
167 changes: 167 additions & 0 deletions src/plugins/expressions/common/expression_functions/specs/math.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
/*
* 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 { map, zipObject } from 'lodash';
import { i18n } from '@kbn/i18n';
import { evaluate } from '@kbn/tinymath';
import { ExpressionFunctionDefinition } from '../types';
import { Datatable, isDatatable } from '../../expression_types';

export type MathArguments = {
expression: string;
onError?: 'null' | 'zero' | 'false' | 'throw';
};

export type MathInput = number | Datatable;

const TINYMATH = '`TinyMath`';
const TINYMATH_URL =
'https://www.elastic.co/guide/en/kibana/current/canvas-tinymath-functions.html';

const isString = (val: any): boolean => typeof val === 'string';

function pivotObjectArray<
RowType extends { [key: string]: any },
ReturnColumns extends string | number | symbol = keyof RowType
>(rows: RowType[], columns?: string[]): Record<string, ReturnColumns[]> {
const columnNames = columns || Object.keys(rows[0]);
if (!columnNames.every(isString)) {
throw new Error('Columns should be an array of strings');
}

const columnValues = map(columnNames, (name) => map(rows, name));
return zipObject(columnNames, columnValues);
}

export const errors = {
emptyExpression: () =>
new Error(
i18n.translate('expressions.functions.math.emptyExpressionErrorMessage', {
defaultMessage: 'Empty expression',
})
),
tooManyResults: () =>
new Error(
i18n.translate('expressions.functions.math.tooManyResultsErrorMessage', {
defaultMessage:
'Expressions must return a single number. Try wrapping your expression in {mean} or {sum}',
values: {
mean: 'mean()',
sum: 'sum()',
},
})
),
executionFailed: () =>
new Error(
i18n.translate('expressions.functions.math.executionFailedErrorMessage', {
defaultMessage: 'Failed to execute math expression. Check your column names',
})
),
emptyDatatable: () =>
new Error(
i18n.translate('expressions.functions.math.emptyDatatableErrorMessage', {
defaultMessage: 'Empty datatable',
})
),
};

const fallbackValue = {
null: null,
zero: 0,
false: false,
} as const;

export const math: ExpressionFunctionDefinition<
'math',
MathInput,
MathArguments,
boolean | number | null
> = {
name: 'math',
type: undefined,
inputTypes: ['number', 'datatable'],
help: i18n.translate('expressions.functions.mathHelpText', {
defaultMessage:
'Interprets a {TINYMATH} math expression using a {TYPE_NUMBER} or {DATATABLE} as {CONTEXT}. ' +
'The {DATATABLE} columns are available by their column name. ' +
'If the {CONTEXT} is a number it is available as {value}.',
values: {
TINYMATH,
CONTEXT: '_context_',
DATATABLE: '`datatable`',
value: '`value`',
TYPE_NUMBER: '`number`',
},
}),
args: {
expression: {
aliases: ['_'],
types: ['string'],
help: i18n.translate('expressions.functions.math.args.expressionHelpText', {
defaultMessage: 'An evaluated {TINYMATH} expression. See {TINYMATH_URL}.',
values: {
TINYMATH,
TINYMATH_URL,
},
}),
},
onError: {
types: ['string'],
options: ['throw', 'false', 'zero', 'null'],
help: i18n.translate('expressions.functions.math.args.onErrorHelpText', {
defaultMessage:
"In case the {TINYMATH} evaluation fails or returns NaN, the return value is specified by onError. When `'throw'`, it will throw an exception, terminating expression execution (default).",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should specify valid values instead of just being of type string (to allow auto-complete)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that as first option, but could not find a way to map a string union to a specific type for expressions. All the examples I check in the documentation have the string type assigned and documenting the valid values on the documentation side.

values: {
TINYMATH,
},
}),
},
},
fn: (input, args) => {
const { expression, onError } = args;
const onErrorValue = onError ?? 'throw';

if (!expression || expression.trim() === '') {
throw errors.emptyExpression();
}

const mathContext = isDatatable(input)
? pivotObjectArray(
input.rows,
input.columns.map((col) => col.name)
)
: { value: input };

try {
const result = evaluate(expression, mathContext);
if (Array.isArray(result)) {
if (result.length === 1) {
return result[0];
}
throw errors.tooManyResults();
}
if (isNaN(result)) {
// make TS happy
if (onErrorValue !== 'throw' && onErrorValue in fallbackValue) {
return fallbackValue[onErrorValue];
}
throw errors.executionFailed();
}
return result;
} catch (e) {
if (onErrorValue !== 'throw' && onErrorValue in fallbackValue) {
return fallbackValue[onErrorValue];
}
if (isDatatable(input) && input.rows.length === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The onError logic has to happen in the catch handler as well to catch things like divide by zero

throw errors.emptyDatatable();
} else {
throw e;
}
}
},
};
Loading