Build datetime filters as datetime before question is saved (#26679)

frontend/src/metabase-lib/Dimension.ts

@@ -1264,6 +1264,10 @@ export class ExpressionDimension extends Dimension {
           base_type = "type/Boolean";
           break;
 
+        case MONOTYPE.DateTime:
+          base_type = "type/DateTime";
+          break;
+
         // fallback
         default:
           base_type = "type/Float";

frontend/src/metabase-lib/expressions/config.js

@@ -324,69 +324,69 @@ export const MBQL_CLAUSES = {
   "get-year": {
     displayName: `year`,
     type: "number",
-    args: ["expression"],
+    args: ["datetime"],
   },
   "get-quarter": {
     displayName: `quarter`,
     type: "number",
-    args: ["expression"],
+    args: ["datetime"],
   },
   "get-month": {
     displayName: `month`,
     type: "number",
-    args: ["expression"],
+    args: ["datetime"],
   },
   "get-week": {
     displayName: `week`,
     type: "number",
-    args: ["expression"],
+    args: ["datetime"],
     hasOptions: true, // optional mode parameter
   },
   "get-day": {
     displayName: `day`,
     type: "number",
-    args: ["expression"],
+    args: ["datetime"],
   },
   "get-day-of-week": {
     displayName: `weekday`,
     type: "number",
-    args: ["expression"],
+    args: ["datetime"],
   },
   "get-hour": {
     displayName: `hour`,
     type: "number",
-    args: ["expression"],
+    args: ["datetime"],
   },
   "get-minute": {
     displayName: `minute`,
     type: "number",
-    args: ["expression"],
+    args: ["datetime"],
   },
   "get-second": {
     displayName: `second`,
     type: "number",
-    args: ["expression"],
+    args: ["datetime"],
   },
   "datetime-diff": {
     displayName: `datetimeDiff`,
     type: "number",
-    args: ["expression", "expression", "string"],
+    args: ["datetime", "datetime", "string"],
     requiresFeature: "datetime-diff",
   },
   "datetime-add": {
     displayName: `datetimeAdd`,
-    type: "expression",
-    args: ["expression", "number", "string"],
+    type: "datetime",
+    args: ["datetime", "number", "string"],
   },
   "datetime-subtract": {
     displayName: `datetimeSubtract`,
-    type: "expression",
-    args: ["expression", "number", "string"],
+    type: "datetime",
+    args: ["datetime", "number", "string"],
   },
   "convert-timezone": {
     displayName: `convertTimezone`,
-    type: "expression",
-    args: ["expression", "string"],
+    type: "datetime",
+    args: ["datetime", "string"],
     hasOptions: true,
     requiresFeature: "convert-timezone",
   },

frontend/src/metabase-lib/expressions/resolver.js

@@ -38,11 +38,24 @@ function findMBQL(op) {
   return clause;
 }
 
+// a is the type of the argument expected,
+// as defined in MBQL_CLAUSES,
+// and b is the inferred type of the argument
 const isCompatible = (a, b) => {
   if (a === b) {
     return true;
   }
-  if (a === "expression" && (b === "number" || b === "string")) {
+
+  // if b is a string, then it can be an arg to a function that expects a datetime argument.
+  // This allows datetime string literals to work as args for functions that expect datetime types.
+  // FIXME: By doing this we are allowing string columns to be arguments to functions, which isn’t valid MBQL.
+  if (a === "datetime" && b === "string") {
+    return true;
+  }
+  if (
+    a === "expression" &&
+    (b === "datetime" || b === "number" || b === "string")
+  ) {
     return true;
   }
   if (a === "aggregation" && b === "number") {

frontend/src/metabase-lib/expressions/typeinferencer.js

@@ -6,6 +6,7 @@ export const MONOTYPE = {
   Number: "number",
   String: "string",
   Boolean: "boolean",
+  DateTime: "datetime",
 };
 
 export function infer(mbql, env) {
@@ -37,9 +38,6 @@ export function infer(mbql, env) {
     case "case":
       return infer(mbql[1][0][1], env);
     case "coalesce":
-    case "convert-timezone":
-    case "datetime-add":
-    case "datetime-subtract":
       return infer(mbql[1], env);
   }
 
@@ -47,6 +45,8 @@ export function infer(mbql, env) {
   if (func) {
     const returnType = func.type;
     switch (returnType) {
+      case "datetime":
+        return MONOTYPE.DateTime;
       case "object":
         return MONOTYPE.Undefined;
       case "aggregation":

frontend/test/metabase/lib/expressions/resolver.unit.spec.js

@@ -159,6 +159,66 @@ describe("metabase-lib/expressions/resolve", () => {
     it("should honor CONCAT's implicit casting", () => {
       expect(() => expr(["concat", ["coalesce", "B", 1]])).not.toThrow();
     });
+
+    describe("datetime functions", () => {
+      it("should resolve unchained functions", () => {
+        expect(() => expr(["get-week", "2022-01-01"])).not.toThrow();
+        expect(() =>
+          expr(["datetime-add", "2022-01-01", 1, "month"]),
+        ).not.toThrow();
+
+        // TODO: Implementation should be fine-tuned so that these throw
+        // as they are not really datetime
+        expect(() => expr(["get-day", A])).not.toThrow();
+        expect(() => expr(["get-day", "a"])).not.toThrow();
+        expect(() => expr(["get-day-of-week", A])).not.toThrow();
+        expect(() => expr(["get-day-of-week", "a"])).not.toThrow();
+        expect(() => expr(["get-week", A])).not.toThrow();
+        expect(() => expr(["get-week", "a"])).not.toThrow();
+        expect(() => expr(["get-month", A])).not.toThrow();
+        expect(() => expr(["get-month", "a"])).not.toThrow();
+        expect(() => expr(["get-quarter", A])).not.toThrow();
+        expect(() => expr(["get-quarter", "a"])).not.toThrow();
+        expect(() => expr(["get-year", A])).not.toThrow();
+        expect(() => expr(["get-year", "a"])).not.toThrow();
+      });
+
+      it("should resolve chained commmands", () => {
+        expect(() =>
+          expr([
+            "datetime-subtract",
+            ["datetime-add", "2022-01-01", 1, "month"],
+            2,
+            "minute",
+          ]),
+        ).not.toThrow();
+      });
+
+      it("should chain datetime functions onto functions of compatible types", () => {
+        expect(() =>
+          expr([
+            "concat",
+            ["datetime-add", "2022-01-01", 1, "month"],
+            "a string",
+          ]),
+        ).not.toThrow();
+      });
+
+      it("should throw if chaining datetime functions onto functions of incompatible types", () => {
+        expect(() =>
+          expr(["trim", ["datetime-add", "2022-01-01", 1, "month"]]),
+        ).toThrow();
+      });
+
+      it("should throw if passing numbers as arguments expected to be datetime", () => {
+        expect(() => expr(["get-day", 15])).toThrow();
+        expect(() => expr(["get-day-of-week", 6])).toThrow();
+        expect(() => expr(["get-week", 52])).toThrow();
+        expect(() => expr(["get-month", 12])).toThrow();
+        expect(() => expr(["get-quarter", 3])).toThrow();
+        expect(() => expr(["get-year", 2025])).toThrow();
+      });
+    });
   });
 
   describe("for aggregations", () => {

frontend/test/metabase/lib/expressions/typeinferencer.unit.spec.js

@@ -116,12 +116,19 @@ describe("metabase-lib/expressions/typeinferencer", () => {
   });
 
   it("should infer the result of datetimeAdd, datetimeSubtract", () => {
-    expect(type('datetimeAdd([CreatedAt], 2, "month")')).toEqual(
-      "type/Datetime",
-    );
+    expect(type('datetimeAdd([CreatedAt], 2, "month")')).toEqual("datetime");
+    expect(type('datetimeAdd("2022-01-01", 2, "month")')).toEqual("datetime");
     expect(type('datetimeSubtract([CreatedAt], 2, "month")')).toEqual(
-      "type/Datetime",
+      "datetime",
+    );
+    expect(type('datetimeSubtract("2022-01-01", 2, "month")')).toEqual(
+      "datetime",
     );
+    expect(
+      type(
+        'datetimeSubtract(datetimeAdd("2022-01-01", 2, "month"), 4, "minute")',
+      ),
+    ).toEqual("datetime");
   });
 
   it("should infer the result of datetimeExtract functions", () => {