diff --git a/src/metabase/mbql/normalize.cljc b/src/metabase/mbql/normalize.cljc
index 676e5bb985b64d261fe3fd4a82c73132ad02d3c3..8da0f84160d4a2319474d35ca67cec404990ef2f 100644
--- a/src/metabase/mbql/normalize.cljc
+++ b/src/metabase/mbql/normalize.cljc
@@ -86,12 +86,30 @@
(cond-> [:aggregation aggregation-index]
(some? option) (conj option)))
+(defn- normalize-ref-opts [opts]
+ (let [opts (normalize-tokens opts :ignore-path)]
+ (cond-> opts
+ (:base-type opts) (update :base-type keyword)
+ (:temporal-unit opts) (update :temporal-unit keyword)
+ (:binning opts) (update :binning (fn [binning]
+ (cond-> binning
+ (:strategy binning) (update :strategy keyword)))))))
+
(defmethod normalize-mbql-clause-tokens :expression
;; For expression references (`[:expression \"my_expression\"]`) keep the arg as is but make sure it is a string.
- [[_ expression-name]]
- [:expression (if (keyword? expression-name)
- (mbql.u/qualified-name expression-name)
- expression-name)])
+ [[_ expression-name opts]]
+ (let [expression [:expression (if (keyword? expression-name)
+ (mbql.u/qualified-name expression-name)
+ expression-name)]
+ opts (->> opts
+ normalize-ref-opts
+ ;; Only keep fields required for handling binned&datetime expressions (#33528)
+ ;; Allowing added alias-info through here breaks
+ ;; [[metabase.query-processor.util.nest-query-test/nest-expressions-ignore-source-queries-test]]
+ (m/filter-keys #{:base-type :temporal-unit :binning})
+ not-empty)]
+ (cond-> expression
+ opts (conj opts))))
(defmethod normalize-mbql-clause-tokens :binning-strategy
;; For `:binning-strategy` clauses (which wrap other Field clauses) normalize the strategy-name and recursively
@@ -103,15 +121,9 @@
(defmethod normalize-mbql-clause-tokens :field
[[_ id-or-name opts]]
- (let [opts (normalize-tokens opts :ignore-path)]
- [:field
- id-or-name
- (cond-> opts
- (:base-type opts) (update :base-type keyword)
- (:temporal-unit opts) (update :temporal-unit keyword)
- (:binning opts) (update :binning (fn [binning]
- (cond-> binning
- (:strategy binning) (update :strategy keyword)))))]))
+ [:field
+ id-or-name
+ (normalize-ref-opts opts)])
(defmethod normalize-mbql-clause-tokens :field-literal
;; Similarly, for Field literals, keep the arg as-is, but make sure it is a string."
diff --git a/test/metabase/mbql/normalize_test.cljc b/test/metabase/mbql/normalize_test.cljc
index b5fa11eeb0e93d21ad55b092a64e852cfd354e95..309aad636bcc9b8b11c86cee761cf9b6a938baa9 100644
--- a/test/metabase/mbql/normalize_test.cljc
+++ b/test/metabase/mbql/normalize_test.cljc
@@ -558,37 +558,43 @@
;; this is also covered
(t/deftest ^:parallel normalize-expressions-test
(normalize-tests
- "Expression names should get normalized to strings."
- {{:query {"expressions" {:abc ["+" 1 2]}
- :fields [["expression" :abc]]}}
- {:query {:expressions {"abc" [:+ 1 2]}
- :fields [[:expression "abc"]]}}}
+ "Expression names should get normalized to strings."
+ {{:query {"expressions" {:abc ["+" 1 2]}
+ :fields [["expression" :abc]]}}
+ {:query {:expressions {"abc" [:+ 1 2]}
+ :fields [[:expression "abc"]]}}}
- "are expression names exempt from lisp-casing/lower-casing?"
- {{"query" {"expressions" {:sales_tax ["-" ["field-id" 10] ["field-id" 20]]}}}
- {:query {:expressions {"sales_tax" [:- [:field-id 10] [:field-id 20]]}}}}
+ "are expression names exempt from lisp-casing/lower-casing?"
+ {{"query" {"expressions" {:sales_tax ["-" ["field-id" 10] ["field-id" 20]]}}}
+ {:query {:expressions {"sales_tax" [:- [:field-id 10] [:field-id 20]]}}}}
- "expressions should handle datetime arithemtics"
- {{:query {:expressions {:prev_month ["+" ["field-id" 13] ["interval" -1 "month"]]}}}
- {:query {:expressions {"prev_month" [:+ [:field-id 13] [:interval -1 :month]]}}}
+ "expressions should handle datetime arithemtics"
+ {{:query {:expressions {:prev_month ["+" ["field-id" 13] ["interval" -1 "month"]]}}}
+ {:query {:expressions {"prev_month" [:+ [:field-id 13] [:interval -1 :month]]}}}
- {:query {:expressions {:prev_month ["-" ["field-id" 13] ["interval" 1 "month"] ["interval" 1 "day"]]}}}
- {:query {:expressions {"prev_month" [:- [:field-id 13] [:interval 1 :month] [:interval 1 :day]]}}}
+ {:query {:expressions {:prev_month ["-" ["field-id" 13] ["interval" 1 "month"] ["interval" 1 "day"]]}}}
+ {:query {:expressions {"prev_month" [:- [:field-id 13] [:interval 1 :month] [:interval 1 :day]]}}}
- {:query {:expressions {:datetime-diff ["datetime-diff" ["field" 1 nil] ["field" 2 nil] "month"]}}}
- {:query {:expressions {"datetime-diff" [:datetime-diff [:field 1 nil] [:field 2 nil] :month]}}}
+ {:query {:expressions {:datetime-diff ["datetime-diff" ["field" 1 nil] ["field" 2 nil] "month"]}}}
+ {:query {:expressions {"datetime-diff" [:datetime-diff [:field 1 nil] [:field 2 nil] :month]}}}
- {:query {:expressions {:datetime-add ["datetime-add" ["field" 1 nil] 1 "month"]}}}
- {:query {:expressions {"datetime-add" [:datetime-add [:field 1 nil] 1 :month]}}}
+ {:query {:expressions {:datetime-add ["datetime-add" ["field" 1 nil] 1 "month"]}}}
+ {:query {:expressions {"datetime-add" [:datetime-add [:field 1 nil] 1 :month]}}}
- {:query {:expressions {:datetime-subtract ["datetime-subtract" ["field" 1 nil] 1 "month"]}}}
- {:query {:expressions {"datetime-subtract" [:datetime-subtract [:field 1 nil] 1 :month]}}}}
+ {:query {:expressions {:datetime-subtract ["datetime-subtract" ["field" 1 nil] 1 "month"]}}}
+ {:query {:expressions {"datetime-subtract" [:datetime-subtract [:field 1 nil] 1 :month]}}}}
- "expressions handle namespaced keywords correctly"
- {{:query {"expressions" {:abc/def ["+" 1 2]}
- :fields [["expression" :abc/def]]}}
- {:query {:expressions {"abc/def" [:+ 1 2]}
- :fields [[:expression "abc/def"]]}}}))
+ "expressions handle namespaced keywords correctly"
+ {{:query {"expressions" {:abc/def ["+" 1 2]}
+ :fields [["expression" :abc/def]]}}
+ {:query {:expressions {"abc/def" [:+ 1 2]}
+ :fields [[:expression "abc/def"]]}}}
+
+ "expression refs can have opts (#33528)"
+ {{:query {:expressions {"abc" [+ 1 2]}
+ :fields [[:expression "abc" {"base-type" "type/Number"}]]}}
+ {:query {:expressions {"abc" [+ 1 2]}
+ :fields [[:expression "abc" {:base-type :type/Number}]]}}}))
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -1378,7 +1384,7 @@
(t/deftest ^:parallel normalize-fragment-filter-test
(t/is (= [:!=
- [:expression "expr"]
+ [:expression "expr" {:base-type :type/Date}]
[:field 66302 {:base-type :type/DateTime}]]
(mbql.normalize/normalize-fragment
[:query :filter]
diff --git a/test/metabase/query_processor_test/date_bucketing_test.clj b/test/metabase/query_processor_test/date_bucketing_test.clj
index a21831b4f83af7db5d3241d001090edea3380576..fbf4c4f4d15496aae2f85bb1bd7d2f0093f8c2b9 100644
--- a/test/metabase/query_processor_test/date_bucketing_test.clj
+++ b/test/metabase/query_processor_test/date_bucketing_test.clj
@@ -23,6 +23,7 @@
[metabase.driver.sql-jdbc.sync :as sql-jdbc.sync]
[metabase.driver.sql.query-processor :as sql.qp]
[metabase.driver.sql.query-processor-test-util :as sql.qp-test-util]
+ [metabase.lib.convert :as lib.convert]
[metabase.lib.core :as lib]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.jvm :as lib.metadata.jvm]
@@ -1343,6 +1344,28 @@
[[0]])
(mt/formatted-rows [int] (qp/process-query query)))))))))
+(deftest filter-by-expression-time-interval-test
+ (testing "Datetime expressions can filter to a date range (#33528)"
+ (mt/test-drivers (mt/normal-drivers-except #{:snowflake :athena})
+ (mt/dataset
+ checkins:1-per-day
+ (let [mp (lib.metadata.jvm/application-database-metadata-provider (mt/id))
+ query (as-> (lib/query mp (lib.metadata/table mp (mt/id :checkins))) $q
+ (lib/expression $q "customdate" (m/find-first (comp #{(mt/id :checkins :timestamp)} :id) (lib/visible-columns $q)))
+ (lib/filter $q (lib/time-interval (lib/expression-ref $q "customdate") :current :week)))
+ mbql-query (mt/mbql-query
+ checkins
+ {:expressions {"customdate" $timestamp}
+ :filter [:time-interval [:expression "customdate" {:base-type :type/DateTime}] :current :week]})
+ processed (qp/process-query query)
+ mbql-processed (qp/process-query mbql-query)]
+ ;; Test both path ways since only mbql-queries were affected.
+ (is (= 7 (count (mt/rows processed))))
+ (is (= 7 (count (mt/rows mbql-processed))))
+ (is (= (get-in (qp/process-query mbql-query) [:data :native_form])
+ (get-in (qp/process-query (lib.convert/->pMBQL mbql-query)) [:data :native_form])
+ (get-in (qp/process-query query) [:data :native_form]))))))))
+
;; TODO -- is this really date BUCKETING? Does this BELONG HERE?!
(deftest june-31st-test
(testing "What happens when you try to add 3 months to March 31st? It should still work (#10072, #21968, #21969)"