From f58359289e5d48938e6e24ac3ec0fbb7bff07013 Mon Sep 17 00:00:00 2001
From: Cam Saul <1455846+camsaul@users.noreply.github.com>
Date: Fri, 25 Aug 2023 08:57:28 -0700
Subject: [PATCH] Crazy fast QP tests! **I shaved 21 seconds off the test suite
this week alone!** (#33483)
---
test/metabase/query_processor/pivot_test.clj | 2 +-
test/metabase/query_processor/test_util.clj | 41 +++-
.../explicit_joins_test.clj | 171 ++++++++-------
test/metabase/test/data.clj | 27 ++-
test/metabase/test/data/impl.clj | 206 ++++++++++++------
5 files changed, 295 insertions(+), 152 deletions(-)
diff --git a/test/metabase/query_processor/pivot_test.clj b/test/metabase/query_processor/pivot_test.clj
index 19ee67ef7cc..14a7f823484 100644
--- a/test/metabase/query_processor/pivot_test.clj
+++ b/test/metabase/query_processor/pivot_test.clj
@@ -307,7 +307,7 @@
(is (= (mt/rows (qp.pivot/run-pivot-query query))
(mt/rows result))))))))))))
-(deftest pivot-with-order-by-test
+(deftest ^:parallel pivot-with-order-by-test
(testing "Pivot queries should work if there is an `:order-by` clause (#17198)"
(mt/dataset sample-dataset
(let [query (mt/mbql-query products
diff --git a/test/metabase/query_processor/test_util.clj b/test/metabase/query_processor/test_util.clj
index d26a5d47a99..6ceccfdc950 100644
--- a/test/metabase/query_processor/test_util.clj
+++ b/test/metabase/query_processor/test_util.clj
@@ -12,7 +12,10 @@
[mb.hawk.init]
[metabase.db.connection :as mdb.connection]
[metabase.driver :as driver]
+ [metabase.lib.core :as lib]
+ [metabase.lib.metadata.jvm :as lib.metadata.jvm]
[metabase.lib.metadata.protocols :as lib.metadata.protocols]
+ [metabase.lib.test-util :as lib.tu]
[metabase.models.field :refer [Field]]
[metabase.models.table :refer [Table]]
[metabase.query-processor :as qp]
@@ -487,18 +490,44 @@
[(:name table) (:name field)]))))
(t2/select-pks-set :model/Field :table_id [:in table-ids]))}))
+(defn- query-results [query]
+ (let [results (qp/process-query query)]
+ (or (get-in results [:data :results_metadata :columns])
+ (throw (ex-info "Missing [:data :results_metadata :columns] from query results" results)))))
+
+;;; TODO -- we should mark this deprecated, I just don't want to have to update a million usages.
(defn card-with-source-metadata-for-query
"Given an MBQL `query`, return the relevant keys for creating a Card with that query and matching `:result_metadata`.
(t2.with-temp/with-temp [Card card (qp.test-util/card-with-source-metadata-for-query
(data/mbql-query venues {:aggregation [[:count]]}))]
- ...)"
+ ...)
+
+ Prefer [[metadata-provider-with-card-with-metadata-for-query]] instead of using this going forward."
[query]
- (let [results (qp/process-userland-query query)
- metadata (or (get-in results [:data :results_metadata :columns])
- (throw (ex-info "Missing [:data :results_metadata :columns] from query results" results)))]
- {:dataset_query query
- :result_metadata metadata}))
+ {:dataset_query query
+ :result_metadata (query-results query)})
+
+(defn metadata-provider-with-card-with-query-and-actual-result-metadata
+ "Create an MLv2 metadata provide based on the app DB metadata provider that adds a Card with ID `1` with `query` and
+ `:result-metadata` based on actually running that query."
+ ([query]
+ (metadata-provider-with-card-with-query-and-actual-result-metadata
+ (lib.metadata.jvm/application-database-metadata-provider (data/id))
+ query))
+
+ ([base-metadata-provider query]
+ (lib/composed-metadata-provider
+ (lib.tu/mock-metadata-provider
+ {:cards [{:id 1
+ :name "Card 1"
+ :database-id 1
+ :dataset-query query
+ ;; use the base metadata provider here to run the query to get results so it gets warmed a bit for
+ ;; subsequent usage.
+ :result-metadata (qp.store/with-metadata-provider base-metadata-provider
+ (query-results query))}]})
+ base-metadata-provider)))
;;; ------------------------------------------------- Timezone Stuff -------------------------------------------------
diff --git a/test/metabase/query_processor_test/explicit_joins_test.clj b/test/metabase/query_processor_test/explicit_joins_test.clj
index 8ded096c9e4..086dd9b72fd 100644
--- a/test/metabase/query_processor_test/explicit_joins_test.clj
+++ b/test/metabase/query_processor_test/explicit_joins_test.clj
@@ -7,14 +7,16 @@
[metabase.driver.sql.query-processor :as sql.qp]
[metabase.driver.sql.query-processor-test-util :as sql.qp-test-util]
[metabase.driver.util :as driver.u]
- [metabase.models :refer [Card]]
+ [metabase.lib.core :as lib]
+ [metabase.lib.metadata.jvm :as lib.metadata.jvm]
+ [metabase.lib.test-util :as lib.tu]
[metabase.query-processor :as qp]
[metabase.query-processor-test.timezones-test :as timezones-test]
+ [metabase.query-processor.store :as qp.store]
[metabase.query-processor.test-util :as qp.test-util]
[metabase.test :as mt]
[metabase.test.data :as data]
- [metabase.test.data.interface :as tx]
- [toucan2.tools.with-temp :as t2.with-temp]))
+ [metabase.test.data.interface :as tx]))
(deftest ^:parallel explict-join-with-default-options-test
(testing "Can we specify an *explicit* JOIN using the default options?"
@@ -300,40 +302,41 @@
:order-by [[:asc $name]]
:limit 3}))))))))
-(deftest join-against-card-source-query-test
+(deftest ^:parallel join-against-card-source-query-test
(mt/test-drivers (mt/normal-drivers-with-feature :left-join)
(testing "Can we join against a `card__id` source query and use `:fields` `:all`?"
- (is (= {:rows
- [[29 "20th Century Cafe" 12 37.775 -122.423 2 12 "Café"]
- [8 "25°" 11 34.1015 -118.342 2 11 "Burger"]
- [93 "33 Taps" 7 34.1018 -118.326 2 7 "Bar"]]
-
- :columns
- (mapv mt/format-name ["id" "name" "category_id" "latitude" "longitude" "price" "id_2" "name_2"])}
- (t2.with-temp/with-temp [Card {card-id :id} (qp.test-util/card-with-source-metadata-for-query (mt/mbql-query categories))]
+ (qp.store/with-metadata-provider (qp.test-util/metadata-provider-with-card-with-query-and-actual-result-metadata
+ (mt/mbql-query categories))
+ (is (= {:rows
+ [[29 "20th Century Cafe" 12 37.775 -122.423 2 12 "Café"]
+ [8 "25°" 11 34.1015 -118.342 2 11 "Burger"]
+ [93 "33 Taps" 7 34.1018 -118.326 2 7 "Bar"]]
+
+ :columns
+ (mapv mt/format-name ["id" "name" "category_id" "latitude" "longitude" "price" "id_2" "name_2"])}
(mt/format-rows-by [int identity int 4.0 4.0 int int identity]
(mt/rows+column-names
(mt/run-mbql-query venues
{:joins [{:alias "cat"
- :source-table (str "card__" card-id)
+ :source-table "card__1"
:fields :all
:condition [:= $category_id &cat.*categories.id]}]
:order-by [[:asc $name]]
:limit 3})))))))))
-(deftest join-on-field-literal-test
+(deftest ^:parallel join-on-field-literal-test
(mt/test-drivers (mt/normal-drivers-with-feature :left-join)
(testing "Can we join on a Field literal for a source query?"
- ;; Also: if you join against an *explicit* source query, do all columns for both queries come back? (Only applies
- ;; if you include `:source-metadata`)
- (is (= {:rows [[1 3 46 3] [2 9 40 9] [4 7 5 7]]
- :columns [(mt/format-name "venue_id") "count" (mt/format-name "category_id") "count_2"]}
- (mt/format-rows-by [int int int int]
- (mt/rows+column-names
- (t2.with-temp/with-temp [Card {card-id :id} (qp.test-util/card-with-source-metadata-for-query
- (mt/mbql-query venues
- {:aggregation [[:count]]
- :breakout [$category_id]}))]
+ (qp.store/with-metadata-provider (qp.test-util/metadata-provider-with-card-with-query-and-actual-result-metadata
+ (mt/mbql-query venues
+ {:aggregation [[:count]]
+ :breakout [$category_id]}))
+ ;; Also: if you join against an *explicit* source query, do all columns for both queries come back? (Only applies
+ ;; if you include `:source-metadata`)
+ (is (= {:rows [[1 3 46 3] [2 9 40 9] [4 7 5 7]]
+ :columns [(mt/format-name "venue_id") "count" (mt/format-name "category_id") "count_2"]}
+ (mt/format-rows-by [int int int int]
+ (mt/rows+column-names
(mt/run-mbql-query checkins
{:source-query {:source-table $$checkins
:aggregation [[:count]]
@@ -341,25 +344,25 @@
:joins
[{:fields :all
:alias "venues"
- :source-table (str "card__" card-id)
- :strategy :inner-join
+ :source-table "card__1"
+ :strategy :inner-join
:condition [:=
[:field "count" {:base-type :type/Number}]
[:field "count" {:base-type :type/Number, :join-alias "venues"}]]}]
:order-by [[:asc $venue_id]]
:limit 3})))))))))
-(deftest aggregate-join-results-test
+(deftest ^:parallel aggregate-join-results-test
(mt/test-drivers (mt/normal-drivers-with-feature :left-join)
(testing "Can we aggregate on the results of a JOIN?"
- (t2.with-temp/with-temp [Card {card-id :id} (qp.test-util/card-with-source-metadata-for-query
- (mt/mbql-query checkins
- {:aggregation [[:count]]
- :breakout [$user_id]}))]
+ (qp.store/with-metadata-provider (qp.test-util/metadata-provider-with-card-with-query-and-actual-result-metadata
+ (mt/mbql-query checkins
+ {:aggregation [[:count]]
+ :breakout [$user_id]}))
(let [query (mt/mbql-query users
{:joins [{:fields :all
:alias "checkins_by_user"
- :source-table (str "card__" card-id)
+ :source-table "card__1"
:condition [:= $id &checkins_by_user.*checkins.user_id]}]
:aggregation [[:avg &checkins_by_user.*count/Float]]
:breakout [!month.last_login]})]
@@ -377,13 +380,13 @@
(mt/rows+column-names
(qp/process-query query)))))))))))
-(deftest get-all-columns-without-metadata-test
+(deftest ^:parallel get-all-columns-without-metadata-test
(mt/test-drivers (mt/normal-drivers-with-feature :left-join)
(testing "NEW! Can we still get all of our columns, even if we *DON'T* specify the metadata?"
- (t2.with-temp/with-temp [Card {card-id :id} (qp.test-util/card-with-source-metadata-for-query
- (mt/mbql-query venues
- {:aggregation [[:count]]
- :breakout [$category_id]}))]
+ (qp.store/with-metadata-provider (qp.test-util/metadata-provider-with-card-with-query-and-actual-result-metadata
+ (mt/mbql-query venues
+ {:aggregation [[:count]]
+ :breakout [$category_id]}))
(is (= {:rows [[1 3 46 3] [2 9 40 9] [4 7 5 7]]
:columns [(mt/format-name "venue_id") "count" (mt/format-name "category_id") "count_2"]}
(mt/rows+column-names
@@ -392,7 +395,7 @@
{:source-query {:source-table $$checkins
:aggregation [[:count]]
:breakout [$venue_id]}
- :joins [{:source-table (str "card__" card-id)
+ :joins [{:source-table "card__1"
:alias "venues"
:fields :all
:strategy :inner-join
@@ -462,17 +465,22 @@
1 "Red Medicine" 4 10.065 -165.374 3]]
rows))))))
-(deftest sql-question-source-query-test
+(deftest ^:parallel sql-question-source-query-test
(mt/test-drivers (mt/normal-drivers-with-feature :nested-queries :left-join)
(testing "we should be able to use a SQL question as a source query in a Join"
- (t2.with-temp/with-temp [Card {card-id :id} (qp.test-util/card-with-source-metadata-for-query
- (mt/native-query (qp/compile (mt/mbql-query venues))))]
+ (qp.store/with-metadata-provider (let [app-db-provider (lib.metadata.jvm/application-database-metadata-provider (mt/id))]
+ (qp.test-util/metadata-provider-with-card-with-query-and-actual-result-metadata
+ app-db-provider
+ ;; use the same metadata provider to compile the query to native that we use
+ ;; to do subsequent steps gets warmed a bit.
+ (qp.store/with-metadata-provider app-db-provider
+ (mt/native-query (qp/compile (mt/mbql-query venues))))))
(is (= [[1 "2014-04-07T00:00:00Z" 5 12 12 "The Misfit Restaurant + Bar" 2 34.0154 -118.497 2]
[2 "2014-09-18T00:00:00Z" 1 31 31 "Bludso's BBQ" 5 33.8894 -118.207 2]]
(mt/formatted-rows [int identity int int int identity int 4.0 4.0 int]
(mt/run-mbql-query checkins
{:joins [{:fields :all
- :source-table (str "card__" card-id)
+ :source-table "card__1"
:alias "card"
:condition [:= $venue_id &card.venues.id]}]
:order-by [[:asc $id]]
@@ -697,7 +705,7 @@
(mt/formatted-rows [str int str int]
(qp/process-query query))))))))))
-(deftest join-against-multiple-saved-questions-with-same-column-test
+(deftest ^:parallel join-against-multiple-saved-questions-with-same-column-test
(testing "Should be able to join multiple against saved questions on the same column (#15863, #20362)"
(mt/test-drivers (mt/normal-drivers-with-feature :nested-queries :left-join)
(mt/dataset sample-dataset
@@ -708,20 +716,28 @@
{:post [(some? %)]}
(-> query qp/process-query :data :results_metadata :columns))
query-card (fn [query]
- {:dataset_query query, :result_metadata (metadata query)})]
- (mt/with-temp [Card {card-1-id :id} (query-card q1)
- Card {card-2-id :id} (query-card q2)
- Card {card-3-id :id} (query-card q3)]
+ {:name "Card"
+ :database-id (mt/id)
+ :dataset-query query
+ :result-metadata (metadata query)})]
+ (qp.store/with-metadata-provider (let [app-db-provider (lib.metadata.jvm/application-database-metadata-provider (mt/id))]
+ (lib/composed-metadata-provider
+ (lib.tu/mock-metadata-provider
+ {:cards (qp.store/with-metadata-provider app-db-provider
+ [(assoc (query-card q1) :id 1)
+ (assoc (query-card q2) :id 2)
+ (assoc (query-card q3) :id 3)])})
+ app-db-provider))
(let [query (mt/mbql-query products
- {:source-table (format "card__%d" card-1-id)
+ {:source-table "card__1"
:joins [{:fields :all
- :source-table (format "card__%d" card-2-id)
+ :source-table "card__2"
:condition [:=
$category
&Q2.category]
:alias "Q2"}
{:fields :all
- :source-table (format "card__%d" card-3-id)
+ :source-table "card__3"
:condition [:=
$category
&Q3.category]
@@ -940,30 +956,39 @@
(mt/formatted-rows [str str str 2.0 4.0]
(qp/process-query query)))))))))
-(deftest mlv2-references-in-join-conditions-test
+(deftest ^:parallel 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"]})}]
+ (qp.store/with-metadata-provider (lib/composed-metadata-provider
+ (lib.tu/mock-metadata-provider
+ {:cards [{:id 1
+ :name "Card 1"
+ :database-id (mt/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"]})}
+ {:id 2
+ :name "Card 2"
+ :database-id (mt/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"]})}]})
+ (lib.metadata.jvm/application-database-metadata-provider (mt/id)))
(let [query {:database (mt/id)
:type :query
- :query {:source-table (str "card__" card-1-id)
+ :query {:source-table "card__1"
:joins [{:fields :all
:strategy :left-join
:alias "Card_2"
@@ -976,9 +1001,9 @@
{: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}}]
+ :source-table "card__2"}]
+ :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]]
diff --git a/test/metabase/test/data.clj b/test/metabase/test/data.clj
index f939ad2769f..7131cdb9910 100644
--- a/test/metabase/test/data.clj
+++ b/test/metabase/test/data.clj
@@ -46,7 +46,7 @@
[metabase.test.data.impl :as data.impl]
[metabase.test.data.interface :as tx]
[metabase.test.data.mbql-query-impl :as mbql-query-impl]
- [metabase.util :as u]))
+ [metabase.util.malli :as mu]))
(set! *warn-on-reflection* true)
@@ -57,9 +57,9 @@
(defn db
"Return the current database.
- Relies on the dynamic variable `*get-db*`, which can be rebound with `with-db`."
+ Relies on the dynamic variable [[metabase.test.data.impl/*db-fn*]], which can be rebound with [[with-db]]."
[]
- (data.impl/*get-db*))
+ (data.impl/db))
(defmacro with-db
"Run body with `db` as the current database. Calls to `db` and `id` use this value."
@@ -187,22 +187,27 @@
[table-name & [query]]
`(run-mbql-query* (mbql-query ~table-name ~(or query {}))))
-(defn format-name
+(mu/defn format-name :- :string
"Format a SQL schema, table, or field identifier in the correct way for the current database by calling the current
driver's implementation of [[ddl.i/format-name]]. (Most databases use the default implementation of `identity`; H2
uses [[clojure.string/upper-case]].) This function DOES NOT quote the identifier."
- [a-name]
- (assert ((some-fn keyword? string? symbol?) a-name)
- (str "Cannot format `nil` name -- did you use a `$field` without specifying its Table? (Change the form to"
- " `$table.field`, or specify a top-level default Table to `$ids` or `mbql-query`.)"))
+ [a-name :- [:or
+ :keyword
+ :string
+ :symbol
+ [:fn
+ {:error/message (str "Cannot format `nil` name -- did you use a `$field` without specifying its Table? "
+ "(Change the form to `$table.field`, or specify a top-level default Table to "
+ "`$ids` or `mbql-query`.)")}
+ (constantly false)]]]
(ddl.i/format-name (tx/driver) (name a-name)))
(defn id
- "Get the ID of the current database or one of its Tables or Fields. Relies on the dynamic variable `*get-db*`, which
- can be rebound with `with-db`."
+ "Get the ID of the current database or one of its Tables or Fields. Relies on the dynamic
+ variable [[metabase.test.data.impl/*db-fn*]], which can be rebound with [[with-db]]."
([]
(mb.hawk.init/assert-tests-are-not-initializing "(mt/id ...) or (data/id ...)")
- (u/the-id (db)))
+ (data.impl/db-id))
([table-name]
(data.impl/the-table-id (id) (format-name table-name)))
diff --git a/test/metabase/test/data/impl.clj b/test/metabase/test/data/impl.clj
index 32eb64722e6..0c63002dc95 100644
--- a/test/metabase/test/data/impl.clj
+++ b/test/metabase/test/data/impl.clj
@@ -5,6 +5,7 @@
[metabase.db.query :as mdb.query]
[metabase.driver :as driver]
[metabase.driver.util :as driver.u]
+ [metabase.lib.schema.id :as lib.schema.id]
[metabase.models :refer [Database Field FieldValues Secret Table]]
[metabase.models.secret :as secret]
[metabase.plugins.classloader :as classloader]
@@ -13,6 +14,7 @@
[metabase.test.data.impl.verify :as verify]
[metabase.test.data.interface :as tx]
[metabase.util :as u]
+ [metabase.util.malli :as mu]
[potemkin :as p]
[toucan.db :as db]
[toucan2.core :as t2]))
@@ -44,81 +46,162 @@
([] (get-or-create-test-data-db! (tx/driver)))
([driver] (get-or-create-database! driver defs/test-data)))
-(def ^:dynamic *get-db*
+(def ^:dynamic ^{:arglists '([])} ^:private *db-fn*
"Implementation of `db` function that should return the current working test database when called, always with no
arguments. By default, this is [[get-or-create-test-data-db!]] for the current [[metabase.driver/*driver*]], which
does exactly what it suggests."
- get-or-create-test-data-db!)
+ #'get-or-create-test-data-db!)
-(defn do-with-db
- "Internal impl of `data/with-db`."
- [db f]
- (assert (and (map? db) (integer? (:id db)))
- (format "Not a valid database: %s" (pr-str db)))
- (binding [*get-db* (constantly db)]
- (f)))
+(mu/defn db :- [:map [:id ::lib.schema.id/database]]
+ []
+ (*db-fn*))
+
+(def ^:private ^:dynamic ^{:arglists '([])} *db-id-fn*
+ (let [f (mdb.connection/memoize-for-application-db
+ (fn [driver]
+ (u/the-id (get-or-create-test-data-db! driver))))]
+ (fn []
+ (f (tx/driver)))))
+
+(mu/defn db-id :- ::lib.schema.id/database
+ []
+ (*db-id-fn*))
+
+;;; ID lookup maps look like these:
+;;;
+;;; Table:
+;;;
+;;; {"VENUES" 10, "USERS" 11, "CHECKINS" 12, "CATEGORIES" 13}
+;;;
+;;; Field:
+;;;
+;;; [parent-id name] => ID
+;;;
+;;; {[nil "PRICE"] 71
+;;; [nil "CATEGORY_ID"] 72
+;;; [nil "DATE"] 79
+;;; [nil "PASSWORD"] 77
+;;; [nil "VENUE_ID"] 82
+;;; [nil "ID"] 81
+;;; [nil "NAME"] 84
+;;; [nil "ID"] 83
+;;; [nil "USER_ID"] 80
+;;; [nil "LAST_LOGIN"] 76
+;;; [nil "ID"] 75
+;;; [nil "LONGITUDE"] 70
+;;; [nil "LATITUDE"] 74
+;;; [nil "NAME"] 73
+;;; [nil "NAME"] 78
+;;; [nil "ID"] 69}
+
+(mu/defn ^:private build-table-lookup-map
+ [database-id :- ::lib.schema.id/database]
+ (t2/select-fn->pk (juxt (constantly database-id) :name)
+ [:model/Table :id :name]
+ :db_id database-id))
+
+(mu/defn ^:private build-field-lookup-map
+ [table-id :- ::lib.schema.id/table]
+ (t2/select-fn->pk (juxt :parent_id :name)
+ [:model/Field :id :name :parent_id]
+ :table_id table-id
+ :active true))
+
+(def ^:private ^{:arglists '([database-id])} table-lookup-map
+ (mdb.connection/memoize-for-application-db build-table-lookup-map))
+
+(def ^:private ^{:arglists '([field-lookup-map])} field-lookup-map
+ (mdb.connection/memoize-for-application-db build-field-lookup-map))
+
+(defn- cached-table-id [db-id table-name]
+ (get (table-lookup-map db-id) [db-id table-name]))
+
+(defn- cached-field-id [table-id parent-id field-name]
+ (get (field-lookup-map table-id) [parent-id field-name]))
+
+(mu/defn do-with-db
+ "Internal impl of [[metabase.test.data/with-db]]."
+ [db :- [:map [:id ::lib.schema.id/database]]
+ thunk :- fn?]
+ (binding [*db-fn* (constantly db)
+ *db-id-fn* (constantly (u/the-id db))]
+ (thunk)))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | id |
;;; +----------------------------------------------------------------------------------------------------------------+
-(defn the-table-id
- "Internal impl of `(data/id table)."
+(defn- table-id-from-app-db
[db-id table-name]
- {:pre [(integer? db-id) ((some-fn keyword? string?) table-name)]}
- (let [table-name (name table-name)
- table-id-for-name (partial t2/select-one-pk Table, :db_id db-id, :name)]
- (or (table-id-for-name table-name)
- (table-id-for-name (let [db-name (t2/select-one-fn :name Database :id db-id)]
- (tx/db-qualified-table-name db-name table-name)))
- (let [{driver :engine, db-name :name} (t2/select-one [Database :engine :name] :id db-id)]
- (throw
- (Exception. (format "No Table %s found for %s Database %d %s.\nFound: %s"
- (pr-str table-name) driver db-id (pr-str db-name)
- (u/pprint-to-str (t2/select-pk->fn :name Table, :db_id db-id, :active true)))))))))
-
-(defn- qualified-field-name [{parent-id :parent_id, field-name :name}]
+ (t2/select-one-pk [Table :id] :db_id db-id, :name table-name))
+
+(defn- throw-unfound-table-error [db-id table-name]
+ (let [{driver :engine, db-name :name} (t2/select-one [:model/Database :name :engine] :id db-id)]
+ (throw
+ (Exception. (format "No Table %s found for %s Database %d %s.\nFound: %s"
+ (pr-str table-name) driver db-id (pr-str db-name)
+ (u/pprint-to-str (t2/select-pk->fn :name Table, :db_id db-id, :active true)))))))
+
+(mu/defn the-table-id :- ::lib.schema.id/table
+ "Internal impl of `(data/id table)."
+ [db-id :- ::lib.schema.id/database
+ table-name :- :string]
+ (or (cached-table-id db-id table-name)
+ (table-id-from-app-db db-id table-name)
+ (let [db-name (t2/select-one-fn :name [:model/Database :name] :id db-id)
+ qualified-table-name (tx/db-qualified-table-name db-name table-name)]
+ (cached-table-id db-id qualified-table-name)
+ (table-id-from-app-db db-id qualified-table-name))
+ (throw-unfound-table-error db-id table-name)))
+
+(defn- field-id-from-app-db [table-id parent-id field-name]
+ (t2/select-one-pk Field, :active true, :table_id table-id, :name field-name, :parent_id parent-id))
+
+(defn- qualified-field-name [parent-id field-name]
(if parent-id
- (str (qualified-field-name (t2/select-one Field :id parent-id))
+ (str (t2/select-one-fn (fn [field]
+ (qualified-field-name (:parent_id field) (:name field)))
+ [:model/Field :parent_id :name]
+ :id parent-id)
\.
field-name)
field-name))
(defn- all-field-names [table-id]
- (into {} (for [field (t2/select Field :active true, :table_id table-id)]
- [(u/the-id field) (qualified-field-name field)])))
-
-(defn- the-field-id* [table-id field-name & {:keys [parent-id]}]
- (or (t2/select-one-pk Field, :active true, :table_id table-id, :name field-name, :parent_id parent-id)
- (let [{db-id :db_id, table-name :name} (t2/select-one [Table :name :db_id] :id table-id)
- db-name (t2/select-one-fn :name Database :id db-id)
- field-name (qualified-field-name {:parent_id parent-id, :name field-name})
- all-field-names (all-field-names table-id)]
- (throw
- (ex-info (format "Couldn't find Field %s for Table %s.\nFound:\n%s"
- (pr-str field-name) (pr-str table-name) (u/pprint-to-str all-field-names))
- {:field-name field-name
- :table table-name
- :table-id table-id
- :database db-name
- :database-id db-id
- :all-fields all-field-names})))))
-
-(defn the-field-id
+ (t2/select-fn->fn :id
+ (fn [field]
+ (qualified-field-name (:parent_id field) (:name field)))
+ [:model/Field :id :parent_id :name]
+ :active true, :table_id table-id))
+
+(defn- throw-unfound-field-errror
+ [table-id parent-id field-name]
+ (let [table-name (t2/select-one-fn [:model/Table :name] :id table-id)
+ field-name (qualified-field-name parent-id field-name)
+ all-field-names (all-field-names table-id)]
+ (throw
+ (ex-info (format "Couldn't find Field %s for Table %s.\nFound:\n%s"
+ (pr-str field-name) (pr-str table-name) (u/pprint-to-str all-field-names))
+ {:field-name field-name
+ :table table-name
+ :table-id table-id
+ :all-fields all-field-names}))))
+
+(defn- the-field-id* [table-id parent-id field-name]
+ (or (cached-field-id table-id parent-id field-name)
+ (field-id-from-app-db table-id parent-id field-name)
+ (throw-unfound-field-errror table-id parent-id field-name)))
+
+(mu/defn the-field-id :- ::lib.schema.id/field
"Internal impl of `(data/id table field)`."
- [table-id field-name & nested-field-names]
- {:pre [(integer? table-id)]}
- (doseq [field-name (cons field-name nested-field-names)]
- (assert ((some-fn keyword? string?) field-name)
- (format "Expected keyword or string field name; got ^%s %s"
- (some-> field-name class .getCanonicalName)
- (pr-str field-name))))
- (loop [parent-id (the-field-id* table-id field-name), [nested-field-name & more] nested-field-names]
+ [table-id :- ::lib.schema.id/table
+ field-name :- :string
+ & nested-field-names :- [:* :string]]
+ (loop [id (the-field-id* table-id nil field-name), [nested-field-name & more] nested-field-names]
(if-not nested-field-name
- parent-id
- (recur (the-field-id* table-id nested-field-name, :parent-id parent-id) more))))
-
+ id
+ (recur (the-field-id* table-id id nested-field-name) more))))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | with-temp-copy-of-db |
@@ -201,7 +284,7 @@
"Internal impl of [[metabase.test/with-temp-copy-of-db]]. Run `f` with a temporary Database that copies the details
from the standard test database, and syncs it."
[f]
- (let [{old-db-id :id, :as old-db} (*get-db*)
+ (let [{old-db-id :id, :as old-db} (*db-fn*)
original-db (-> old-db copy-secrets (select-keys [:details :engine :name]))
{new-db-id :id, :as new-db} (first (t2/insert-returning-instances! Database original-db))]
(try
@@ -236,8 +319,9 @@
(binding [db/*disable-db-logging* true]
(let [db (get-or-create-database! driver dbdef)]
(assert db)
- (assert (t2/exists? Database :id (u/the-id db)))
- db))))]
- (binding [*get-db* (fn []
- (get-db-for-driver (tx/driver)))]
+ (assert (pos-int? (:id db)))
+ db))))
+ db-fn #(get-db-for-driver (tx/driver))]
+ (binding [*db-fn* db-fn
+ *db-id-fn* #(u/the-id (db-fn))]
(f))))
--
GitLab