Skip to content
Snippets Groups Projects
Unverified Commit 4a76c4b0 authored by Ariya Hidayat's avatar Ariya Hidayat Committed by GitHub
Browse files

Fix parsing of some aggregation functions with no args (#20432)

In an expression `COUNT/2`, `COUNT` there should be treated as a
function (i.e. `COUNT()`) instead of a field.
Another function is the same category is `CUMULATIVECOUNT`.
Both don't require any arguments, hence the parentheses are optional.
parent e6649aff
No related branches found
No related tags found
No related merge requests found
import { t } from "ttag";
import {
MBQL_CLAUSES,
getMBQLName,
parseDimension,
parseMetric,
......@@ -17,6 +18,7 @@ import {
useShorthands,
adjustCase,
adjustOptions,
transformNoArgFunction,
} from "metabase/lib/expressions/recursive-parser";
import { tokenize, TOKEN, OPERATOR } from "metabase/lib/expressions/tokenizer";
......@@ -43,7 +45,9 @@ export function diagnose(source, startRule, query) {
const token = tokens[i];
if (token.type === TOKEN.Identifier && source[token.start] !== "[") {
const functionName = source.slice(token.start, token.end);
if (getMBQLName(functionName)) {
const fn = getMBQLName(functionName);
const clause = fn ? MBQL_CLAUSES[fn] : null;
if (clause && clause.args.length > 0) {
const next = tokens[i + 1];
if (next.op !== OPERATOR.OpenParenthesis) {
return {
......@@ -121,6 +125,7 @@ function prattCompiler(source, startRule, query) {
passes: [
adjustOptions,
useShorthands,
transformNoArgFunction,
adjustCase,
expr => resolve(expr, startRule, resolveMBQLField),
],
......
......@@ -303,11 +303,30 @@ export const adjustCase = tree =>
return node;
});
// [ "dimension", "count "] becomes ["count"], since Count is a valid no-arg function
export const transformNoArgFunction = tree =>
modify(tree, node => {
if (Array.isArray(node)) {
const [operator, ...operands] = node;
if (operands.length === 1 && operator === "dimension") {
const text = operands[0];
const fn = getMBQLName(text.trim().toLowerCase());
// refering a function with no args?
if (fn && MBQL_CLAUSES[fn].args.length === 0) {
return [fn];
}
}
}
return node;
});
const pipe = (...fns) => x => fns.reduce((v, f) => f(v), x);
export const parse = pipe(
recursiveParse,
adjustOptions,
useShorthands,
transformNoArgFunction,
adjustCase,
);
......@@ -48,6 +48,9 @@ const isCompatible = (a, b) => {
if (a === "aggregation" && b === "number") {
return true;
}
if (a === "number" && b === "aggregation") {
return true;
}
return false;
};
......
......@@ -149,6 +149,11 @@ describe("metabase/lib/expressions/recursive-parser", () => {
expect(process("A and not X")).toEqual(["and", A, ["not", X]]);
});
it("should detect aggregation functions with no argument", () => {
expect(process("COUNT/2")).toEqual(["/", ["count"], 2]);
expect(process("1+CumulativeCount")).toEqual(["+", 1, ["cum-count"]]);
});
it("should resolve segments", () => {
expect(process("Expensive", "boolean")).toEqual(["segment", "Expensive"]);
expect(process("NOT LowVolume")).toEqual(["not", ["segment", "LowVolume"]]);
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment