From e094c38bfe748c365a01e8cc713765dc6781efe8 Mon Sep 17 00:00:00 2001
From: lbrdnk <lbrdnk@users.noreply.github.com>
Date: Thu, 17 Oct 2024 21:06:40 +0200
Subject: [PATCH] Splice expression into appropriate stage during v2 metric
expansion (#48800)
* Fix expression stage
* Add test
* Parallel test
* Adjust test
* Adjust test
---
.../query_processor/middleware/metrics.clj | 7 +++---
.../middleware/metrics_test.clj | 25 +++++++++++++++++++
2 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/src/metabase/query_processor/middleware/metrics.clj b/src/metabase/query_processor/middleware/metrics.clj
index 89d34f7844e..805a36d9a93 100644
--- a/src/metabase/query_processor/middleware/metrics.clj
+++ b/src/metabase/query_processor/middleware/metrics.clj
@@ -64,8 +64,8 @@
not-empty)))
(defn- expression-with-name-from-source
- [query [_ {:lib/keys [expression-name]} :as expression]]
- (lib/expression query 0 expression-name expression))
+ [query agg-stage-index [_ {:lib/keys [expression-name]} :as expression]]
+ (lib/expression query agg-stage-index expression-name expression))
(defn- update-metric-query-expression-names
[metric-query unique-name-fn]
@@ -137,7 +137,8 @@
(:qp/stage-had-source-card (lib.util/query-stage query agg-stage-index)))))
(let [metric-query (update-metric-query-expression-names metric-query unique-name-fn)]
(as-> query $q
- (reduce expression-with-name-from-source $q (lib/expressions metric-query -1))
+ (reduce #(expression-with-name-from-source %1 agg-stage-index %2)
+ $q (lib/expressions metric-query -1))
(include-implicit-joins $q agg-stage-index metric-query)
(reduce #(lib/filter %1 agg-stage-index %2) $q (lib/filters metric-query -1))
(replace-metric-aggregation-refs $q agg-stage-index lookup)))
diff --git a/test/metabase/query_processor/middleware/metrics_test.clj b/test/metabase/query_processor/middleware/metrics_test.clj
index c8eb2cea6ed..e32643568d8 100644
--- a/test/metabase/query_processor/middleware/metrics_test.clj
+++ b/test/metabase/query_processor/middleware/metrics_test.clj
@@ -1,6 +1,7 @@
(ns metabase.query-processor.middleware.metrics-test
(:require
[clojure.test :refer [deftest is testing]]
+ [java-time.api :as t]
[mb.hawk.assert-exprs.approximately-equal :as =?]
[medley.core :as m]
[metabase.analytics.prometheus :as prometheus]
@@ -788,6 +789,30 @@
[u.date/temporal-str->iso8601-str int int 3.0]
(mt/rows (qp/process-query query)))))))))
+(deftest ^:parallel expressions-from-metrics-are-spliced-to-correct-stage-test
+ (testing "Integration test: Expression is spliced into correct stage during metric expansion (#48722)"
+ (mt/with-temp [:model/Card source-model {:dataset_query (mt/mbql-query orders {})
+ :database_id (mt/id)
+ :name "source model"
+ :type :model}
+ :model/Card metric {:dataset_query
+ (mt/mbql-query
+ orders
+ {:source-table (str "card__" (:id source-model))
+ :expressions {"somedays" [:datetime-diff
+ [:field "CREATED_AT" {:base-type :type/DateTime}]
+ (t/offset-date-time 2024 10 16 0 0 0)
+ :day]}
+ :aggregation [[:median [:expression "somedays" {:base-type :type/Integer}]]]})
+ :database_id (mt/id)
+ :name "somedays median"
+ :type :metric}]
+ (let [mp (lib.metadata.jvm/application-database-metadata-provider (mt/id))
+ query (-> (lib/query mp (lib.metadata/card mp (:id source-model)))
+ (lib/aggregate (lib.metadata/metric mp (:id metric))))]
+ (is (= 2162
+ (ffirst (mt/rows (qp/process-query query)))))))))
+
(deftest ^:parallel ^:mb/once fetch-referenced-metrics-test
(testing "Metric's aggregation `:name` is used in expanded aggregation (#48625)"
(let [mp (lib.metadata.jvm/application-database-metadata-provider (mt/id))
--
GitLab