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

MLv2: lib.metadata function simplification (#31917)

* MLv2: lib.metadata function simplification

* Kondo
parent 62857840
No related branches found
No related tags found
No related merge requests found
......@@ -184,63 +184,22 @@
(lib.metadata.protocols/tables (->metadata-provider metadata-providerable)))
(mu/defn table :- TableMetadata
"Find metadata for a specific Table, either by string `table-name`, and optionally `schema`, or by ID.
This supports legacy `card__<id>` style strings only for the benefit of legacy metadata: some column metadata comes
back like
{:table-id \"card__100\"}
and it is easier to work around that situation here than it is to update all of the various MetadataProviders to fix
things or to update all the places that call this function. Maybe in the future we can get rid of this icky
workaround."
([metadata-providerable :- MetadataProviderable
table-id :- ::lib.schema.id/table]
(lib.metadata.protocols/table (->metadata-provider metadata-providerable) table-id))
([metadata-providerable :- MetadataProviderable
table-schema :- [:maybe ::lib.schema.common/non-blank-string]
table-name :- ::lib.schema.common/non-blank-string]
(some (fn [table-metadata]
(when (and (or (nil? table-schema)
(= (:schema table-metadata) table-schema))
(= (:name table-metadata) table-name))
table-metadata))
(tables metadata-providerable))))
"Find metadata for a specific Table, either by string `table-name`, and optionally `schema`, or by ID."
[metadata-providerable :- MetadataProviderable
table-id :- ::lib.schema.id/table]
(lib.metadata.protocols/table (->metadata-provider metadata-providerable) table-id))
(mu/defn fields :- [:sequential ColumnMetadata]
"Get metadata about all the Fields belonging to a specific Table."
([metadata-providerable :- MetadataProviderable
table-id :- ::lib.schema.id/table]
(lib.metadata.protocols/fields (->metadata-provider metadata-providerable) table-id))
([metadata-provider
table-schema :- [:maybe ::lib.schema.common/non-blank-string]
table-name :- ::lib.schema.common/non-blank-string]
(fields metadata-provider
(:id (table metadata-provider table-schema table-name)))))
[metadata-providerable :- MetadataProviderable
table-id :- ::lib.schema.id/table]
(lib.metadata.protocols/fields (->metadata-provider metadata-providerable) table-id))
(mu/defn field :- ColumnMetadata
"Get metadata about a specific Field in the Database we're querying."
([metadata-providerable :- MetadataProviderable
field-id :- ::lib.schema.id/field]
(lib.metadata.protocols/field (->metadata-provider metadata-providerable) field-id))
;; TODO -- we need to figure out how to deal with nested fields... should field-name be a varargs thing?
([metadata-providerable :- MetadataProviderable
table-id :- ::lib.schema.id/table
field-name :- ::lib.schema.common/non-blank-string]
(some (fn [field-metadata]
(when (= (:name field-metadata) field-name)
field-metadata))
(fields metadata-providerable table-id)))
([metadata-providerable :- MetadataProviderable
table-schema :- [:maybe ::lib.schema.common/non-blank-string]
table-name :- ::lib.schema.common/non-blank-string
field-name :- ::lib.schema.common/non-blank-string]
(let [table-metadata (table metadata-providerable table-schema table-name)]
(field metadata-providerable (:id table-metadata) field-name))))
[metadata-providerable :- MetadataProviderable
field-id :- ::lib.schema.id/field]
(lib.metadata.protocols/field (->metadata-provider metadata-providerable) field-id))
;;;; Stage metadata
......
......@@ -5,7 +5,6 @@
[medley.core :as m]
[metabase.lib.convert :as lib.convert]
[metabase.lib.core :as lib]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
[metabase.lib.query :as lib.query]
[metabase.lib.schema.expression :as lib.schema.expression]
......@@ -196,7 +195,7 @@
(-> q
(lib/aggregate {:operator :sum
:lib/type :lib/external-op
:args [(lib/ref (lib.metadata/field q nil "VENUES" "CATEGORY_ID"))]})
:args [(lib/ref (meta/field-metadata :venues :category-id))]})
(dissoc :lib/metadata)))))))
(deftest ^:parallel type-of-sum-test
......
......@@ -352,7 +352,7 @@
{:strategy :num-bins :num-bins 10}
{:strategy :bin-width :bin-width 1.0}
{:strategy :default}])
:let [field-metadata (lib.metadata/field meta/metadata-provider "PUBLIC" "ORDERS" "SUBTOTAL")]
:let [field-metadata (meta/field-metadata :orders :subtotal)]
[what x] {"column metadata" field-metadata
"field ref" (lib/ref field-metadata)}
:let [x' (lib/with-binning x binning1)]]
......@@ -387,10 +387,10 @@
(deftest ^:parallel available-binning-strategies-test
(doseq [{:keys [expected-options field-metadata query]}
[{:query (lib/query meta/metadata-provider (meta/table-metadata :orders))
:field-metadata (lib.metadata/field meta/metadata-provider "PUBLIC" "ORDERS" "SUBTOTAL")
:field-metadata (meta/field-metadata :orders :subtotal)
:expected-options (lib.binning/numeric-binning-strategies)}
{:query (lib/query meta/metadata-provider (meta/table-metadata :people))
:field-metadata (lib.metadata/field meta/metadata-provider "PUBLIC" "PEOPLE" "LATITUDE")
:field-metadata (meta/field-metadata :people :latitude)
:expected-options (lib.binning/coordinate-binning-strategies)}]]
(testing (str (:semantic-type field-metadata) " Field")
(doseq [[what x] [["column metadata" field-metadata]
......@@ -426,7 +426,7 @@
(deftest ^:parallel binning-display-info-test
(testing "numeric binning"
(let [query (lib/query meta/metadata-provider (meta/table-metadata :orders))
field-metadata (lib.metadata/field meta/metadata-provider "PUBLIC" "ORDERS" "SUBTOTAL")
field-metadata (meta/field-metadata :orders :subtotal)
strategies (lib.binning/numeric-binning-strategies)]
(doseq [[strat exp] (zipmap strategies [{:display-name "Auto binned" :default true}
{:display-name "10 bins"}
......@@ -441,7 +441,7 @@
(testing "coordinate binning"
(let [query (lib/query meta/metadata-provider (meta/table-metadata :people))
field-metadata (lib.metadata/field meta/metadata-provider "PUBLIC" "PEOPLE" "LATITUDE")
field-metadata (meta/field-metadata :people :latitude)
strategies (lib.binning/coordinate-binning-strategies)]
(doseq [[strat exp] (zipmap strategies [{:display-name "Auto binned" :default true}
{:display-name "0.1°"}
......
......@@ -12,15 +12,13 @@
(apply f args))))
(deftest ^:parallel general-filter-clause-test
(let [q1 (lib/query meta/metadata-provider (meta/table-metadata :categories))
q2 (lib/saved-question-query meta/metadata-provider meta/saved-question)
q3 (lib/query meta/metadata-provider (meta/table-metadata :checkins))
venues-category-id-metadata (lib.metadata/field q1 nil "VENUES" "CATEGORY_ID")
venues-name-metadata (lib.metadata/field q1 nil "VENUES" "NAME")
venues-latitude-metadata (lib.metadata/field q1 nil "VENUES" "LATITUDE")
venues-longitude-metadata (lib.metadata/field q1 nil "VENUES" "LONGITUDE")
(let [q2 (lib/saved-question-query meta/metadata-provider meta/saved-question)
venues-category-id-metadata (meta/field-metadata :venues :category-id)
venues-name-metadata (meta/field-metadata :venues :name)
venues-latitude-metadata (meta/field-metadata :venues :latitude)
venues-longitude-metadata (meta/field-metadata :venues :longitude)
categories-id-metadata (lib.metadata/stage-column q2 -1 "ID")
checkins-date-metadata (lib.metadata/field q3 nil "CHECKINS" "DATE")]
checkins-date-metadata (meta/field-metadata :checkins :date)]
(testing "comparisons"
(doseq [[op f] [[:= lib/=]
[:!= lib/!=]
......@@ -109,7 +107,7 @@
(deftest ^:parallel filter-test
(let [q1 (lib/query meta/metadata-provider (meta/table-metadata :categories))
q2 (lib/saved-question-query meta/metadata-provider meta/saved-question)
venues-category-id-metadata (lib.metadata/field q1 nil "VENUES" "CATEGORY_ID")
venues-category-id-metadata (meta/field-metadata :venues :category-id)
original-filter
[:between
{:lib/uuid string?}
......@@ -145,7 +143,7 @@
(deftest ^:parallel add-filter-test
(let [simple-query (lib/query meta/metadata-provider (meta/table-metadata :categories))
venues-name-metadata (lib.metadata/field simple-query nil "VENUES" "NAME")
venues-name-metadata (meta/field-metadata :venues :name)
first-filter [:between
{:lib/uuid string?}
[:field
......
......@@ -49,8 +49,8 @@
j (lib/query meta/metadata-provider (meta/table-metadata :categories))]
(lib/join q (lib/join-clause j [{:lib/type :lib/external-op
:operator :=
:args [(lib/ref (lib.metadata/field q nil "VENUES" "CATEGORY_ID"))
(lib/ref (lib.metadata/field j nil "CATEGORIES" "ID"))]}]))))))
:args [(lib/ref (meta/field-metadata :venues :category-id))
(lib/ref (meta/field-metadata :categories :id))]}]))))))
(deftest ^:parallel join-saved-question-test
(is (=? {:lib/type :mbql/query
......@@ -83,7 +83,7 @@
(testing "Should be able to use raw Field metadatas in the join condition"
(let [q1 (lib/query meta/metadata-provider (meta/table-metadata :categories))
q2 (lib/saved-question-query meta/metadata-provider meta/saved-question)
venues-category-id-metadata (lib.metadata/field q1 nil "VENUES" "CATEGORY_ID")
venues-category-id-metadata (meta/field-metadata :venues :category-id)
categories-id-metadata (lib.metadata/stage-column q2 "ID")]
(let [clause (lib/join-clause q2 [(lib/= categories-id-metadata venues-category-id-metadata)])]
(is (=? {:lib/type :mbql/join
......
......@@ -11,14 +11,10 @@
#?(:cljs (comment metabase.test-runner.assert-exprs.approximately-equal/keep-me))
(deftest ^:parallel field-metadata-test
(are [x] (=? (merge
{:lib/type :metadata/column}
(meta/field-metadata :venues :category-id))
x)
(lib.metadata/field meta/metadata-provider (meta/id :venues :category-id))
(lib.metadata/field meta/metadata-provider (meta/id :venues) "CATEGORY_ID")
(lib.metadata/field meta/metadata-provider "PUBLIC" "VENUES" "CATEGORY_ID")
(lib.metadata/field meta/metadata-provider nil "VENUES" "CATEGORY_ID")))
(is (=? (merge
{:lib/type :metadata/column}
(meta/field-metadata :venues :category-id))
(lib.metadata/field meta/metadata-provider (meta/id :venues :category-id)))))
(deftest ^:parallel stage-metadata-test
(let [query (lib/saved-question-query meta/metadata-provider meta/saved-question)]
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment