Skip to content
Snippets Groups Projects
Unverified Commit c820265e authored by github-automation-metabase's avatar github-automation-metabase Committed by GitHub
Browse files

[QP] Update breakouts in `:order-by` when setting time granularity (#49403) (#49411)


Previously these were omitted, and it caused invalid SQL to be
generated, since the ORDER BY (time unit not updated) and GROUP BY
(time unit updated) did not line up.

Fixes #49263.

Co-authored-by: default avatarBraden Shepherdson <braden@metabase.com>
parent 15ed2d31
No related branches found
No related tags found
No related merge requests found
......@@ -92,6 +92,14 @@
(mbql.u/wrap-field-id-if-needed field)
(parse-param-value-for-type query param-type param-value (mbql.u/unwrap-field-or-expression-clause field))]))
(defn- update-breakout-unit-in [query subkey target-field-id temporal-unit new-unit]
(lib.util.match/replace-in
query [:query subkey]
[(tag :guard #{:field :expression})
(_ :guard #(= target-field-id %))
(opts :guard #(= temporal-unit (:temporal-unit %)))]
[tag target-field-id (assoc opts :temporal-unit new-unit)]))
(defn- update-breakout-unit
[query
{[_dimension [_field target-field-id {:keys [base-type temporal-unit]}]] :target
......@@ -107,12 +115,9 @@
:is-curated true
:base-type base-type
:unit new-unit})))
(lib.util.match/replace-in
query [:query :breakout]
[(tag :guard #{:field :expression})
(_ :guard #(= target-field-id %))
(opts :guard #(= temporal-unit (:temporal-unit %)))]
[tag target-field-id (assoc opts :temporal-unit new-unit)])))
(-> query
(update-breakout-unit-in :breakout target-field-id temporal-unit new-unit)
(update-breakout-unit-in :order-by target-field-id temporal-unit new-unit))))
(defn expand
"Expand parameters for MBQL queries in `query` (replacing Dashboard or Card-supplied params with the appropriate
......
......@@ -432,3 +432,22 @@
(testing "Should prefer :value over :default"
(is (= 59
(venues-with-price {:default 1, :value 2})))))))
(deftest ^:parallel time-granularity-parameters-test
(testing "time granularity parameters should update the matching clause in the breakouts and order-by clauses"
(let [by-unit (fn [unit]
[:field
(mt/id :orders :created_at)
{:base-type :type/DateTimeWithLocalTZ, :temporal-unit unit}])
query {:database (mt/id)
:type :query
:query {:source-table (mt/id :orders)
:aggregation [[:count]]
:breakout [(by-unit :day)
(by-unit :month)]
:order-by [[:asc (by-unit :month)]]}
:parameters [{:type :temporal-unit
:target [:dimension (by-unit :month)]
:value :week}]}]
(is (= ["2016-04-30T00:00:00Z" "2016-04-24T00:00:00Z" 1]
(first (mt/rows (mt/process-query query))))))))
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment