From 140f7d2f71b67c1d2a01d8195468e46a24735949 Mon Sep 17 00:00:00 2001 From: Ariya Hidayat <ariya@metabase.com> Date: Wed, 12 May 2021 00:18:33 -0700 Subject: [PATCH] Dimension type inference: handle CASE and COALESCE (#15996) * Remove stray it.only --- .../lib/expressions/typeinferencer.js | 13 ++++-- .../expressions/typeinferencer.unit.spec.js | 16 ++++++- .../question/custom_column.cy.spec.js | 42 +++++++++++++++++++ 3 files changed, 66 insertions(+), 5 deletions(-) diff --git a/frontend/src/metabase/lib/expressions/typeinferencer.js b/frontend/src/metabase/lib/expressions/typeinferencer.js index 10e82feca3a..85b359e7bcb 100644 --- a/frontend/src/metabase/lib/expressions/typeinferencer.js +++ b/frontend/src/metabase/lib/expressions/typeinferencer.js @@ -32,9 +32,16 @@ export function infer(mbql, env) { return MONOTYPE.Boolean; } - if (op === "case" || op === "coalesce") { - // TODO - return MONOTYPE.Undefined; + if (op === "case") { + const clauses = mbql[1]; + const first = clauses[0]; + // TODO: type-checker must ensure the consistent types of all clauses. + return infer(first[1], env); + } + + if (op === "coalesce") { + // TODO: type-checker must ensure the consistent types of all arguments + return infer(mbql[1], env); } const func = MBQL_CLAUSES[op]; diff --git a/frontend/test/metabase/lib/expressions/typeinferencer.unit.spec.js b/frontend/test/metabase/lib/expressions/typeinferencer.unit.spec.js index 985723442be..95e160557f4 100644 --- a/frontend/test/metabase/lib/expressions/typeinferencer.unit.spec.js +++ b/frontend/test/metabase/lib/expressions/typeinferencer.unit.spec.js @@ -27,10 +27,13 @@ describe("metabase/lib/expressions/typeinferencer", () => { case "Price": return "number"; case "FirstName": + case "LastName": return "string"; case "BirthDate": + case "MiscDate": return "type/Temporal"; case "Location": + case "Place": return "type/Coordinate"; } } @@ -92,9 +95,18 @@ describe("metabase/lib/expressions/typeinferencer", () => { expect(type("[Location]")).toEqual("type/Coordinate"); }); - it.skip("should infer the result of CASE", () => { + it("should infer the result of CASE", () => { expect(type("CASE([X], 1, 2)")).toEqual("number"); expect(type("CASE([Y], 'this', 'that')")).toEqual("string"); - expect(type("CASE(BigSale, Price>100, Price>200)")).toEqual("boolean"); + expect(type("CASE([Z], [Price]>100, [Price]>200)")).toEqual("boolean"); + expect(type("CASE([ABC], [FirstName], [LastName])")).toEqual("string"); + expect(type("CASE([F], [BirthDate], [MiscDate])")).toEqual("type/Temporal"); + }); + + it("should infer the result of COALESCE", () => { + expect(type("COALESCE([Price])")).toEqual("number"); + expect(type("COALESCE([FirstName], [LastName])")).toEqual("string"); + expect(type("COALESCE([BirthDate], [MiscDate])")).toEqual("type/Temporal"); + expect(type("COALESCE([Place], [Location])")).toEqual("type/Coordinate"); }); }); 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 dc77ff4648e..e1f128aff1d 100644 --- a/frontend/test/metabase/scenarios/question/custom_column.cy.spec.js +++ b/frontend/test/metabase/scenarios/question/custom_column.cy.spec.js @@ -453,6 +453,48 @@ describe("scenarios > question > custom columns", () => { cy.findByText("Previous"); cy.findByText("Days"); }); + + it("should handle CASE", () => { + openOrdersTable({ mode: "notebook" }); + cy.findByText("Custom column").click(); + popover().within(() => { + cy.get("[contenteditable='true']") + .type("case([Discount] > 0, [Created At], [Product → Created At])") + .blur(); + cy.findByPlaceholderText("Something nice and descriptive").type( + "MiscDate", + ); + cy.findByRole("button", { name: "Done" }).click(); + }); + cy.findByText("Filter").click(); + popover() + .findByText("MiscDate") + .click(); + cy.findByPlaceholderText("Enter a number").should("not.exist"); + cy.findByText("Previous"); + cy.findByText("Days"); + }); + + it("should handle COALESCE", () => { + openOrdersTable({ mode: "notebook" }); + cy.findByText("Custom column").click(); + popover().within(() => { + cy.get("[contenteditable='true']") + .type("COALESCE([Product → Created At], [Created At])") + .blur(); + cy.findByPlaceholderText("Something nice and descriptive").type( + "MiscDate", + ); + cy.findByRole("button", { name: "Done" }).click(); + }); + cy.findByText("Filter").click(); + popover() + .findByText("MiscDate") + .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