Skip to content

Commit

Permalink
[Metric V2] When joining a metric add its aggregation clause
Browse files Browse the repository at this point in the history
Fixes #41932
  • Loading branch information
snoe committed May 6, 2024
1 parent 1da4f64 commit b3061a0
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 10 deletions.
10 changes: 7 additions & 3 deletions src/metabase/lib/join.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
[clojure.string :as str]
[inflections.core :as inflections]
[medley.core :as m]
[metabase.lib.aggregation :as lib.aggregation]
[metabase.lib.card :as lib.card]
[metabase.lib.common :as lib.common]
[metabase.lib.dispatch :as lib.dispatch]
Expand All @@ -14,6 +15,7 @@
[metabase.lib.join.util :as lib.join.util]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
[metabase.lib.metric.basics :as lib.metric.basics]
[metabase.lib.options :as lib.options]
[metabase.lib.query :as lib.query]
[metabase.lib.ref :as lib.ref]
Expand Down Expand Up @@ -515,9 +517,11 @@
(suggested-join-conditions query stage-number (joined-thing query a-join)))
a-join (cond-> a-join
(seq suggested-conditions) (with-join-conditions suggested-conditions))
a-join (add-default-alias query stage-number a-join)]
(lib.util/update-query-stage query stage-number update :joins (fn [existing-joins]
(conj (vec existing-joins) a-join))))))
a-join (add-default-alias query stage-number a-join)
metric (lib.metric.basics/join-metric query a-join)]
(cond-> (lib.util/update-query-stage query stage-number update :joins (fn [existing-joins]
(conj (vec existing-joins) a-join)))
metric (lib.aggregation/aggregate (lib.ref/ref metric))))))

(mu/defn joins :- [:maybe ::lib.schema.join/joins]
"Get all joins in a specific `stage` of a `query`. If `stage` is unspecified, returns joins in the final stage of the
Expand Down
15 changes: 15 additions & 0 deletions test/metabase/lib/join_test.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -1470,3 +1470,18 @@
(testing "but when editing the first join, Orders.USER_ID is not visible and no condition is suggested"
(is (=? nil
(lib/suggested-join-conditions query -1 (meta/table-metadata :people) 0)))))))))

(deftest ^:parallel joining-metric-should-add-aggregation
(let [mp (-> meta/metadata-provider
(lib.tu/metadata-provider-with-card-from-query 1 (-> lib.tu/venues-query (lib/aggregate (lib/count))) {:type :metric})
(lib.tu/metadata-provider-with-card-from-query 2 (-> lib.tu/venues-query (lib/aggregate (lib/count))) {:type :metric}))
query (-> (lib/query mp (meta/table-metadata :checkins))
(lib/join (lib.metadata/card mp 1))
(lib/join (lib.metadata/card mp 2)))
join-1 (get-in query [:stages 0 :joins 0 :alias])
join-2 (get-in query [:stages 0 :joins 1 :alias])]
(is (=? {:stages [{:joins [{:stages [{:source-card 1}]}
{:stages [{:source-card 2}]}]
:aggregation [[:metric {:join-alias join-1} 1]
[:metric {:join-alias join-2} 2]]}]}
query))))
9 changes: 2 additions & 7 deletions test/metabase/query_processor/middleware/metrics_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,7 @@
query (as-> (lib/query mp (meta/table-metadata :orders)) $q
(lib/join $q (lib/join-clause (lib.metadata/card mp (:id source-metric))
[(lib/= (meta/field-metadata :orders :product-id)
(meta/field-metadata :products :id))]))
(lib/aggregate $q (lib.options/ensure-uuid [:metric {:join-alias "Mock metric - Product"} (:id source-metric)])))]
(meta/field-metadata :products :id))])))]
(is (=?
;; joins get an extra, empty stage from 'fetch-source-query'
{:stages [{:joins [{:stages [{:source-table (meta/id :products)} {}]}]
Expand All @@ -179,10 +178,7 @@
q (as-> (lib/query mp products-table) $q
(lib/join $q (lib/join-clause m1))
(lib/join $q (lib/join-clause m2))
(lib/join $q (lib/join-clause m3))
(lib/aggregate $q (first (lib/available-metrics $q)))
(lib/aggregate $q (second (lib/available-metrics $q)))
(lib/aggregate $q (last (lib/available-metrics $q))))]
(lib/join $q (lib/join-clause m3)))]
(is (=? {:stages [{:aggregation [[:count {}]
[:avg {} [:field {:join-alias (:name m2)} (meta/id :orders :tax)]]
[:sum {} [:field {:join-alias (:name m3)} (meta/id :orders :tax)]]]}]}
Expand Down Expand Up @@ -243,7 +239,6 @@
(find-by-id (mt/id :orders :id) (lib/joinable-columns $q -1 metric))
join-alias)
(lib.metadata/field mp (mt/id :products :id)))]))
(lib/aggregate $q (first (lib/available-metrics $q)))
(lib/breakout $q (lib/with-temporal-bucket
(find-by-id (mt/id :orders :created_at) (lib/breakoutable-columns $q))
:month))
Expand Down

0 comments on commit b3061a0

Please sign in to comment.