diff --git a/src/metabase/lib/convert.cljc b/src/metabase/lib/convert.cljc
index d596c4aabc808b91fcd0908974f3bce47d049915..7dac73e2997991a92128e2f1b3b601c343394305 100644
--- a/src/metabase/lib/convert.cljc
+++ b/src/metabase/lib/convert.cljc
@@ -268,14 +268,21 @@
lib.dispatch/dispatch-value
:hierarchy lib.hierarchy/hierarchy)
+(defn- metabase-lib-keyword?
+ "Does keyword `k` have a`:lib/` or a `:metabase.lib.*/` namespace?"
+ [k]
+ (and (qualified-keyword? k)
+ (when-let [symb-namespace (namespace k)]
+ (or (= symb-namespace "lib")
+ (str/starts-with? symb-namespace "metabase.lib.")))))
+
(defn- disqualify
- "Remove any keys starting with the `:lib/` namespace from map `m`.
+ "Remove any keys starting with the `:lib/` `:metabase.lib.*/` namespaces from map `m`.
- No args = return transducer to remove `:lib/` keys from a map. One arg = update a map `m`."
+ No args = return transducer to remove keys from a map. One arg = update a map `m`."
([]
(remove (fn [[k _v]]
- (and (qualified-keyword? k)
- (= (namespace k) "lib")))))
+ (metabase-lib-keyword? k))))
([m]
(into {} (disqualify) m)))
diff --git a/test/metabase/lib/convert_test.cljc b/test/metabase/lib/convert_test.cljc
index 65e991250399b42f3e8fba15fe07be0d407d375e..66ee2ff03025d6c46166650e70bed17a4dc1d49c 100644
--- a/test/metabase/lib/convert_test.cljc
+++ b/test/metabase/lib/convert_test.cljc
@@ -92,17 +92,17 @@
(deftest ^:parallel ->pMBQL-native-query-test
(testing "template tag dimensions are converted"
- (let [original {:type :native,
+ (let [original {:type :native
:native
- {:query "SELECT count(*) AS count FROM PUBLIC.PEOPLE WHERE true [[AND {{NAME}}]]",
+ {:query "SELECT count(*) AS count FROM PUBLIC.PEOPLE WHERE true [[AND {{NAME}}]]"
:template-tags
{"NAME"
- {:name "NAME",
- :display-name "Name",
- :type :dimension,
- :dimension [:field 866 nil],
- :widget-type :string/=,
- :default nil}}},
+ {:name "NAME"
+ :display-name "Name"
+ :type :dimension
+ :dimension [:field 866 nil]
+ :widget-type :string/=
+ :default nil}}}
:database 76}
converted (lib.convert/->pMBQL original)]
(is (=? {:stages [{:template-tags {"NAME" {:dimension [:field {:lib/uuid string?} 866]}}}]}
@@ -239,16 +239,16 @@
;; Miscellaneous queries that have caused test failures in the past, captured here for quick feedback.
(are [query] (test-round-trip query)
;; query with segment
- {:database 282,
- :type :query,
- :query {:source-table 661,
- :aggregation [[:metric "ga:totalEvents"]],
+ {:database 282
+ :type :query
+ :query {:source-table 661
+ :aggregation [[:metric "ga:totalEvents"]]
:filter [:and
[:segment "gaid::-4"]
[:segment 42]
[:= [:field 1972 nil] "Run Query"]
[:time-interval [:field 1974 nil] -30 :day]
- [:!= [:field 1973 nil] "(not set)" "url"]],
+ [:!= [:field 1973 nil] "(not set)" "url"]]
:breakout [[:field 1973 nil]]}}
;; :aggregation-options on a non-aggregate expression with an inner aggregate.
@@ -319,12 +319,12 @@
:source-table 224}
:type :query}
- {:database 1,
- :type :query,
+ {:database 1
+ :type :query
:query
- {:source-table 2,
- :aggregation [[:count]],
- :breakout [[:field 14 {:temporal-unit :month}]],
+ {:source-table 2
+ :aggregation [[:count]]
+ :breakout [[:field 14 {:temporal-unit :month}]]
:order-by [[:asc [:aggregation 0]]]}}
{:database 23001
@@ -353,37 +353,37 @@
:limit 1
:source-table 4}}
- {:database 310,
- :query {:middleware {:disable-remaps? true},
- :source-card-id 1301,
- :source-query {:native "SELECT id, name, category_id, latitude, longitude, price FROM venues ORDER BY id ASC LIMIT 2"}},
+ {:database 310
+ :query {:middleware {:disable-remaps? true}
+ :source-card-id 1301
+ :source-query {:native "SELECT id, name, category_id, latitude, longitude, price FROM venues ORDER BY id ASC LIMIT 2"}}
:type :query}
- {:type :native,
+ {:type :native
:native
{:query
- "SELECT \"PUBLIC\".\"VENUES\".\"ID\" AS \"ID\", \"PUBLIC\".\"VENUES\".\"NAME\" AS \"NAME\", \"PUBLIC\".\"VENUES\".\"CATEGORY_ID\" AS \"CATEGORY_ID\", \"PUBLIC\".\"VENUES\".\"LATITUDE\" AS \"LATITUDE\", \"PUBLIC\".\"VENUES\".\"LONGITUDE\" AS \"LONGITUDE\", \"PUBLIC\".\"VENUES\".\"PRICE\" AS \"PRICE\" FROM \"PUBLIC\".\"VENUES\" LIMIT 1048575",
+ "SELECT \"PUBLIC\".\"VENUES\".\"ID\" AS \"ID\", \"PUBLIC\".\"VENUES\".\"NAME\" AS \"NAME\", \"PUBLIC\".\"VENUES\".\"CATEGORY_ID\" AS \"CATEGORY_ID\", \"PUBLIC\".\"VENUES\".\"LATITUDE\" AS \"LATITUDE\", \"PUBLIC\".\"VENUES\".\"LONGITUDE\" AS \"LONGITUDE\", \"PUBLIC\".\"VENUES\".\"PRICE\" AS \"PRICE\" FROM \"PUBLIC\".\"VENUES\" LIMIT 1048575"
:params nil}
:database 2360}
- {:database 1,
- :native {:query "select 111 as my_number, 'foo' as my_string"},
- :parameters [{:target [:dimension [:field 16 {:source-field 5}]],
- :type :category,
- :value [:param-value]}],
+ {:database 1
+ :native {:query "select 111 as my_number, 'foo' as my_string"}
+ :parameters [{:target [:dimension [:field 16 {:source-field 5}]]
+ :type :category
+ :value [:param-value]}]
:type :native}
- {:type :native,
+ {:type :native
:native
- {:query "SELECT count(*) AS count FROM PUBLIC.PEOPLE WHERE true [[AND {{NAME}}]]",
+ {:query "SELECT count(*) AS count FROM PUBLIC.PEOPLE WHERE true [[AND {{NAME}}]]"
:template-tags
{"NAME"
- {:name "NAME",
- :display-name "Name",
- :type :dimension,
- :dimension [:field 866 nil],
- :widget-type :string/=,
- :default nil}}},
+ {:name "NAME"
+ :display-name "Name"
+ :type :dimension
+ :dimension [:field 866 nil]
+ :widget-type :string/=
+ :default nil}}}
:database 76}
{:database 1
@@ -399,18 +399,18 @@
;; #32055
{:type :query
:database 5
- :query {:source-table 5822,
+ :query {:source-table 5822
:expressions {"Additional Information Capture" [:coalesce
[:field 519195 nil]
[:field 519196 nil]
[:field 519194 nil]
[:field 519193 nil]
- "None"],
+ "None"]
"Additional Info Present" [:case
[[[:= [:expression "Additional Information Capture"] "None"]
"No"]]
- {:default "Yes"}]},
- :filter [:= [:field 518086 nil] "active"],
+ {:default "Yes"}]}
+ :filter [:= [:field 518086 nil] "active"]
:aggregation [[:aggregation-options
[:share [:= [:expression "Additional Info Present"] "Yes"]]
{:name "Additional Information", :display-name "Additional Information"}]]}}))
@@ -554,7 +554,7 @@
(deftest ^:parallel clean-test
(testing "irrecoverable queries"
- ;; Eventually we should get to a place where ->pMBQL throws an exception here,
+ ;; Eventually we should get to a place where ->pMBQL throws an exception here
;; but legacy e2e tests make this impossible right now
(is (= {:type :query
:query {}}
@@ -621,3 +621,54 @@
converted (lib.convert/->pMBQL query)]
(is (empty? (get-in converted [:stages 0 :breakout])))
(is (empty? (get-in converted [:stages 0 :group-by])))))))
+
+
+(deftest ^:parallel remove-namespaced-lib-keys-from-legacy-refs-test
+ (testing "namespaced lib keys should be removed when converting to legacy (#33012)"
+ (testing `lib.convert/disqualify
+ (is (= {:join-alias "O"
+ :temporal-unit :year}
+ (#'lib.convert/disqualify
+ {:join-alias "O"
+ :temporal-unit :year
+ :metabase.lib.field/original-effective-type :type/DateTimeWithLocalTZ}))))
+ (testing `lib.convert/->legacy-MBQL
+ (let [query (-> (lib/query meta/metadata-provider (meta/table-metadata :products))
+ (lib/join (-> (lib/join-clause (meta/table-metadata :orders))
+ (lib/with-join-alias "O")
+ (lib/with-join-conditions [(lib/=
+ (-> (meta/field-metadata :products :created-at)
+ lib/ref
+ (lib/with-temporal-bucket :year))
+ (-> (meta/field-metadata :orders :created-at)
+ (lib/with-join-alias "O")
+ lib/ref
+ (lib/with-temporal-bucket :year)))]))))]
+ (testing "sanity check: the pMBQL query should namespaced have keys (:metabase.lib.field/original-effective-type)"
+ (is (=? {:stages [{:joins [{:alias "O"
+ :conditions [[:=
+ {}
+ [:field
+ {:temporal-unit :year
+ :metabase.lib.field/original-effective-type :type/DateTimeWithLocalTZ}
+ (meta/id :products :created-at)]
+ [:field
+ {:join-alias "O"
+ :temporal-unit :year
+ :metabase.lib.field/original-effective-type :type/DateTimeWithLocalTZ}
+ (meta/id :orders :created-at)]]]}]}]}
+ query)))
+ (is (=? {:query {:joins [{:alias "O"
+ :condition [:=
+ [:field
+ (meta/id :products :created-at)
+ {:base-type :type/DateTimeWithLocalTZ
+ :temporal-unit :year
+ :metabase.lib.field/original-effective-type (symbol "nil #_\"key is not present.\"")}]
+ [:field
+ (meta/id :orders :created-at)
+ {:base-type :type/DateTimeWithLocalTZ
+ :join-alias "O"
+ :temporal-unit :year
+ :metabase.lib.field/original-effective-type (symbol "nil #_\"key is not present.\"")}]]}]}}
+ (lib.convert/->legacy-MBQL query)))))))