diff --git a/src/metabase/query_processor/middleware/catch_exceptions.clj b/src/metabase/query_processor/middleware/catch_exceptions.clj
index 086a23f28e34254b5003e1933cefa07998ce7d13..e8e5eda82d605f3625c605f2adfb1660a65954ab 100644
--- a/src/metabase/query_processor/middleware/catch_exceptions.clj
+++ b/src/metabase/query_processor/middleware/catch_exceptions.clj
@@ -34,11 +34,15 @@
(defmethod format-exception ExceptionInfo
[e]
- (let [{error-type :type, :as data} (ex-data e)]
+ ;; `:is-curated` is a flag that signals whether the error message in `e` was approved by product
+ ;; to be shown to the user. It is used by FE.
+ (let [{error-type :type, is-curated :is-curated, :as data} (ex-data e)]
(merge
((get-method format-exception Throwable) e)
(when (qp.error-type/known-error-type? error-type)
{:error_type error-type})
+ (when is-curated
+ {:error_is_curated is-curated})
;; TODO - we should probably change this key to `:data` so we're not mixing lisp-case and snake_case keys
{:ex-data data})))
diff --git a/src/metabase/query_processor/middleware/parameters/mbql.clj b/src/metabase/query_processor/middleware/parameters/mbql.clj
index b75c16b5c2112207a2bb591170177724df0310ab..4d17a4d0d9a77a3d7e925a6b4f3b4eb52009524a 100644
--- a/src/metabase/query_processor/middleware/parameters/mbql.clj
+++ b/src/metabase/query_processor/middleware/parameters/mbql.clj
@@ -7,9 +7,13 @@
[metabase.legacy-mbql.util :as mbql.u]
[metabase.lib.convert :as lib.convert]
[metabase.lib.core :as lib]
+ [metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.protocols :as lib.metadata.protocols]
[metabase.lib.util.match :as lib.util.match]
+ [metabase.query-processor.error-type :as qp.error-type]
[metabase.query-processor.store :as qp.store]
+ [metabase.query-processor.util.temporal-bucket :as qp.u.temporal-bucket]
+ [metabase.util.i18n :refer [tru]]
[metabase.util.malli :as mu]))
(set! *warn-on-reflection* true)
@@ -90,9 +94,19 @@
(defn- update-breakout-unit
[query
- {[_dimension [_field target-field-id {:keys [temporal-unit]}]] :target
+ {[_dimension [_field target-field-id {:keys [base-type temporal-unit]}]] :target
:keys [value] :as _param}]
- (let [new-unit (keyword value)]
+ (let [new-unit (keyword value)
+ base-type (or base-type
+ (when (integer? target-field-id)
+ (:base-type (lib.metadata/field (qp.store/metadata-provider) target-field-id))))]
+ (assert (some? base-type) "`base-type` is not set.")
+ (when-not (qp.u.temporal-bucket/compatible-temporal-unit? base-type new-unit)
+ (throw (ex-info (tru "This chart can not be broken out by the selected unit of time: {0}." value)
+ {:type qp.error-type/invalid-query
+ :is-curated true
+ :base-type base-type
+ :unit new-unit})))
(lib.util.match/replace-in
query [:query :breakout]
[:field (_ :guard #(= target-field-id %)) (opts :guard #(= temporal-unit (:temporal-unit %)))]
diff --git a/src/metabase/query_processor/middleware/validate_temporal_bucketing.clj b/src/metabase/query_processor/middleware/validate_temporal_bucketing.clj
index f480c540576224bd7f94b6ea5f50b71c81d562c9..5c4c66d3c1374797dc5275447ef330c9401fea9f 100644
--- a/src/metabase/query_processor/middleware/validate_temporal_bucketing.clj
+++ b/src/metabase/query_processor/middleware/validate_temporal_bucketing.clj
@@ -1,33 +1,12 @@
(ns metabase.query-processor.middleware.validate-temporal-bucketing
(:require
- [clojure.set :as set]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.util.match :as lib.util.match]
[metabase.query-processor.error-type :as qp.error-type]
[metabase.query-processor.store :as qp.store]
+ [metabase.query-processor.util.temporal-bucket :as qp.u.temporal-bucket]
[metabase.util.i18n :refer [tru]]))
-(def ^:private valid-date-units
- #{:default :day :day-of-week :day-of-month :day-of-year
- :week :week-of-year :month :month-of-year :quarter :quarter-of-year :year})
-
-(def ^:private valid-time-units
- #{:default :millisecond :second :minute :minute-of-hour :hour :hour-of-day})
-
-(def ^:private valid-datetime-units (set/union valid-date-units valid-time-units))
-
-;; TODO -- this should be changed to `:effective-type` once we finish the metadata changes.
-(defmulti ^:private valid-units-for-base-type
- {:arglists '([base-type])}
- keyword)
-
-;; for stuff like UNIX timestamps -- skip validation for now. (UNIX timestamp should be bucketable with any unit
-;; anyway). Once `:effective-type` is in place, we can actually check those Fields here.
-(defmethod valid-units-for-base-type :type/* [_] valid-datetime-units)
-(defmethod valid-units-for-base-type :type/Date [_] valid-date-units)
-(defmethod valid-units-for-base-type :type/Time [_] valid-time-units)
-(defmethod valid-units-for-base-type :type/DateTime [_] valid-datetime-units)
-
(defn validate-temporal-bucketing
"Make sure temporal bucketing of Fields (i.e., `:datetime-field` clauses) in this query is valid given the combination
of Field base-type and unit. For example, you should not be allowed to bucket a `:type/Date` Field by `:minute`."
@@ -36,7 +15,7 @@
(let [base-type (if (integer? id-or-name)
(:base-type (lib.metadata/field (qp.store/metadata-provider) id-or-name))
base-type)
- valid-units (valid-units-for-base-type base-type)]
+ valid-units (qp.u.temporal-bucket/valid-units-for-base-type base-type)]
(when-not (valid-units temporal-unit)
(throw (ex-info (tru "Unsupported temporal bucketing: You can''t bucket a {0} Field by {1}."
base-type temporal-unit)
diff --git a/src/metabase/query_processor/util/temporal_bucket.clj b/src/metabase/query_processor/util/temporal_bucket.clj
new file mode 100644
index 0000000000000000000000000000000000000000..68b5042ba751a92b6fa8b29a5992a70ca9bafe6e
--- /dev/null
+++ b/src/metabase/query_processor/util/temporal_bucket.clj
@@ -0,0 +1,27 @@
+(ns metabase.query-processor.util.temporal-bucket
+ (:require
+ [metabase.lib.schema.temporal-bucketing :as lib.schema.temporal-bucketing]))
+
+(def ^:private valid-date-units (into #{:default} lib.schema.temporal-bucketing/date-bucketing-units))
+(def ^:private valid-time-units (into #{:default} lib.schema.temporal-bucketing/time-bucketing-units))
+(def ^:private valid-datetime-units lib.schema.temporal-bucketing/temporal-bucketing-units)
+
+;; TODO -- this should be changed to `:effective-type` once we finish the metadata changes.
+(defmulti valid-units-for-base-type
+ "Returns valid temrpoal units for the `base-type`."
+ {:arglists '([base-type])}
+ keyword)
+
+;; for stuff like UNIX timestamps -- skip validation for now. (UNIX timestamp should be bucketable with any unit
+;; anyway). Once `:effective-type` is in place, we can actually check those Fields here.
+(defmethod valid-units-for-base-type :type/* [_] valid-datetime-units)
+(defmethod valid-units-for-base-type :type/Date [_] valid-date-units)
+(defmethod valid-units-for-base-type :type/Time [_] valid-time-units)
+(defmethod valid-units-for-base-type :type/DateTime [_] valid-datetime-units)
+
+(defn compatible-temporal-unit?
+ "Check whether some column of `a-type` can be bucketted by the`temporal-unit`. Any column can be bucketed by `nil`
+ temporal unit."
+ [a-type temporal-unit]
+ (or (nil? temporal-unit)
+ (contains? (valid-units-for-base-type a-type) temporal-unit)))
diff --git a/test/metabase/query_processor/middleware/catch_exceptions_test.clj b/test/metabase/query_processor/middleware/catch_exceptions_test.clj
index 13d5a9cbbf295fc0eb4bfab3ebae2926b6a8c75e..2bf40a704527ff503da56c373de92ee14d8e5c59 100644
--- a/test/metabase/query_processor/middleware/catch_exceptions_test.clj
+++ b/test/metabase/query_processor/middleware/catch_exceptions_test.clj
@@ -28,7 +28,7 @@
(testing "Should nicely format a chain of exceptions, with the top-level Exception appearing first"
(testing "lowest-level error `:type` should be pulled up to the top-level"
(let [e1 (ex-info "1" {:level 1})
- e2 (ex-info "2" {:level 2, :type qp.error-type/qp} e1)
+ e2 (ex-info "2" {:level 2, :type qp.error-type/qp :is-curated true} e1)
e3 (ex-info "3" {:level 3} e2)]
(is (= {:status :failed
:class clojure.lang.ExceptionInfo
@@ -36,12 +36,13 @@
:stacktrace true
:error_type :qp
:ex-data {:level 1}
- :via [{:status :failed
- :class clojure.lang.ExceptionInfo
- :error "2"
- :stacktrace true
- :ex-data {:level 2, :type :qp}
- :error_type :qp}
+ :via [{:status :failed
+ :class clojure.lang.ExceptionInfo
+ :error "2"
+ :stacktrace true
+ :ex-data {:level 2, :type :qp, :is-curated true}
+ :error_type :qp
+ :error_is_curated true}
{:status :failed
:class clojure.lang.ExceptionInfo
:error "3"
diff --git a/test/metabase/query_processor_test/date_bucketing_test.clj b/test/metabase/query_processor_test/date_bucketing_test.clj
index d89b44514d76464f0b1cf9e38f889363805ced39..1f0ed84ff0b83e63da4836b2dca57a8332d55253 100644
--- a/test/metabase/query_processor_test/date_bucketing_test.clj
+++ b/test/metabase/query_processor_test/date_bucketing_test.clj
@@ -33,6 +33,7 @@
[metabase.query-processor :as qp]
[metabase.query-processor.compile :as qp.compile]
[metabase.query-processor.middleware.format-rows :as format-rows]
+ [metabase.query-processor.preprocess :as qp.preprocess]
[metabase.query-processor.test-util :as qp.test-util]
[metabase.test :as mt]
[metabase.test.data.interface :as tx]
@@ -1659,6 +1660,20 @@
(testing "annual"
(is (= [488444.41] (unit-totals "year")))))))
+(deftest ^:parallel incompatible-temporal-unit-parameter-test
+ (testing "Incompatible time unit parameter yields expected error"
+ (is (thrown-with-msg?
+ clojure.lang.ExceptionInfo #"This chart can not be broken out by the selected unit of time: minute\."
+ (qp.preprocess/preprocess
+ (mt/mbql-query
+ checkins
+ {:type :query
+ :query {:aggregation [[:count]]
+ :breakout [!day.date]}
+ :parameters [{:type :temporal-unit
+ :target [:dimension !day.date]
+ :value "minute"}]}))))))
+
(deftest day-of-week-custom-start-of-week-test
(mt/test-drivers (mt/normal-drivers)
(testing "`:day-of-week` bucketing should respect the `start-of-week` Setting (#13604)"