Skip to content

Commit

Permalink
Merge pull request #1090 from finos/computed-fix
Browse files Browse the repository at this point in the history 
Computed expressions respect left-to-right associativity and operator precedence
  • Loading branch information
@texodus
texodus authored Jun 24, 2020
2 parents 1abd35d + d6011b9 commit ae1dce8
Show file tree
Hide file tree
Showing 11 changed files with 1,524 additions and 491 deletions.
5 changes: 3 additions & 2 deletions packages/perspective-viewer/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
"build": "npm-run-all --silent build:babel build:webpack:cjs build:webpack:umd",
"watch": "webpack --color --watch --config src/config/view.config.js",
"test:build": "cpx \"test/html/*\" dist/umd && cpx \"test/csv/*\" dist/umd && cpx \"test/css/*\" dist/umd",
"test:run": "jest --rootDir=. --config=../perspective-test/jest.config.js --silent --color 2>&1",
"test:run": "jest --rootDir=. --config=../perspective-test/jest.config.js --color 2>&1",
"test": "npm-run-all test:build test:run",
"clean": "rimraf dist",
"clean:screenshots": "rimraf \"screenshots/**/*.@(failed|diff).png\"",
Expand Down Expand Up @@ -70,6 +70,7 @@
},
"devDependencies": {
"@finos/perspective-test": "^0.5.0",
"@finos/perspective-webpack-plugin": "^0.5.0"
"@finos/perspective-webpack-plugin": "^0.5.0",
"jsverify": "^0.8.4"
}
}
321 changes: 205 additions & 116 deletions packages/perspective-viewer/src/js/computed_expressions/computed_expression_parser.js
View file

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/perspective-viewer/src/js/computed_expressions/lexer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ export const clean_tokens = function(tokens) {
const cleaned_tokens = [];

for (const token of tokens) {
if (token.tokenType.name !== "whitespace") {
if (!tokenMatcher(token, Whitespace)) {
cleaned_tokens.push(token);
}
}
Expand Down
183 changes: 139 additions & 44 deletions packages/perspective-viewer/src/js/computed_expressions/parser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,54 +9,119 @@
import {CstParser} from "chevrotain";
import {PerspectiveParserErrorMessage} from "./error";

/**
* A Chevrotain expression parser that produces a concrete syntax tree
* representing every single parsed token.
*/
export class ComputedExpressionColumnParser extends CstParser {
constructor(vocabulary) {
super(vocabulary, {
errorMessageProvider: PerspectiveParserErrorMessage
});

/**
* The overarching rule - required so that one single array of
* parsed computed columns can be maintained across multiple operator,
* functional, and parenthetical expressions.
*/
this.RULE("SuperExpression", () => {
this.SUBRULE(this.Expression);
});

/**
* The `base rule` for all expressions, except that the state of the
* computed column array can be changed between each invocation of
* this rule.
*/
this.RULE("Expression", () => {
this.OR(
[
{
ALT: () => {
this.SUBRULE(this.OperatorComputedColumn);
}
},
{
ALT: () => {
this.SUBRULE(this.FunctionComputedColumn);
}
}
],
{
ERR_MSG: "Expected an expression of the form `x + y` or `func(x)`."
}
);
this.SUBRULE(this.OperatorComputedColumn);
});

/**
* A computed column in `x + y` notation. Because it appears earlier,
* it has lower precedence compared to the rules that are to follow.
*/
this.RULE("OperatorComputedColumn", () => {
this.SUBRULE(this.ColumnName, {LABEL: "left"});
this.AT_LEAST_ONE(() => {
this.SUBRULE(this.AdditionOperatorComputedColumn, {LABEL: "left"});

// 0...n operators and right-hand expressions are available here.
// Though a single column name is syntactically valid, it does
// not actually generate any computed columns. However, this
// rule must allow for 0...n and not 1...n as it allows for
// a function-only expression (`sqrt("a")`) without a right
// hand side of the expression.
this.MANY(() => {
this.SUBRULE(this.Operator);
this.SUBRULE2(this.ColumnName, {LABEL: "right"});
this.SUBRULE2(this.AdditionOperatorComputedColumn, {LABEL: "right"});
this.OPTION(() => {
this.SUBRULE(this.As, {LABEL: "as"});
});
});
this.OPTION(() => {
this.SUBRULE(this.As, {LABEL: "as"});
});

/**
* A computed column in `x + y` or `x - y` notation. To maintain a
* notion of operator precedence, different rules must be created for
* add/subtract and multiply/divide operators, even if the actual
* evaluator logic is the same.
*/
this.RULE("AdditionOperatorComputedColumn", () => {
this.SUBRULE(this.MultiplicationOperatorComputedColumn, {LABEL: "left"});
this.MANY(() => {
this.SUBRULE(this.AdditionOperator);
this.SUBRULE2(this.MultiplicationOperatorComputedColumn, {LABEL: "right"});
this.OPTION(() => {
this.SUBRULE(this.As, {LABEL: "as"});
});
});
});

/**
* A computed column in `x * y` or `x / y` notation. Because it is
* defined after the addition and generic operators, it is evaluated
* before the addition/generic operators - hence satisfying precedence.
*/
this.RULE("MultiplicationOperatorComputedColumn", () => {
this.SUBRULE(this.ExponentOperatorComputedColumn, {LABEL: "left"});
this.MANY(() => {
this.SUBRULE(this.MultiplicationOperator);
this.SUBRULE2(this.ExponentOperatorComputedColumn, {LABEL: "right"});
this.OPTION(() => {
this.SUBRULE(this.As, {LABEL: "as"});
});
});
});

/**
* A computed column in `x ^ y` notation. Exponents are evaluated before
* multiplication/division and addition/subtraction, so it is defined
* after those rules to give itself precedence.
*/
this.RULE("ExponentOperatorComputedColumn", () => {
this.SUBRULE(this.ColumnName, {LABEL: "left"});
this.MANY(() => {
this.SUBRULE(this.ExponentOperator);
this.SUBRULE2(this.ColumnName, {LABEL: "right"});
this.OPTION(() => {
this.SUBRULE(this.As, {LABEL: "as"});
});
});
});

/**
* A computed column in `f(x)` notation. It is evaluated before all
* operator computed columns.
*/
this.RULE("FunctionComputedColumn", () => {
this.SUBRULE(this.Function);
this.CONSUME(vocabulary["leftParen"]);

this.AT_LEAST_ONE_SEP({
SEP: vocabulary["comma"],
DEF: () => {
this.SUBRULE(this.ColumnName);
// Allow for arbitary expressions inside functions without
// use of parentheses.
this.SUBRULE(this.Expression, {LABEL: "param"});
}
});
this.CONSUME(vocabulary["rightParen"]);
Expand All @@ -65,21 +130,6 @@ export class ComputedExpressionColumnParser extends CstParser {
});
});

this.RULE("ColumnName", () => {
this.OR([{ALT: () => this.SUBRULE(this.ParentheticalExpression)}, {ALT: () => this.CONSUME(vocabulary["columnName"])}], {
ERR_MSG: "Expected a column name (wrapped in double quotes) or a parenthesis-wrapped expression."
});
});

this.RULE("TerminalColumnName", () => {
this.CONSUME(vocabulary["columnName"]);
});

this.RULE("As", () => {
this.CONSUME(vocabulary["as"]);
this.SUBRULE(this.TerminalColumnName);
});

this.RULE("Function", () => {
this.OR([
{ALT: () => this.CONSUME(vocabulary["sqrt"])},
Expand Down Expand Up @@ -112,13 +162,25 @@ export class ComputedExpressionColumnParser extends CstParser {
]);
});

/**
* Consume an addition or subtraction symbol. Rules for operators with
* defined precedence rules are separated from the general
* `Operator` rule.
*/
this.RULE("AdditionOperator", () => {
this.OR([{ALT: () => this.CONSUME(vocabulary["add"])}, {ALT: () => this.CONSUME(vocabulary["subtract"])}]);
});

this.RULE("MultiplicationOperator", () => {
this.OR([{ALT: () => this.CONSUME(vocabulary["multiply"])}, {ALT: () => this.CONSUME(vocabulary["divide"])}]);
});

this.RULE("ExponentOperator", () => {
this.CONSUME(vocabulary["pow"]);
});

this.RULE("Operator", () => {
this.OR([
{ALT: () => this.CONSUME(vocabulary["add"])},
{ALT: () => this.CONSUME(vocabulary["subtract"])},
{ALT: () => this.CONSUME(vocabulary["multiply"])},
{ALT: () => this.CONSUME(vocabulary["divide"])},
{ALT: () => this.CONSUME(vocabulary["pow"])},
{ALT: () => this.CONSUME(vocabulary["percent_of"])},
{ALT: () => this.CONSUME(vocabulary["equals"])},
{ALT: () => this.CONSUME(vocabulary["not_equals"])},
Expand All @@ -128,9 +190,42 @@ export class ComputedExpressionColumnParser extends CstParser {
]);
});

/**
* A special rule for column names used as alias after `as` to prevent
* further evaluation of possible expressions.
*/
this.RULE("TerminalColumnName", () => {
this.CONSUME(vocabulary["columnName"]);
});

/**
* A rule for aliasing computed columns - placed at the top so that it
* is evaluated after everything else.
*
* TODO: make AS left evaluative by default: an expression like
* x + y + z as "abc" currently breaks to abc + z, when it should be
* x + abc.
*/
this.RULE("As", () => {
this.CONSUME(vocabulary["as"]);
this.SUBRULE(this.TerminalColumnName);
});

/**
* A column name, which can evaluate to a parenthetical expression,
* a functional column, or a literal column name - a string
* wrapped in double or single quotes.
*/
this.RULE("ColumnName", () => {
this.OR([{ALT: () => this.SUBRULE(this.ParentheticalExpression)}, {ALT: () => this.SUBRULE(this.FunctionComputedColumn)}, {ALT: () => this.CONSUME(vocabulary["columnName"])}], {
ERR_MSG: "Expected a column name (wrapped in double quotes) or a parenthesis-wrapped expression."
});
});

/**
* The rule for parenthetical expressions, which consume parentheses
* and resolve to this.Expression.
* and resolve to this.Expression. Because it is lowest in the
* tree, it is evaluated before everything else.
*/
this.RULE("ParentheticalExpression", () => {
this.CONSUME(vocabulary["leftParen"]);
Expand Down
Loading

0 comments on commit ae1dce8

Please sign in to comment.