From aed6976ad82987b309e3f7ea5f2c05a14208b19d Mon Sep 17 00:00:00 2001
From: Ariya Hidayat <ariya@metabase.com>
Date: Tue, 11 May 2021 23:54:39 -0700
Subject: [PATCH] Type inference can have access to the environment. (#15982)
In this case, to fetch the field type.
---
frontend/src/metabase-lib/lib/Dimension.js | 33 ++++++++++++-------
.../lib/expressions/typeinferencer.js | 6 +++-
.../expressions/typeinferencer.unit.spec.js | 24 ++++++++++++--
.../question/custom_column.cy.spec.js | 18 ++++++++++
4 files changed, 67 insertions(+), 14 deletions(-)
diff --git a/frontend/src/metabase-lib/lib/Dimension.js b/frontend/src/metabase-lib/lib/Dimension.js
index a8ad075ee8a..3b656dfc22a 100644
--- a/frontend/src/metabase-lib/lib/Dimension.js
+++ b/frontend/src/metabase-lib/lib/Dimension.js
@@ -1010,21 +1010,32 @@ export class ExpressionDimension extends Dimension {
if (query) {
const datasetQuery = query.query();
const expressions = datasetQuery ? datasetQuery.expressions : {};
- type = infer(expressions[this.name()]);
+ const env = mbql => {
+ const dimension = Dimension.parseMBQL(
+ mbql,
+ this._metadata,
+ this._query,
+ );
+ return dimension.field().base_type;
+ };
+ type = infer(expressions[this.name()], env);
} else {
type = infer(this._args[0]);
}
- let base_type = "type/Float"; // fallback
- switch (type) {
- case MONOTYPE.String:
- base_type = "type/Text";
- break;
- case MONOTYPE.Boolean:
- base_type = "type/Boolean";
- break;
- default:
- break;
+ let base_type = type;
+ if (!type.startsWith("type/")) {
+ base_type = "type/Float"; // fallback
+ switch (type) {
+ case MONOTYPE.String:
+ base_type = "type/Text";
+ break;
+ case MONOTYPE.Boolean:
+ base_type = "type/Boolean";
+ break;
+ default:
+ break;
+ }
}
return new Field({
diff --git a/frontend/src/metabase/lib/expressions/typeinferencer.js b/frontend/src/metabase/lib/expressions/typeinferencer.js
index 25d13256dc8..10e82feca3a 100644
--- a/frontend/src/metabase/lib/expressions/typeinferencer.js
+++ b/frontend/src/metabase/lib/expressions/typeinferencer.js
@@ -8,7 +8,7 @@ export const MONOTYPE = {
Boolean: "boolean",
};
-export function infer(mbql) {
+export function infer(mbql, env) {
if (!Array.isArray(mbql)) {
return typeof mbql;
}
@@ -50,5 +50,9 @@ export function infer(mbql) {
}
}
+ if (op === "field" && env) {
+ return env(mbql);
+ }
+
return MONOTYPE.Undefined;
}
diff --git a/frontend/test/metabase/lib/expressions/typeinferencer.unit.spec.js b/frontend/test/metabase/lib/expressions/typeinferencer.unit.spec.js
index 9fe75a94655..985723442be 100644
--- a/frontend/test/metabase/lib/expressions/typeinferencer.unit.spec.js
+++ b/frontend/test/metabase/lib/expressions/typeinferencer.unit.spec.js
@@ -3,7 +3,7 @@ import { infer } from "metabase/lib/expressions/typeinferencer";
describe("metabase/lib/expressions/typeinferencer", () => {
function resolve(kind, name) {
- return [kind, name];
+ return ["field", name];
}
function compileAs(source, startRule) {
let mbql = null;
@@ -22,8 +22,21 @@ describe("metabase/lib/expressions/typeinferencer", () => {
return mbql;
}
+ function mockEnv(fieldRef) {
+ switch (fieldRef[1]) {
+ case "Price":
+ return "number";
+ case "FirstName":
+ return "string";
+ case "BirthDate":
+ return "type/Temporal";
+ case "Location":
+ return "type/Coordinate";
+ }
+ }
+
function type(expression) {
- return infer(tryCompile(expression));
+ return infer(tryCompile(expression), mockEnv);
}
it("should infer the type of primitives", () => {
@@ -72,6 +85,13 @@ describe("metabase/lib/expressions/typeinferencer", () => {
expect(type("Length([Category]) > 0")).toEqual("boolean");
});
+ it("should relay the field type", () => {
+ expect(type("[Price]")).toEqual("number");
+ expect(type("([FirstName])")).toEqual("string");
+ expect(type("[BirthDate]")).toEqual("type/Temporal");
+ expect(type("[Location]")).toEqual("type/Coordinate");
+ });
+
it.skip("should infer the result of CASE", () => {
expect(type("CASE([X], 1, 2)")).toEqual("number");
expect(type("CASE([Y], 'this', 'that')")).toEqual("string");
diff --git a/frontend/test/metabase/scenarios/question/custom_column.cy.spec.js b/frontend/test/metabase/scenarios/question/custom_column.cy.spec.js
index ca4611b4247..dc77ff4648e 100644
--- a/frontend/test/metabase/scenarios/question/custom_column.cy.spec.js
+++ b/frontend/test/metabase/scenarios/question/custom_column.cy.spec.js
@@ -5,6 +5,7 @@ import {
_typeUsingPlaceholder,
openOrdersTable,
openProductsTable,
+ openPeopleTable,
visitQuestionAdhoc,
} from "__support__/e2e/cypress";
@@ -435,6 +436,23 @@ describe("scenarios > question > custom columns", () => {
cy.findByPlaceholderText("Enter a number").should("not.exist");
cy.findByPlaceholderText("Enter some text");
});
+
+ it("should relay the type of a date field", () => {
+ openPeopleTable({ mode: "notebook" });
+ cy.findByText("Custom column").click();
+ popover().within(() => {
+ _typeUsingGet("[contenteditable='true']", "[Birth Date]", 400);
+ _typeUsingPlaceholder("Something nice and descriptive", "DoB");
+ cy.findByText("Done").click();
+ });
+ cy.findByText("Filter").click();
+ popover()
+ .findByText("DoB")
+ .click();
+ cy.findByPlaceholderText("Enter a number").should("not.exist");
+ cy.findByText("Previous");
+ cy.findByText("Days");
+ });
});
it("should handle using `case()` when referencing the same column names (metabase#14854)", () => {
--
GitLab