Skip to content
Snippets Groups Projects
Commit 1bbb0c7c authored by Ariya Hidayat's avatar Ariya Hidayat
Browse files

Custom expression: build a compact version of the syntax tree (#14579)

* Custom expression: build a compact version of the syntax tree

The Chevrotain-based parser produces a very verbose syntax tree
(matching the grammar productions on every step of the way).
For instance, a custom expression as simple as "1" results in
the following tree:

```
(root)
  expression
    relationalExpression
      additionExpression
        multiplicationExpression
          atomicExpression
            numberLiteral
```

This makes it hard for type-checker to perform its analysis, due to the
need to traverse deeply every one of its children (even if there is only
one direct child).

The compact syntax tree can be constructed from the above
representation, which trims the unnecessary intermediate nodes:

```
(root)
  numberLiteral
```
parent 003e73e1
Branches
Tags
No related merge requests found
......@@ -99,3 +99,89 @@ export function typeCheck(cst, rootType) {
checker.visit(cst);
return { typeErrors: checker.errors };
}
/*
Create a copy of the syntax tree where the unnecessary intermediate nodes
are not present anymore.
Example:
For a simple expression "42", the syntax tree produced by the parser is
expression <-- this is the root node
relationalExpression
additionExpression
multiplicationExpression
atomicExpression
numberLiteral
Meanwhile, the compact variant of the syntax tree:
numberLiteral
*/
export function compactSyntaxTree(node) {
if (!node) {
return;
}
const { name, children } = node;
switch (name) {
case "any":
case "aggregation":
case "atomicExpression":
case "boolean":
case "booleanExpression":
case "booleanUnaryExpression":
case "expression":
case "parenthesisExpression":
case "string":
if (children.expression) {
const expression = children.expression.map(compactSyntaxTree);
return expression.length === 1
? expression[0]
: { name, children: { expression: expression } };
}
break;
case "logicalNotExpression":
if (children.operands) {
const operands = children.operands.map(compactSyntaxTree);
const operators = children.operators;
return { name, children: { operators, operands } };
}
break;
case "additionExpression":
case "multiplicationExpression":
case "logicalAndExpression":
case "logicalOrExpression":
case "relationalExpression":
if (children.operands) {
const operands = children.operands.map(compactSyntaxTree);
const operators = children.operators;
return operands.length === 1
? operands[0]
: { name, children: { operators, operands } };
}
break;
case "functionExpression":
case "caseExpression": {
const { functionName, LParen, RParen } = children;
const args = children.arguments
? children.arguments.map(compactSyntaxTree)
: [];
return {
name,
children: { functionName, arguments: args, LParen, RParen },
};
}
default:
break;
}
return { name, children };
}
import { parse } from "metabase/lib/expressions/parser";
import { ExpressionVisitor } from "metabase/lib/expressions/visitor";
import { parseIdentifierString } from "metabase/lib/expressions/index";
import { compactSyntaxTree } from "metabase/lib/expressions/typechecker";
// Since the type checking is inserted as the last stage in the expression parser,
// the whole tests must continue to pass (i.e. none of them should thrown
......@@ -165,4 +166,67 @@ describe("type-checker", () => {
expect(aggregation("[X]+Sum([Y])").metrics).toEqual(["X"]);
});
});
describe("compactSyntaxTree", () => {
function exprRoot(source) {
const tokenVector = null;
const startRule = "expression";
const { cst } = parse({ source, tokenVector, startRule });
const compactCst = compactSyntaxTree(cst);
const { name } = compactCst;
return name;
}
function filterRoot(source) {
const tokenVector = null;
const startRule = "boolean";
const { cst } = parse({ source, tokenVector, startRule });
const compactCst = compactSyntaxTree(cst);
const { name } = compactCst;
return name;
}
it("should handle literals", () => {
expect(exprRoot("42")).toEqual("numberLiteral");
expect(exprRoot("(43)")).toEqual("numberLiteral");
expect(exprRoot("'Answer'")).toEqual("stringLiteral");
expect(exprRoot('"Answer"')).toEqual("stringLiteral");
expect(exprRoot('("The Answer")')).toEqual("stringLiteral");
});
it("should handle binary expressions", () => {
expect(exprRoot("1+2")).toEqual("additionExpression");
expect(exprRoot("3-4")).toEqual("additionExpression");
expect(exprRoot("1+2-3")).toEqual("additionExpression");
expect(exprRoot("(1+2-3)")).toEqual("additionExpression");
expect(exprRoot("(1+2)-3")).toEqual("additionExpression");
expect(exprRoot("1+(2-3)")).toEqual("additionExpression");
expect(exprRoot("5*6")).toEqual("multiplicationExpression");
expect(exprRoot("7/8")).toEqual("multiplicationExpression");
expect(exprRoot("5*6/7")).toEqual("multiplicationExpression");
expect(exprRoot("(5*6/7)")).toEqual("multiplicationExpression");
expect(exprRoot("5*(6/7)")).toEqual("multiplicationExpression");
expect(exprRoot("(5*6)/7")).toEqual("multiplicationExpression");
});
it("should handle function expressions", () => {
expect(exprRoot("LOWER(A)")).toEqual("functionExpression");
expect(exprRoot("UPPER(B)")).toEqual("functionExpression");
expect(filterRoot("BETWEEN(C,0,9)")).toEqual("functionExpression");
});
it("should handle case expressions", () => {
expect(exprRoot("CASE(X,1)")).toEqual("caseExpression");
expect(exprRoot("CASE(Y,2,3)")).toEqual("caseExpression");
});
it("should handle relational expressions", () => {
expect(filterRoot("1<2")).toEqual("relationalExpression");
expect(filterRoot("3>4")).toEqual("relationalExpression");
expect(filterRoot("5=6")).toEqual("relationalExpression");
expect(filterRoot("7!=8")).toEqual("relationalExpression");
});
it("should handle logical expressions", () => {
expect(filterRoot("A AND B")).toEqual("logicalAndExpression");
expect(filterRoot("C OR D")).toEqual("logicalOrExpression");
expect(filterRoot("A AND B OR C")).toEqual("logicalOrExpression");
expect(filterRoot("NOT E")).toEqual("logicalNotExpression");
expect(filterRoot("NOT NOT F")).toEqual("logicalNotExpression");
});
});
});
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment