Skip to content
Snippets Groups Projects
Unverified Commit 3d349035 authored by Cam Saul's avatar Cam Saul Committed by GitHub
Browse files

Remove `:metabase.lib.*/` keys from `:field` refs when converting to legacy (#33111) 

* Remove `:metabase.lib.*` keys from field refs when converting refs to legacy

* Fix hardcoded database ID
parent d1ad5194
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/lib/convert.cljc
+ 11
− 4
View file @ 3d349035
......@@ -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)))
......
test/metabase/lib/convert_test.cljc
+ 93
− 42
View file @ 3d349035
......@@ -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)))))))
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment