[Metrics V2] Handle clashing expression names when adjusting metrics (#42984)

* [Metrics V2] Handle clashing expression names when adjusting metrics

Fixes #42782

* Address PR review

src/metabase/query_processor/middleware/metrics.clj

@@ -62,6 +62,24 @@
   [query [_ {:lib/keys [expression-name]} :as expression]]
   (lib/expression query 0 expression-name expression))
 
+(defn- update-metric-query-expression-names
+  [metric-query unique-name-fn]
+  (let [original+new-name-pairs (into []
+                                      (keep (fn [[_ {:lib/keys [expression-name]}]]
+                                              (let [new-name (unique-name-fn expression-name)]
+                                                (when-not (= new-name expression-name)
+                                                  [expression-name new-name]))))
+                                      (lib/expressions metric-query))]
+    (reduce
+      (fn [metric-query [original-name new-name]]
+        (let [expression (m/find-first (comp #{original-name} :lib/expression-name second) (lib/expressions metric-query))]
+          (lib/replace-clause
+            metric-query
+            expression
+            (lib/with-expression-name expression new-name))))
+      metric-query
+      original+new-name-pairs)))
+
 (defn- temp-query-at-stage-path
   [query stage-path]
   (cond-> query
@@ -72,18 +90,24 @@
   "Splices in metric definitions that are compatible with the query."
   [query path expanded-stages]
   (if-let [lookup (fetch-referenced-metrics query (:aggregation (first expanded-stages)))]
-    (let [new-query (reduce
+    (let [temp-query (temp-query-at-stage-path query path)
+          unique-name-fn (lib.util/unique-name-generator
+                           (:lib/metadata query)
+                           (map
+                             (comp :lib/expression-name second)
+                             (lib/expressions temp-query)))
+          new-query (reduce
                       (fn [query [_metric-id {metric-query :query}]]
                         (if (and (= (lib.util/source-table-id query) (lib.util/source-table-id metric-query))
-                                 (= 1 (lib/stage-count metric-query)))
-                          (let [{:keys [expressions filters]} (lib.util/query-stage metric-query 0)]
+                              (= 1 (lib/stage-count metric-query)))
+                          (let [metric-query (update-metric-query-expression-names metric-query unique-name-fn)]
                             (as-> query $q
-                              (reduce expression-with-name-from-source $q expressions)
-                              (reduce lib/filter $q filters)
+                              (reduce expression-with-name-from-source $q (lib/expressions metric-query 0))
+                              (reduce lib/filter $q (lib/filters metric-query 0))
                               (replace-metric-aggregation-refs $q 0 lookup)))
                           (throw (ex-info "Incompatible metric" {:query query
                                                                  :metric metric-query}))))
-                      (temp-query-at-stage-path query path)
+                      temp-query
                       lookup)]
       (:stages new-query))
     expanded-stages))

test/metabase/query_processor/middleware/metrics_test.clj

@@ -123,6 +123,28 @@
                      :aggregation [[:avg {} [:field {} (comp #{"rating"} u/lower-case-en)]]]}]}
           (adjust query)))))
 
+(deftest ^:parallel adjust-expression-name-collision-test
+  (let [[source-metric mp] (mock-metric (-> (basic-metric-query)
+                                            (lib/expression "foobar" (lib/+ 1 1))
+                                            (as-> $q
+                                              (lib/expression $q "qux" (lib/+ (lib/expression-ref $q "foobar") 1))
+                                              (lib/filter $q (lib/= (lib/expression-ref $q "qux") (lib/expression-ref $q "foobar"))))))
+        query (-> (lib/query mp (meta/table-metadata :products))
+                  (lib/expression "foobar" (lib/- 2 2))
+                  (as-> $q
+                    (lib/expression $q "qux" (lib/- (lib/expression-ref $q "foobar") 2))
+                    (lib/filter $q (lib/= (lib/expression-ref $q "foobar") (lib/expression-ref $q "qux"))))
+                  (lib/aggregate (lib.metadata/metric mp (:id source-metric))))]
+    (is (=?
+          {:stages [{:expressions [[:- {:lib/expression-name "foobar"} 2 2]
+                                   [:- {:lib/expression-name "qux"} [:expression {} "foobar"] 2]
+                                   [:+ {:lib/expression-name "foobar_2"} 1 1]
+                                   [:+ {:lib/expression-name "qux_2"} [:expression {} "foobar_2"] 1]]
+                     :filters [[:= {} [:expression {} "foobar"] [:expression {} "qux"]]
+                               [:= {} [:expression {} "qux_2"] [:expression {} "foobar_2"]]]
+                     :aggregation [[:avg {} [:field {} (meta/id :products :rating)]]]}]}
+          (adjust query)))))
+
 (deftest ^:parallel adjust-filter-test
   (let [[source-metric mp] (mock-metric (lib/filter (basic-metric-query) (lib/> (meta/field-metadata :products :price) 1)))
         query (-> (lib/query mp source-metric)