From a195650fe4e5414e2e14f097a87a49150a93b671 Mon Sep 17 00:00:00 2001
From: Cam Saul <1455846+camsaul@users.noreply.github.com>
Date: Tue, 22 Aug 2023 09:07:45 -0700
Subject: [PATCH] Fix #33083 (#33395)
---
.../query_processor/util/add_alias_info.clj | 3 +-
.../util/add_alias_info_test.clj | 27 ++++++++++++
.../explicit_joins_test.clj | 44 +++++++++++++++++++
3 files changed, 73 insertions(+), 1 deletion(-)
diff --git a/src/metabase/query_processor/util/add_alias_info.clj b/src/metabase/query_processor/util/add_alias_info.clj
index b05cd1b5759..5da5d4ab927 100644
--- a/src/metabase/query_processor/util/add_alias_info.clj
+++ b/src/metabase/query_processor/util/add_alias_info.clj
@@ -105,7 +105,8 @@
[:field id-or-name opts]
;; this doesn't use [[mbql.u/update-field-options]] because this gets called a lot and the overhead actually adds up
;; a bit
- [:field id-or-name (remove-namespaced-options (dissoc opts :source-field))]
+ [:field id-or-name (remove-namespaced-options (cond-> (dissoc opts :source-field :effective-type)
+ (integer? id-or-name) (dissoc :base-type)))]
;; for `:expression` and `:aggregation` references, remove the options map if they are empty.
[:expression expression-name opts]
diff --git a/test/metabase/query_processor/util/add_alias_info_test.clj b/test/metabase/query_processor/util/add_alias_info_test.clj
index 741bd3c50c2..81190dc9e82 100644
--- a/test/metabase/query_processor/util/add_alias_info_test.clj
+++ b/test/metabase/query_processor/util/add_alias_info_test.clj
@@ -739,3 +739,30 @@
(#'add/field-alias-in-source-query
{:source-query source-query}
[:field "PRICE" {:base-type :type/Float}])))))))
+
+(deftest ^:parallel find-matching-field-ignore-MLv2-extra-type-info-in-field-opts-test
+ (testing "MLv2 refs can include extra info like `:base-type`; make sure we ignore that when finding matching refs (#33083)"
+ (let [source-query {:joins [{:alias "Card_2"
+ :source-query {:breakout [[:field 78 {:join-alias "Products"
+ :temporal-unit :month
+ ::add/source-table "Products"
+ ::add/source-alias "CREATED_AT"
+ ::add/desired-alias "Products__CREATED_AT"
+ ::add/position 0}]]
+ :aggregation [[:aggregation-options
+ [:distinct [:field 76 {:join-alias "Products"
+ ::add/source-table "Products"
+ ::add/source-alias "ID"}]]
+ {:name "count"
+ ::add/source-alias "count"
+ ::add/position 1
+ ::add/desired-alias "count"}]]}}]}
+ field-clause [:field 78 {:base-type :type/DateTime, :temporal-unit :month, :join-alias "Card_2"}]]
+ (is (=? [:field
+ 78
+ {:join-alias "Products"
+ :temporal-unit :month
+ ::add/source-table "Products"
+ ::add/source-alias "CREATED_AT"
+ ::add/desired-alias "Products__CREATED_AT"}]
+ (#'add/matching-field-in-join-at-this-level source-query field-clause))))))
diff --git a/test/metabase/query_processor_test/explicit_joins_test.clj b/test/metabase/query_processor_test/explicit_joins_test.clj
index 901376afb15..37ea536ec02 100644
--- a/test/metabase/query_processor_test/explicit_joins_test.clj
+++ b/test/metabase/query_processor_test/explicit_joins_test.clj
@@ -939,3 +939,47 @@
["Doohickey" "Balistreri-Ankunding" "2018-03-01T00:00:00Z" 315.36 3.1536]]
(mt/formatted-rows [str str str 2.0 4.0]
(qp/process-query query)))))))))
+
+(deftest mlv2-references-in-join-conditions-test
+ (testing "Make sure join conditions that contain MLv2-generated refs with extra info like `:base-type` work correctly (#33083)"
+ (mt/dataset sample-dataset
+ (t2.with-temp/with-temp [:model/Card {card-1-id :id} {:dataset_query
+ (mt/mbql-query reviews
+ {:joins [{:source-table $$products
+ :alias "Products"
+ :condition [:= $product_id &Products.products.id]
+ :fields :all}]
+ :breakout [!month.&Products.products.created_at]
+ :aggregation [[:distinct &Products.products.id]]
+ :filter [:= &Products.products.category "Doohickey"]})}
+ :model/Card {card-2-id :id} {:dataset_query
+ (mt/mbql-query reviews
+ {:joins [{:source-table $$products
+ :alias "Products"
+ :condition [:= $product_id &Products.products.id]
+ :fields :all}]
+ :breakout [!month.&Products.products.created_at]
+ :aggregation [[:distinct &Products.products.id]]
+ :filter [:= &Products.products.category "Gizmo"]})}]
+ (let [query {:database (mt/id)
+ :type :query
+ :query {:source-table (str "card__" card-1-id)
+ :joins [{:fields :all
+ :strategy :left-join
+ :alias "Card_2"
+ :condition [:=
+ [:field
+ "CREATED_AT"
+ {:base-type :type/DateTime, :temporal-unit :month}]
+ [:field
+ (mt/id :products :created_at)
+ {:base-type :type/DateTime
+ :temporal-unit :month
+ :join-alias "Card_2"}]]
+ :source-table (str "card__" card-2-id)}]
+ :order-by [[:asc [:field "CREATED_AT" {:base-type :type/DateTime}]]]
+ :limit 2}}]
+ (mt/with-native-query-testing-context query
+ (is (= [["2016-05-01T00:00:00Z" 3 nil nil]
+ ["2016-06-01T00:00:00Z" 2 "2016-06-01T00:00:00Z" 1]]
+ (mt/rows (qp/process-query query))))))))))
--
GitLab