diff --git a/modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj b/modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj
index 67bb8b13e789ae42edad6c7150ceea0f2a50c01b..e1af0169d3b2c2d4662ec552771b145f8c9bef5d 100644
--- a/modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj
+++ b/modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj
@@ -19,13 +19,13 @@
[metabase.test
[data :as data]
[util :as tu]]
- [metabase.test.data.datasets :refer [expect-with-driver]]
+ [metabase.test.data.datasets :as datasets]
[metabase.test.util.timezone :as tu.tz]
[metabase.util.honeysql-extensions :as hx]
[toucan.util.test :as tt]))
;; Test native queries
-(expect-with-driver :bigquery
+(datasets/expect-with-driver :bigquery
[[100]
[99]]
(get-in (qp/process-query
@@ -38,7 +38,7 @@
[:data :rows]))
;;; table-rows-sample
-(expect-with-driver :bigquery
+(datasets/expect-with-driver :bigquery
[[1 "Red Medicine"]
[2 "Stout Burgers & Beers"]
[3 "The Apple Pan"]
@@ -52,10 +52,22 @@
;; make sure that BigQuery native queries maintain the column ordering specified in the SQL -- post-processing
;; ordering shouldn't apply (Issue #2821)
-(expect-with-driver :bigquery
- [{:name "venue_id", :display_name "venue_id", :source :native, :base_type :type/Integer}
- {:name "user_id", :display_name "user_id", :source :native, :base_type :type/Integer}
- {:name "checkins_id", :display_name "checkins_id", :source :native, :base_type :type/Integer}]
+(datasets/expect-with-driver :bigquery
+ [{:name "venue_id"
+ :display_name "venue_id"
+ :source :native
+ :base_type :type/Integer
+ :field_ref [:field-literal "venue_id" :type/Integer]}
+ {:name "user_id"
+ :display_name "user_id"
+ :source :native
+ :base_type :type/Integer
+ :field_ref [:field-literal "user_id" :type/Integer]}
+ {:name "checkins_id"
+ :display_name "checkins_id"
+ :source :native
+ :base_type :type/Integer
+ :field_ref [:field-literal "checkins_id" :type/Integer]}]
(qp.test/cols
(qp/process-query
{:native {:query (str "SELECT `test_data.checkins`.`venue_id` AS `venue_id`, "
@@ -67,7 +79,7 @@
:database (data/id)})))
;; make sure that the bigquery driver can handle named columns with characters that aren't allowed in BQ itself
-(expect-with-driver :bigquery
+(datasets/expect-with-driver :bigquery
{:rows [[113]]
:columns ["User_ID_Plus_Venue_ID"]}
(qp.test/rows+column-names
@@ -124,7 +136,7 @@
(#'bigquery/pre-alias-aggregations :bigquery {})))
-(expect-with-driver :bigquery
+(datasets/expect-with-driver :bigquery
{:rows [[7929 7929]], :columns ["sum" "sum_2"]}
(qp.test/rows+column-names
(qp/process-query {:database (data/id)
@@ -133,7 +145,7 @@
:aggregation [[:sum [:field-id (data/id :checkins :user_id)]]
[:sum [:field-id (data/id :checkins :user_id)]]]}})))
-(expect-with-driver :bigquery
+(datasets/expect-with-driver :bigquery
{:rows [[7929 7929 7929]], :columns ["sum" "sum_2" "sum_3"]}
(qp.test/rows+column-names
(qp/process-query {:database (data/id)
@@ -143,7 +155,7 @@
[:sum [:field-id (data/id :checkins :user_id)]]
[:sum [:field-id (data/id :checkins :user_id)]]]}})))
-(expect-with-driver :bigquery
+(datasets/expect-with-driver :bigquery
"UTC"
(tu/db-timezone-id))
@@ -151,7 +163,7 @@
;; make sure that BigQuery properly aliases the names generated for Join Tables. It's important to use the right
;; alias, e.g. something like `categories__via__category_id`, which is considerably different from what other SQL
;; databases do. (#4218)
-(expect-with-driver :bigquery
+(datasets/expect-with-driver :bigquery
(str "SELECT `categories__via__category_id`.`name` AS `name`,"
" count(*) AS `count` "
"FROM `test_data.venues` "
@@ -183,14 +195,14 @@
;; This query tests out the timezone handling of parsed dates. For this test a UTC date is returned, we should
;; read/return it as UTC
-(expect-with-driver :bigquery
+(datasets/expect-with-driver :bigquery
"2018-08-31T00:00:00.000Z"
(native-timestamp-query (data/id) "2018-08-31 00:00:00" "UTC"))
;; This test includes a `use-jvm-timezone` flag of true that will assume that the date coming from BigQuery is already
;; in the JVM's timezone. The test puts the JVM's timezone into America/Chicago an ensures that the correct date is
;; compared
-(expect-with-driver :bigquery
+(datasets/expect-with-driver :bigquery
"2018-08-31T00:00:00.000-05:00"
(tu.tz/with-jvm-tz (time/time-zone-for-id "America/Chicago")
(tt/with-temp* [Database [db {:engine :bigquery
@@ -199,7 +211,7 @@
(native-timestamp-query db "2018-08-31 00:00:00-05" "America/Chicago"))))
;; Similar to the above test, but covers a positive offset
-(expect-with-driver :bigquery
+(datasets/expect-with-driver :bigquery
"2018-08-31T00:00:00.000+07:00"
(tu.tz/with-jvm-tz (time/time-zone-for-id "Asia/Jakarta")
(tt/with-temp* [Database [db {:engine :bigquery
@@ -221,7 +233,7 @@
:query-hash (byte-array [1 2 3 4])}})
@native-query)))
-(expect-with-driver :bigquery
+(datasets/expect-with-driver :bigquery
(str
"-- Metabase:: userID: 1000 queryType: MBQL queryHash: 01020304\n"
"SELECT `test_data.venues`.`id` AS `id`,"
@@ -241,7 +253,7 @@
:query-hash (byte-array [1 2 3 4])}}))
;; let's make sure we're generating correct HoneySQL + SQL for aggregations
-(expect-with-driver :bigquery
+(datasets/expect-with-driver :bigquery
{:select [[(hx/identifier :field "test_data.venues" "price") (hx/identifier :field-alias "price")]
[(hsql/call :avg (hx/identifier :field "test_data.venues" "category_id")) (hx/identifier :field-alias "avg")]]
:from [(hx/identifier :table "test_data.venues")]
@@ -255,7 +267,7 @@
:breakout [$price]
:order-by [[:asc [:aggregation 0]]]}))))
-(expect-with-driver :bigquery
+(datasets/expect-with-driver :bigquery
{:query (str "SELECT `test_data.venues`.`price` AS `price`,"
" avg(`test_data.venues`.`category_id`) AS `avg` "
"FROM `test_data.venues` "
diff --git a/modules/drivers/druid/test/metabase/driver/druid_test.clj b/modules/drivers/druid/test/metabase/driver/druid_test.clj
index 16ece4145952b8d942bad0ceac084cdd953c12f3..2f37d59eded1e1268f85849332d0f9793b6e6481 100644
--- a/modules/drivers/druid/test/metabase/driver/druid_test.clj
+++ b/modules/drivers/druid/test/metabase/driver/druid_test.clj
@@ -102,12 +102,31 @@
:data {:rows [["2013-01-03T08:00:00.000Z" "931" "Simcha Yan" "1" "Kinaree Thai Bistro" 1]
["2013-01-10T08:00:00.000Z" "285" "Kfir Caj" "2" "Ruen Pair Thai Restaurant" 1]]
:cols (mapv #(merge col-defaults %)
- [{:name "timestamp", :source :native, :display_name "timestamp"}
- {:name "id", :source :native, :display_name "id"}
- {:name "user_name", :source :native, :display_name "user_name"}
- {:name "venue_price", :source :native, :display_name "venue_price"}
- {:name "venue_name", :source :native, :display_name "venue_name"}
- {:name "count", :source :native, :display_name "count", :base_type :type/Integer}])
+ [{:name "timestamp"
+ :source :native
+ :display_name "timestamp"
+ :field_ref [:field-literal "timestamp" :type/Text]}
+ {:name "id"
+ :source :native
+ :display_name "id"
+ :field_ref [:field-literal "id" :type/Text]}
+ {:name "user_name"
+ :source :native
+ :display_name "user_name"
+ :field_ref [:field-literal "user_name" :type/Text]}
+ {:name "venue_price"
+ :source :native
+ :display_name "venue_price"
+ :field_ref [:field-literal "venue_price" :type/Text]}
+ {:name "venue_name"
+ :source :native
+ :display_name "venue_name"
+ :field_ref [:field-literal "venue_name" :type/Text]}
+ {:name "count"
+ :source :native
+ :display_name "count"
+ :base_type :type/Integer
+ :field_ref [:field-literal "count" :type/Integer]}])
:native_form {:query native-query-1}}}
(-> (process-native-query native-query-1)
(m/dissoc-in [:data :insights])))
@@ -338,7 +357,7 @@
:breakout [[:field-id (data/id :checkins :venue_price)]]}})))))
(expect
- #"com.jcraft.jsch.JSchException:"
+ com.jcraft.jsch.JSchException
(try
(let [engine :druid
details {:ssl false
@@ -352,9 +371,12 @@
:tunnel-port 22
:tunnel-user "bogus"}]
(tu.log/suppress-output
- (driver.u/can-connect-with-details? engine details :throw-exceptions)))
- (catch Exception e
- (.getMessage e))))
+ (driver.u/can-connect-with-details? engine details :throw-exceptions)))
+ (catch Throwable e
+ (loop [^Throwable e e]
+ (or (when (instance? com.jcraft.jsch.JSchException e)
+ e)
+ (some-> (.getCause e) recur))))))
;; Query cancellation test, needs careful coordination between the query thread, cancellation thread to ensure
;; everything works correctly together
diff --git a/modules/drivers/googleanalytics/test/metabase/driver/googleanalytics_test.clj b/modules/drivers/googleanalytics/test/metabase/driver/googleanalytics_test.clj
index 8cbf2435e02b05547add749711a751185b4a4b14..8e2d56b401f715dc31dd1de611c0cc21fd2f5fe4 100644
--- a/modules/drivers/googleanalytics/test/metabase/driver/googleanalytics_test.clj
+++ b/modules/drivers/googleanalytics/test/metabase/driver/googleanalytics_test.clj
@@ -294,7 +294,7 @@
query (query-with-some-fields objects)]
(-> (tu/doall-recursive (qp query))
(update-in [:data :cols] #(for [col %]
- (dissoc col :table_id :id)))
+ (dissoc col :table_id :id :field_ref)))
(m/dissoc-in [:data :results_metadata])
(m/dissoc-in [:data :insights])))))))
diff --git a/modules/drivers/mongo/test/metabase/driver/mongo/util_test.clj b/modules/drivers/mongo/test/metabase/driver/mongo/util_test.clj
index ab30316f6f35e6ff26db90f1829ccf3835c88ba8..99d2e0f490c013a81188efc3141b293af9362986 100644
--- a/modules/drivers/mongo/test/metabase/driver/mongo/util_test.clj
+++ b/modules/drivers/mongo/test/metabase/driver/mongo/util_test.clj
@@ -3,9 +3,7 @@
[metabase.driver.mongo.util :as mongo-util]
[metabase.driver.util :as driver.u]
[metabase.test.util.log :as tu.log])
- (:import com.mongodb.ReadPreference
- (com.mongodb MongoClient DB ServerAddress MongoClientException)))
-
+ (:import [com.mongodb DB MongoClient MongoClientException ReadPreference ServerAddress]))
(defn- connect-mongo [opts]
(let [connection-info (#'mongo-util/details->mongo-connection-info
@@ -216,7 +214,7 @@
.build))
(expect
- #"We couldn't connect to the ssh tunnel host"
+ com.jcraft.jsch.JSchException
(try
(let [engine :mongo
details {:ssl false
@@ -231,5 +229,8 @@
:tunnel-user "bogus"}]
(tu.log/suppress-output
(driver.u/can-connect-with-details? engine details :throw-exceptions)))
- (catch Exception e
- (.getMessage e))))
+ (catch Throwable e
+ (loop [^Throwable e e]
+ (or (when (instance? com.jcraft.jsch.JSchException e)
+ e)
+ (some-> (.getCause e) recur))))))
diff --git a/modules/drivers/mongo/test/metabase/driver/mongo_test.clj b/modules/drivers/mongo/test/metabase/driver/mongo_test.clj
index 337b3ca02c2e86b2d80a31d117429643777a47c6..3b56602b87184db0798de65bf76532c5efdf3a34 100644
--- a/modules/drivers/mongo/test/metabase/driver/mongo_test.clj
+++ b/modules/drivers/mongo/test/metabase/driver/mongo_test.clj
@@ -79,7 +79,11 @@
{:status :completed
:row_count 1
:data {:rows [[1]]
- :cols [{:name "count", :display_name "count", :base_type :type/Integer, :source :native}]
+ :cols [{:name "count"
+ :display_name "count"
+ :base_type :type/Integer
+ :source :native
+ :field_ref [:field-literal "count" :type/Integer]}]
:native_form {:collection "venues"
:query native-query}}}
(-> (qp/process-query {:native {:query native-query
diff --git a/modules/drivers/sqlserver/test/metabase/test/data/sqlserver.clj b/modules/drivers/sqlserver/test/metabase/test/data/sqlserver.clj
index 57d61c7006ba5b80f50afd300cbd8df818733c5b..4f0fc74bd7cc61c4cb036a9b72cf0e408991df16 100644
--- a/modules/drivers/sqlserver/test/metabase/test/data/sqlserver.clj
+++ b/modules/drivers/sqlserver/test/metabase/test/data/sqlserver.clj
@@ -5,8 +5,7 @@
[metabase.test.data
[interface :as tx]
[sql :as sql.tx]
- [sql-jdbc :as sql-jdbc.tx]]
- [metabase.util :as u]))
+ [sql-jdbc :as sql-jdbc.tx]]))
(sql-jdbc.tx/add-test-extensions! :sqlserver)
@@ -21,29 +20,13 @@
(defmethod sql.tx/field-base-type->sql-type [:sqlserver :type/Text] [_ _] "VARCHAR(254)")
(defmethod sql.tx/field-base-type->sql-type [:sqlserver :type/Time] [_ _] "TIME")
-
-(defonce ^:private ^{:doc "To kick other users off of the database when we destroy it, we `ALTER DATABASE SET
- SINGLE_USER ROLLBACK IMMEDIATE`. This has the side effect of preventing any other connections to the database. If
- our tests barf for any reason, we're left with a database that can't be connected to until the hanging connection
- gets killed at some indeterminate point in the future. In other cases, JDBC will attempt to reuse connections to the
- same database, which fail once it it's in SINGLE_USER mode.
-
- To prevent our tests from failing for silly reasons, we'll instead generate database names like
- `sad-toucan-incidents_100`. We'll pick a random number here."}
- db-name-suffix-number
- (rand-int 10000))
-
-(defn- +suffix [db-name]
- (str db-name \_ db-name-suffix-number))
-
(defmethod tx/dbdef->connection-details :sqlserver [_ context {:keys [database-name]}]
{:host (tx/db-test-env-var-or-throw :sqlserver :host)
:port (Integer/parseInt (tx/db-test-env-var-or-throw :sqlserver :port "1433"))
:user (tx/db-test-env-var-or-throw :sqlserver :user)
:password (tx/db-test-env-var-or-throw :sqlserver :password)
:db (when (= context :db)
- (+suffix database-name))})
-
+ database-name)})
(defmethod sql.tx/drop-db-if-exists-sql :sqlserver [_ {:keys [database-name]}]
;; Kill all open connections to the DB & drop it
@@ -52,38 +35,28 @@
ALTER DATABASE \"%s\" SET SINGLE_USER WITH ROLLBACK IMMEDIATE;
DROP DATABASE \"%s\";
END;"
- (repeat 3 (+suffix database-name))))
+ (repeat 3 database-name)))
(defmethod sql.tx/drop-table-if-exists-sql :sqlserver [_ {:keys [database-name]} {:keys [table-name]}]
- (let [db-name (+suffix database-name)]
+ (let [db-name database-name]
(format "IF object_id('%s.dbo.%s') IS NOT NULL DROP TABLE \"%s\".dbo.\"%s\";" db-name table-name db-name table-name)))
-(defmethod sql.tx/qualified-name-components :sqlserver
- ([_ db-name] [(+suffix db-name)])
- ([_ db-name table-name] [(+suffix db-name) "dbo" table-name])
- ([_ db-name table-name field-name] [(+suffix db-name) "dbo" table-name field-name]))
+(defn- server-spec []
+ (sql-jdbc.conn/connection-details->spec :sqlserver
+ (tx/dbdef->connection-details :sqlserver :server nil)))
-(defmethod sql.tx/pk-sql-type :sqlserver [_] "INT IDENTITY(1,1)")
+(defn- database-exists? [database-name]
+ (seq (jdbc/query (server-spec) (format "SELECT name FROM master.dbo.sysdatabases WHERE name = N'%s'" database-name))))
+;; skip recreating the DB if it already exists
+(defmethod tx/create-db! :sqlserver [driver {:keys [database-name], :as db-def} & options]
+ (if (database-exists? database-name)
+ (printf "SQL Server database '%s' already exists.\n" database-name)
+ (apply (get-method tx/create-db! :sql-jdbc/test-extensions) driver db-def options)))
-;; Clean up any leftover DBs that weren't destroyed by the last test run (eg, if it failed for some reason). This is
-;; important because we're limited to a quota of 30 DBs on RDS.
-;;
-;; This doesn't kill databases with active connections (i.e. CI instances testing against them) -- `DROP DATABASE`
-;; will fail if the DB has open connections
-(defmethod tx/before-run :sqlserver [_]
- (let [connection-spec (sql-jdbc.conn/connection-details->spec :sqlserver
- (tx/dbdef->connection-details :sqlserver :server nil))
- leftover-dbs (map :name (jdbc/query
- connection-spec
- (str "SELECT name "
- "FROM master.dbo.sysdatabases "
- "WHERE name NOT IN ('tempdb', 'master', 'model', 'msdb', 'rdsadmin');")))]
- (with-redefs [+suffix identity]
- (doseq [db leftover-dbs]
- (u/ignore-exceptions
- (printf "Deleting leftover SQL Server DB '%s'...\n" db)
- ;; Don't try to kill other connections to this DB with SET SINGLE_USER -- some other instance (eg CI) might
- ;; be using it
- (jdbc/execute! connection-spec [(format "DROP DATABASE \"%s\";" db)])
- (println "[ok]"))))))
+(defmethod sql.tx/qualified-name-components :sqlserver
+ ([_ db-name] [db-name])
+ ([_ db-name table-name] [db-name "dbo" table-name])
+ ([_ db-name table-name field-name] [db-name "dbo" table-name field-name]))
+
+(defmethod sql.tx/pk-sql-type :sqlserver [_] "INT IDENTITY(1,1)")
diff --git a/project.clj b/project.clj
index 370e92e28e2e3251cc6036f95d1a32f16e96c4c6..22ee01f130c5e9b2a8cd3a3ac940914e0bc85b4b 100644
--- a/project.clj
+++ b/project.clj
@@ -99,7 +99,7 @@
com.sun.jdmk/jmxtools
com.sun.jmx/jmxri]]
[medley "1.2.0"] ; lightweight lib of useful functions
- [metabase/mbql "1.0.2"] ; MBQL language schema & util fns
+ [metabase/mbql "1.0.3"] ; MBQL language schema & util fns
[metabase/throttle "1.0.1"] ; Tools for throttling access to API endpoints and other code pathways
[javax.xml.bind/jaxb-api "2.4.0-b180830.0359"] ; add the `javax.xml.bind` classes which we're still using but were removed in Java 11
[net.sf.cssbox/cssbox "4.12" :exclusions [org.slf4j/slf4j-api]] ; HTML / CSS rendering
@@ -121,7 +121,7 @@
[org.eclipse.jetty/jetty-server "9.4.15.v20190215"] ; We require JDK 8 which allows us to run Jetty 9.4, ring-jetty-adapter runs on 1.7 which forces an older version
[ring/ring-json "0.4.0"] ; Ring middleware for reading/writing JSON automatically
[stencil "0.5.0"] ; Mustache templates for Clojure
- [toucan "1.11.0" :exclusions [org.clojure/java.jdbc honeysql]] ; Model layer, hydration, and DB utilities
+ [toucan "1.12.0" :exclusions [org.clojure/java.jdbc honeysql]] ; Model layer, hydration, and DB utilities
[weavejester/dependency "0.2.1"]] ; Dependency graphs and topological sorting
:main ^:skip-aot metabase.core
diff --git a/src/metabase/api/card.clj b/src/metabase/api/card.clj
index d5e05fdbcedcff4940c077ea2604d80396fd766e..fab99e4df3758c73f517244e5516b47db7cc149c 100644
--- a/src/metabase/api/card.clj
+++ b/src/metabase/api/card.clj
@@ -61,47 +61,51 @@
;;; ----------------------------------------------- Filtered Fetch Fns -----------------------------------------------
-;; TODO - rewrite the functions below as multimethod & impls
-(defn- cards:all
- "Return all `Cards`."
- []
+(defmulti ^:private cards-for-filter-option*
+ {:arglists '([filter-option & args])}
+ (fn [filter-option & _]
+ (keyword filter-option)))
+
+;; return all Cards. This is the default filter option.
+(defmethod cards-for-filter-option* :all
+ [_]
(db/select Card, :archived false, {:order-by [[:%lower.name :asc]]}))
-(defn- cards:mine
- "Return all `Cards` created by current user."
- []
+;; return Cards created by the current user
+(defmethod cards-for-filter-option* :mine
+ [_]
(db/select Card, :creator_id api/*current-user-id*, :archived false, {:order-by [[:%lower.name :asc]]}))
-(defn- cards:fav
- "Return all `Cards` favorited by the current user."
- []
+;;return all Cards favorited by the current user.
+(defmethod cards-for-filter-option* :fav
+ [_]
(->> (hydrate (db/select [CardFavorite :card_id], :owner_id api/*current-user-id*)
:card)
(map :card)
(filter (complement :archived))
(sort-by :name)))
-(defn- cards:database
- "Return all `Cards` belonging to `Database` with DATABASE-ID."
- [database-id]
+;; Return all Cards belonging to Database with `database-id`.
+(defmethod cards-for-filter-option* :database
+ [_ database-id]
(db/select Card, :database_id database-id, :archived false, {:order-by [[:%lower.name :asc]]}))
-(defn- cards:table
- "Return all `Cards` belonging to `Table` with TABLE-ID."
- [table-id]
+;; Return all Cards belonging to `Table` with `table-id`.
+(defmethod cards-for-filter-option* :table
+ [_ table-id]
(db/select Card, :table_id table-id, :archived false, {:order-by [[:%lower.name :asc]]}))
(s/defn ^:private cards-with-ids :- (s/maybe [CardInstance])
- "Return unarchived `Cards` with CARD-IDS.
- Make sure cards are returned in the same order as CARD-IDS`; `[in card-ids]` won't preserve the order."
+ "Return unarchived Cards with `card-ids`.
+ Make sure cards are returned in the same order as `card-ids`; `[in card-ids]` won't preserve the order."
[card-ids :- [su/IntGreaterThanZero]]
(when (seq card-ids)
(let [card-id->card (u/key-by :id (db/select Card, :id [:in (set card-ids)], :archived false))]
(filter identity (map card-id->card card-ids)))))
-(defn- cards:recent
- "Return the 10 `Cards` most recently viewed by the current user, sorted by how recently they were viewed."
- []
+;; Return the 10 Cards most recently viewed by the current user, sorted by how recently they were viewed.
+(defmethod cards-for-filter-option* :recent
+ [_]
(cards-with-ids (map :model_id (db/select [ViewLog :model_id [:%max.timestamp :max]]
:model "card"
:user_id api/*current-user-id*
@@ -109,37 +113,23 @@
:order-by [[:max :desc]]
:limit 10}))))
-(defn- cards:popular
- "All `Cards`, sorted by popularity (the total number of times they are viewed in `ViewLogs`).
- (yes, this isn't actually filtering anything, but for the sake of simplicitiy it is included amongst the filter
- options for the time being)."
- []
+;; All Cards, sorted by popularity (the total number of times they are viewed in `ViewLogs`). (yes, this isn't
+;; actually filtering anything, but for the sake of simplicitiy it is included amongst the filter options for the time
+;; being).
+(defmethod cards-for-filter-option* :popular
+ [_]
(cards-with-ids (map :model_id (db/select [ViewLog :model_id [:%count.* :count]]
:model "card"
{:group-by [:model_id]
:order-by [[:count :desc]]}))))
-(defn- cards:archived
- "`Cards` that have been archived."
- []
+;; Cards that have been archived.
+(defmethod cards-for-filter-option* :archived
+ [_]
(db/select Card, :archived true, {:order-by [[:%lower.name :asc]]}))
-(def ^:private filter-option->fn
- "Functions that should be used to return cards for a given filter option. These functions are all be called with
- `model-id` as the sole paramenter; functions that don't use the param discard it via `u/drop-first-arg`.
-
- ((filter->option->fn :recent) model-id) -> (cards:recent)"
- {:all (u/drop-first-arg cards:all)
- :mine (u/drop-first-arg cards:mine)
- :fav (u/drop-first-arg cards:fav)
- :database cards:database
- :table cards:table
- :recent (u/drop-first-arg cards:recent)
- :popular (u/drop-first-arg cards:popular)
- :archived (u/drop-first-arg cards:archived)})
-
-(defn- cards-for-filter-option [filter-option model-id]
- (-> ((filter-option->fn (or filter-option :all)) model-id)
+(defn- cards-for-filter-option [filter-option model-id-or-nil]
+ (-> (apply cards-for-filter-option* (or filter-option :all) (when model-id-or-nil [model-id-or-nil]))
(hydrate :creator :collection :favorite)))
@@ -147,7 +137,7 @@
(def ^:private CardFilterOption
"Schema for a valid card filter option."
- (apply s/enum (map name (keys filter-option->fn))))
+ (apply s/enum (map name (keys (methods cards-for-filter-option*)))))
(api/defendpoint GET "/"
"Get all the Cards. Option filter param `f` can be used to change the set of Cards that are returned; default is
diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj
index 3787ec8bf2d6e836d44bac58090fdcc1465bece3..98f0996c794bea839143b84b007dabf8684a62f0 100644
--- a/src/metabase/models/dashboard.clj
+++ b/src/metabase/models/dashboard.clj
@@ -86,7 +86,7 @@
(defn- revert-dashboard!
"Revert a Dashboard to the state defined by `serialized-dashboard`."
- [dashboard-id user-id serialized-dashboard]
+ [_ dashboard-id user-id serialized-dashboard]
;; Update the dashboard description / name / permissions
(db/update! Dashboard dashboard-id, (dissoc serialized-dashboard :cards))
;; Now update the cards as needed
@@ -113,9 +113,9 @@
serialized-dashboard)
-(defn diff-dashboards-str
+(defn- diff-dashboards-str
"Describe the difference between two Dashboard instances."
- [dashboardâ‚ dashboardâ‚‚]
+ [_ dashboardâ‚ dashboardâ‚‚]
(when dashboardâ‚
(let [[removals changes] (diff dashboardâ‚ dashboardâ‚‚)
check-series-change (fn [idx card-changes]
@@ -155,8 +155,8 @@
revision/IRevisioned
(merge revision/IRevisionedDefaults
{:serialize-instance (fn [_ _ dashboard] (serialize-dashboard dashboard))
- :revert-to-revision! (u/drop-first-arg revert-dashboard!)
- :diff-str (u/drop-first-arg diff-dashboards-str)}))
+ :revert-to-revision! revert-dashboard!
+ :diff-str diff-dashboards-str}))
;;; +----------------------------------------------------------------------------------------------------------------+
diff --git a/src/metabase/query_processor/middleware/add_implicit_joins.clj b/src/metabase/query_processor/middleware/add_implicit_joins.clj
index 984ee00f089e3fa9b7fbdfe4a7402a2b69f1af4f..af8a5eac9dd3901dbb1d97ee8152a41faad89fdb 100644
--- a/src/metabase/query_processor/middleware/add_implicit_joins.clj
+++ b/src/metabase/query_processor/middleware/add_implicit_joins.clj
@@ -50,9 +50,6 @@
:where [:and
[:in :source-fk.id (set fk-field-ids)]
[:= :target-table.db_id (u/get-id (qp.store/database))]
- #_(if source-table-id
- [:= :source-fk.table_id source-table-id]
- true)
(mdb/isa :source-fk.special_type :type/FK)]})]
(for [{:keys [fk-name table-name], :as info} infos]
(assoc info :alias (join-alias table-name fk-name))))))
diff --git a/src/metabase/query_processor/middleware/annotate.clj b/src/metabase/query_processor/middleware/annotate.clj
index 08045b280ada43dfcb4fcbf1be065543ae040e23..a803cefa873c230e9883114aacba65a40bae9a84 100644
--- a/src/metabase/query_processor/middleware/annotate.clj
+++ b/src/metabase/query_processor/middleware/annotate.clj
@@ -1,6 +1,7 @@
(ns metabase.query-processor.middleware.annotate
"Middleware for annotating (adding type information to) the results of a query, under the `:cols` column."
(:require [clojure.string :as str]
+ [medley.core :as m]
[metabase
[driver :as driver]
[util :as u]]
@@ -28,6 +29,9 @@
(s/optional-key :special_type) (s/maybe su/FieldType)
;; where this column came from in the original query.
:source (s/enum :aggregation :fields :breakout :native)
+ ;; a field clause that can be used to refer to this Field if this query is subsequently used as a source query.
+ ;; Added by this middleware as one of the last steps.
+ (s/optional-key :field_ref) mbql.s/FieldOrAggregationReference
;; various other stuff from the original Field can and should be included such as `:settings`
s/Any s/Any})
@@ -42,19 +46,42 @@
;;; | Adding :cols info for native queries |
;;; +----------------------------------------------------------------------------------------------------------------+
+(defn- check-driver-native-columns
+ "Double-check that the *driver* returned the correct number of `columns` for native query results."
+ [columns rows]
+ (when (seq rows)
+ (let [expected-count (count columns)
+ actual-count (count (first rows))]
+ (when-not (= expected-count actual-count)
+ (throw (ex-info (str (tru "Query processor error: number of columns returned by driver does not match results.")
+ "\n"
+ (tru "Expected {0} columns, but first row of resuls has {1} columns."
+ expected-count actual-count))
+ {:expected-columns columns
+ :first-row (first rows)}))))))
+
(defmethod column-info :native
[_ {:keys [columns rows]}]
+ (check-driver-native-columns columns rows)
;; Infer the types of columns by looking at the first value for each in the results. Native queries don't have the
;; type information from the original `Field` objects used in the query.
(vec
(for [i (range (count columns))
- :let [col (nth columns i)]]
- {:name (name col)
- :display_name (u/keyword->qualified-name col)
- :base_type (or (driver.common/values->base-type (for [row rows]
- (nth row i)))
- :type/*)
- :source :native})))
+ :let [col (nth columns i)
+ base-type (or (driver.common/values->base-type (for [row rows]
+ (nth row i)))
+ :type/*)]]
+ (merge
+ {:name (name col)
+ :display_name (u/keyword->qualified-name col)
+ :base_type base-type
+ :source :native}
+ ;; It is perfectly legal for a driver to return a column with a blank name; for example, SQL Server does this
+ ;; for aggregations like `count(*)` if no alias is used. However, it is *not* legal to use blank names in MBQL
+ ;; `:field-literal` clauses, because `SELECT ""` doesn't make any sense. So if we can't return a valid
+ ;; `:field-literal`, omit the `:field_ref`.
+ (when (seq (name col))
+ {:field_ref [:field-literal (name col) base-type]})))))
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -327,15 +354,21 @@
(s/defn ^:private cols-for-fields
[{:keys [fields], :as inner-query} :- su/Map]
(for [field fields]
- (assoc (col-info-for-field-clause inner-query field) :source :fields)))
+ (assoc (col-info-for-field-clause inner-query field)
+ :source :fields
+ :field_ref field)))
(s/defn ^:private cols-for-ags-and-breakouts
[{aggregations :aggregation, breakouts :breakout, :as inner-query} :- su/Map]
(concat
(for [breakout breakouts]
- (assoc (col-info-for-field-clause inner-query breakout) :source :breakout))
- (for [aggregation aggregations]
- (assoc (col-info-for-aggregation-clause inner-query aggregation) :source :aggregation))))
+ (assoc (col-info-for-field-clause inner-query breakout)
+ :source :breakout
+ :field_ref breakout))
+ (for [[i aggregation] (m/indexed aggregations)]
+ (assoc (col-info-for-aggregation-clause inner-query aggregation)
+ :source :aggregation
+ :field_ref [:aggregation i]))))
(s/defn cols-for-mbql-query
"Return results metadata about the expected columns in an 'inner' MBQL query."
diff --git a/src/metabase/util.clj b/src/metabase/util.clj
index 605687028f93bf676617df479fb299dc3e7886ae..08757e5ebf1ce9d18b4ed981c3adf610842e86af 100644
--- a/src/metabase/util.clj
+++ b/src/metabase/util.clj
@@ -322,30 +322,35 @@
(throw (TimeoutException. (str (tru "Timed out after {0} milliseconds." timeout-ms)))))
result))
+(defn do-with-timeout
+ "Impl for `with-timeout` macro."
+ [timeout-ms f]
+ (let [result (deref-with-timeout
+ (future
+ (try
+ (f)
+ (catch Throwable e
+ e)))
+ timeout-ms)]
+ (if (instance? Throwable result)
+ (throw result)
+ result)))
+
(defmacro with-timeout
"Run `body` in a `future` and throw an exception if it fails to complete after `timeout-ms`."
[timeout-ms & body]
- `(deref-with-timeout (future ~@body) ~timeout-ms))
+ `(do-with-timeout ~timeout-ms (fn [] ~@body)))
(defn round-to-decimals
- "Round (presumabily floating-point) NUMBER to DECIMAL-PLACE. Returns a `Double`.
+ "Round (presumabily floating-point) `number` to `decimal-place`. Returns a `Double`.
(round-to-decimals 2 35.5058998M) -> 35.51"
^Double [^Integer decimal-place, ^Number number]
{:pre [(integer? decimal-place) (number? number)]}
(double (.setScale (bigdec number) decimal-place BigDecimal/ROUND_HALF_UP)))
-(defn ^:deprecated drop-first-arg
- "Returns a new fn that drops its first arg and applies the rest to the original.
- Useful for creating `extend` method maps when you don't care about the `this` param. :flushed:
-
- ((drop-first-arg :value) xyz {:value 100}) -> (apply :value [{:value 100}]) -> 100"
- ^clojure.lang.IFn [^clojure.lang.IFn f]
- (comp (partial apply f) rest list))
-
-
(defn- check-protocol-impl-method-map
- "Check that the methods expected for PROTOCOL are all implemented by METHOD-MAP, and that no extra methods are
+ "Check that the methods expected for `protocol` are all implemented by `method-map`, and that no extra methods are
provided. Used internally by `strict-extend`."
[protocol method-map]
(let [[missing-methods extra-methods] (data/diff (set (keys (:method-map protocol))) (set (keys method-map)))]
diff --git a/test/metabase/api/dataset_test.clj b/test/metabase/api/dataset_test.clj
index 5c4a3068acf92347ea6029eb968af7c028f247f7..eb073cf9b30059fc3c390d073800d630e4fb3f74 100644
--- a/test/metabase/api/dataset_test.clj
+++ b/test/metabase/api/dataset_test.clj
@@ -7,6 +7,9 @@
[dk.ative.docjure.spreadsheet :as spreadsheet]
[expectations :refer [expect]]
[medley.core :as m]
+ [metabase
+ [query-processor-test :as qp.test]
+ [util :as u]]
[metabase.mbql.schema :as mbql.s]
[metabase.models
[card :refer [Card]]
@@ -22,7 +25,6 @@
[datasets :refer [expect-with-driver]]
[users :as test-users]]
[metabase.test.util.log :as tu.log]
- [metabase.util :as u]
[schema.core :as s]
[toucan.db :as db]
[toucan.util.test :as tt])
@@ -56,11 +58,7 @@
(expect
{:api-response
{:data {:rows [[1000]]
- :cols [{:base_type "type/Integer"
- :special_type "type/Number"
- :name "count"
- :display_name "count"
- :source "aggregation"}]
+ :cols [(tu/obj->json->obj (qp.test/aggregate-col :count))]
:native_form true}
:row_count 1
:status "completed"
diff --git a/test/metabase/api/embed_test.clj b/test/metabase/api/embed_test.clj
index 23976b26b5054bfd71fba05a0f52dc457e31ee8f..b6d27b6163723e765e326b811a1868ea1150480c 100644
--- a/test/metabase/api/embed_test.clj
+++ b/test/metabase/api/embed_test.clj
@@ -10,6 +10,7 @@
[expectations :refer [expect]]
[metabase
[http-client :as http]
+ [query-processor-test :as qp.test]
[util :as u]]
[metabase.api
[embed :as embed-api]
@@ -70,11 +71,7 @@
(defn successful-query-results
([]
- {:data {:cols [{:base_type "type/Integer"
- :special_type "type/Number"
- :name "count"
- :display_name "count"
- :source "aggregation"}]
+ {:data {:cols [(tu/obj->json->obj (qp.test/aggregate-col :count))]
:rows [[100]]
:insights nil}
:json_query {:parameters nil}
diff --git a/test/metabase/driver/sql_jdbc/native_test.clj b/test/metabase/driver/sql_jdbc/native_test.clj
index 2bdf686b2d3b5897fd1fb8b3549369e65489c8cd..fc477980b10fc9a3a6cad9544c622bda88fd0803 100644
--- a/test/metabase/driver/sql_jdbc/native_test.clj
+++ b/test/metabase/driver/sql_jdbc/native_test.clj
@@ -16,7 +16,11 @@
:row_count 2
:data {:rows [[100]
[99]]
- :cols [{:name "ID", :display_name "ID", :base_type :type/Integer, :source :native}]
+ :cols [{:name "ID"
+ :display_name "ID"
+ :base_type :type/Integer
+ :source :native
+ :field_ref [:field-literal "ID" :type/Integer]}]
:native_form {:query "SELECT ID FROM VENUES ORDER BY ID DESC LIMIT 2", :params []}}}
(-> (qp/process-query {:native {:query "SELECT ID FROM VENUES ORDER BY ID DESC LIMIT 2"}
:type :native
@@ -30,9 +34,21 @@
:row_count 2
:data {:rows [[100 "Mohawk Bend" 46]
[99 "Golden Road Brewing" 10]]
- :cols [{:name "ID", :display_name "ID", :source :native, :base_type :type/Integer}
- {:name "NAME", :display_name "NAME", :source :native, :base_type :type/Text}
- {:name "CATEGORY_ID", :display_name "CATEGORY_ID", :source :native, :base_type :type/Integer}]
+ :cols [{:name "ID"
+ :display_name "ID"
+ :source :native
+ :base_type :type/Integer
+ :field_ref [:field-literal "ID" :type/Integer]}
+ {:name "NAME"
+ :display_name "NAME"
+ :source :native
+ :base_type :type/Text
+ :field_ref [:field-literal "NAME" :type/Text]}
+ {:name "CATEGORY_ID"
+ :display_name "CATEGORY_ID"
+ :source :native
+ :base_type :type/Integer
+ :field_ref [:field-literal "CATEGORY_ID" :type/Integer]}]
:native_form {:query "SELECT ID, NAME, CATEGORY_ID FROM VENUES ORDER BY ID DESC LIMIT 2", :params []}}}
(-> (qp/process-query {:native {:query "SELECT ID, NAME, CATEGORY_ID FROM VENUES ORDER BY ID DESC LIMIT 2"}
:type :native
diff --git a/test/metabase/driver/sql_jdbc_test.clj b/test/metabase/driver/sql_jdbc_test.clj
index d54f849c5d6f99f391b83a6ec84dbd09be700c05..d1b99462f0ff807e23b95b246d1fdc62d9d9b351 100644
--- a/test/metabase/driver/sql_jdbc_test.clj
+++ b/test/metabase/driver/sql_jdbc_test.clj
@@ -99,24 +99,29 @@
;;; Make sure invalid ssh credentials are detected if a direct connection is possible
-(expect
- #"com.jcraft.jsch.JSchException:"
- (try (let [details {:ssl false
- :password "changeme"
- :tunnel-host "localhost" ; this test works if sshd is running or not
- :tunnel-pass "BOGUS-BOGUS-BOGUS"
- :port 5432
- :dbname "test"
- :host "localhost"
- :tunnel-enabled true
- :tunnel-port 22
- :engine :postgres
- :user "postgres"
- :tunnel-user "example"}]
- (tu.log/suppress-output
- (driver.u/can-connect-with-details? :postgres details :throw-exceptions)))
- (catch Exception e
- (.getMessage e))))
+(datasets/expect-with-driver :postgres
+ com.jcraft.jsch.JSchException
+ (try
+ ;; this test works if sshd is running or not
+ (let [details {:dbname "test"
+ :engine :postgres
+ :host "localhost"
+ :password "changeme"
+ :port 5432
+ :ssl false
+ :tunnel-enabled true
+ :tunnel-host "localhost" ; this test works if sshd is running or not
+ :tunnel-pass "BOGUS-BOGUS-BOGUS"
+ :tunnel-port 22
+ :tunnel-user "example"
+ :user "postgres"}]
+ (tu.log/suppress-output
+ (driver.u/can-connect-with-details? :postgres details :throw-exceptions)))
+ (catch Throwable e
+ (loop [^Throwable e e]
+ (or (when (instance? com.jcraft.jsch.JSchException e)
+ e)
+ (some-> (.getCause e) recur))))))
;;; --------------------------------- Tests for splice-parameters-into-native-query ----------------------------------
diff --git a/test/metabase/models/dashboard_test.clj b/test/metabase/models/dashboard_test.clj
index e96686ef8a450e7cef219060256c4f8ac595f7ce..661e27e6fa45245cc1f9be61ef7b43017123fc1f 100644
--- a/test/metabase/models/dashboard_test.clj
+++ b/test/metabase/models/dashboard_test.clj
@@ -51,20 +51,22 @@
;; diff-dashboards-str
(expect
"renamed it from \"Diff Test\" to \"Diff Test Changed\" and added a description."
- (diff-dashboards-str
- {:name "Diff Test"
- :description nil
- :cards []}
+ (#'dashboard/diff-dashboards-str
+ nil
+ {:name "Diff Test"
+ :description nil
+ :cards []}
{:name "Diff Test Changed"
:description "foobar"
:cards []}))
(expect
"added a card."
- (diff-dashboards-str
- {:name "Diff Test"
- :description nil
- :cards []}
+ (#'dashboard/diff-dashboards-str
+ nil
+ {:name "Diff Test"
+ :description nil
+ :cards []}
{:name "Diff Test"
:description nil
:cards [{:sizeX 2
@@ -77,23 +79,24 @@
(expect
"rearranged the cards, modified the series on card 1 and added some series to card 2."
- (diff-dashboards-str
- {:name "Diff Test"
- :description nil
- :cards [{:sizeX 2
- :sizeY 2
- :row 0
- :col 0
- :id 1
- :card_id 1
- :series [5 6]}
- {:sizeX 2
- :sizeY 2
- :row 0
- :col 0
- :id 2
- :card_id 2
- :series []}]}
+ (#'dashboard/diff-dashboards-str
+ nil
+ {:name "Diff Test"
+ :description nil
+ :cards [{:sizeX 2
+ :sizeY 2
+ :row 0
+ :col 0
+ :id 1
+ :card_id 1
+ :series [5 6]}
+ {:sizeX 2
+ :sizeY 2
+ :row 0
+ :col 0
+ :id 2
+ :card_id 2
+ :series []}]}
{:name "Diff Test"
:description nil
:cards [{:sizeX 2
@@ -157,7 +160,7 @@
;; capture our updated dashboard state
(let [serialized-dashboard2 (serialize-dashboard (Dashboard dashboard-id))]
;; now do the reversion
- (#'dashboard/revert-dashboard! dashboard-id (users/user->id :crowberto) serialized-dashboard)
+ (#'dashboard/revert-dashboard! nil dashboard-id (users/user->id :crowberto) serialized-dashboard)
;; final output is original-state, updated-state, reverted-state
[(update serialized-dashboard :cards check-ids)
serialized-dashboard2
diff --git a/test/metabase/query_processor/middleware/annotate_test.clj b/test/metabase/query_processor/middleware/annotate_test.clj
index 12de36de56af37eb0645235df999449a91210475..82cae575a5508229d52842380651ade6f6494b43 100644
--- a/test/metabase/query_processor/middleware/annotate_test.clj
+++ b/test/metabase/query_processor/middleware/annotate_test.clj
@@ -20,8 +20,16 @@
;; make sure that `column-info` for `:native` queries can still infer types even if the initial value(s) are `nil`
;; (#4256)
(expect
- [{:name "a", :display_name "a", :base_type :type/Integer, :source :native}
- {:name "b", :display_name "b", :base_type :type/Integer, :source :native}]
+ [{:name "a"
+ :display_name "a"
+ :base_type :type/Integer
+ :source :native
+ :field_ref [:field-literal "a" :type/Integer]}
+ {:name "b"
+ :display_name "b"
+ :base_type :type/Integer
+ :source :native
+ :field_ref [:field-literal "b" :type/Integer]}]
(annotate/column-info
{:type :native}
{:columns [:a :b]
@@ -34,7 +42,7 @@
;; make sure that `column-info` for `:native` queries defaults `base_type` to `type/*` if there are no non-nil values
;; when we peek.
(expect
- [{:name "a", :display_name "a", :base_type :type/*, :source :native}]
+ [{:name "a", :display_name "a", :base_type :type/*, :source :native, :field_ref [:field-literal "a" :type/*]}]
(annotate/column-info {:type :native} {:columns [:a], :rows [[nil]]}))
@@ -45,13 +53,15 @@
(defn- info-for-field
([field-id]
(db/select-one (into [Field] (disj (set @#'qp.store/field-columns-to-fetch) :database_type)) :id field-id))
+
([table-key field-key]
(info-for-field (data/id table-key field-key))))
;; make sure columns are comming back the way we'd expect
(expect
[(assoc (info-for-field :venues :price)
- :source :fields)]
+ :source :fields
+ :field_ref [:field-id (data/id :venues :price)])]
(qp.test-util/with-everything-store
(vec
(annotate/column-info
@@ -64,23 +74,27 @@
;; TODO - this can be removed, now that `fk->` forms are "sugar" and replaced with `:joined-field` clauses before the
;; query ever makes it to the 'annotate' stage
(expect
- [(assoc (info-for-field :categories :name)
- :fk_field_id (data/id :venues :category_id), :source :fields)]
+ [(data/$ids venues
+ (assoc (info-for-field :categories :name)
+ :fk_field_id %category_id
+ :source :fields
+ :field_ref $category_id->categories.name))]
(qp.test-util/with-everything-store
(doall
(annotate/column-info
{:type :query
- :query {:fields [[:fk->
- [:field-id (data/id :venues :category_id)]
- [:field-id (data/id :categories :name)]]]}}
+ :query {:fields (data/$ids venues [$category_id->categories.name])}}
{:columns [:name]}))))
;; we should get `:fk_field_id` and information where possible when using `:joined-field` clauses; display_name should
;; include the joined table
(expect
- [(assoc (info-for-field :categories :name)
- :display_name "VENUES → Name"
- :fk_field_id (data/id :venues :category_id), :source :fields)]
+ [(data/$ids venues
+ (assoc (info-for-field :categories :name)
+ :display_name "VENUES → Name"
+ :fk_field_id %category_id
+ :source :fields
+ :field_ref &CATEGORIES__via__CATEGORY_ID.categories.name))]
(qp.test-util/with-everything-store
(data/$ids venues
(doall
@@ -97,9 +111,12 @@
;; when using `:joined-field` clauses for a join a source query (instead of a source table), `display_name` should
;; include the join alias
(expect
- [(assoc (info-for-field :categories :name)
- :display_name "cats → Name"
- :fk_field_id (data/id :venues :category_id), :source :fields)]
+ [(data/$ids venues
+ (assoc (info-for-field :categories :name)
+ :display_name "cats → Name"
+ :fk_field_id %category_id
+ :source :fields
+ :field_ref &cats.categories.name))]
(qp.test-util/with-everything-store
(data/$ids venues
(doall
@@ -115,23 +132,27 @@
;; when a `:datetime-field` form is used, we should add in info about the `:unit`
(expect
- [(assoc (info-for-field :venues :price)
- :unit :month
- :source :fields)]
+ [(data/$ids venues
+ (assoc (info-for-field :venues :price)
+ :unit :month
+ :source :fields
+ :field_ref !month.price))]
(qp.test-util/with-everything-store
(doall
(annotate/column-info
{:type :query
- :query {:fields [[:datetime-field [:field-id (data/id :venues :price)] :month]]}}
+ :query {:fields (data/$ids venues [!month.price])}}
{:columns [:price]}))))
;; datetime unit should work on field literals too
(expect
- [{:name "price"
- :base_type :type/Number
- :display_name "Price"
- :unit :month
- :source :fields}]
+ [(data/$ids venues
+ {:name "price"
+ :base_type :type/Number
+ :display_name "Price"
+ :unit :month
+ :source :fields
+ :field_ref !month.*price/Number})]
(doall
(annotate/column-info
{:type :query
@@ -149,7 +170,15 @@
:bin_width 5
:min_value -100
:max_value 100
- :binning_strategy :num-bins}}]
+ :binning_strategy :num-bins}
+ :field_ref [:binning-strategy
+ [:datetime-field [:field-literal "price" :type/Number] :month]
+ :num-bins
+ 10
+ {:num-bins 10
+ :bin-width 5
+ :min-value -100
+ :max-value 100}]}]
(doall
(annotate/column-info
{:type :query
@@ -246,7 +275,8 @@
{:cols [{:name "totalEvents"
:display_name "Total Events"
:base_type :type/Text
- :source :aggregation}]}
+ :source :aggregation
+ :field_ref [:aggregation 0]}]}
(binding [driver/*driver* :h2]
((annotate/add-column-info (constantly {:cols [{:name "totalEvents"
:display_name "Total Events"
@@ -262,21 +292,25 @@
:special_type :type/Number
:name "count"
:display_name "count"
- :source :aggregation}
+ :source :aggregation
+ :field_ref [:aggregation 0]}
{:source :aggregation
:name "sum"
:display_name "sum"
- :base_type :type/Number}
+ :base_type :type/Number
+ :field_ref [:aggregation 1]}
{:base_type :type/Number
:special_type :type/Number
:name "count_2"
:display_name "count"
- :source :aggregation}
+ :source :aggregation
+ :field_ref [:aggregation 2]}
{:base_type :type/Number
:special_type :type/Number
:name "count_2_2"
:display_name "count_2"
- :source :aggregation}]}
+ :source :aggregation
+ :field_ref [:aggregation 3]}]}
(binding [driver/*driver* :h2]
((annotate/add-column-info (constantly {:cols [{:name "count"
:display_name "count"
@@ -301,7 +335,8 @@
:base_type :type/Float
:special_type :type/Number
:expression_name "discount_price"
- :source :fields}
+ :source :fields
+ :field_ref [:expression "discount_price"]}
(-> (qp.test-util/with-everything-store
((annotate/add-column-info (constantly {}))
(data/mbql-query venues
diff --git a/test/metabase/query_processor/middleware/parameters/sql_test.clj b/test/metabase/query_processor/middleware/parameters/sql_test.clj
index 26230eb0d0a22f5f98ee9dcccb48f1d92db8522d..e16ee1daa522bbc3778d86fe6efa8c3692f92a56 100644
--- a/test/metabase/query_processor/middleware/parameters/sql_test.clj
+++ b/test/metabase/query_processor/middleware/parameters/sql_test.clj
@@ -5,7 +5,7 @@
[metabase
[driver :as driver]
[query-processor :as qp]
- [query-processor-test :as qpt :refer [first-row format-rows-by]]]
+ [query-processor-test :as qp.test]]
[metabase.mbql.normalize :as normalize]
[metabase.query-processor.middleware.parameters.sql :as sql]
[metabase.test
@@ -22,6 +22,7 @@
(defn- parse-template
([sql]
(parse-template sql {}))
+
([sql param-key->value]
(driver/with-driver :h2
(#'sql/parse-template sql param-key->value))))
@@ -572,7 +573,7 @@
;; as with the MBQL parameters tests Redshift fail for unknown reasons; disable their tests for now
(def ^:private sql-parameters-engines
- (delay (disj (qpt/non-timeseries-drivers-with-feature :native-parameters) :redshift)))
+ (delay (disj (qp.test/non-timeseries-drivers-with-feature :native-parameters) :redshift)))
(defn- process-native {:style/indent 0} [& kvs]
(du/with-effective-timezone (data/db)
@@ -581,8 +582,8 @@
(datasets/expect-with-drivers @sql-parameters-engines
[29]
- (first-row
- (format-rows-by [int]
+ (qp.test/first-row
+ (qp.test/format-rows-by [int]
(process-native
:native {:query (format "SELECT COUNT(*) FROM %s WHERE {{checkin_date}}" (checkins-identifier))
:template-tags {"checkin_date" {:name "checkin_date"
@@ -596,8 +597,8 @@
;; no parameter -- should give us a query with "WHERE 1 = 1"
(datasets/expect-with-drivers @sql-parameters-engines
[1000]
- (first-row
- (format-rows-by [int]
+ (qp.test/first-row
+ (qp.test/format-rows-by [int]
(process-native
:native {:query (format "SELECT COUNT(*) FROM %s WHERE {{checkin_date}}" (checkins-identifier))
:template-tags {"checkin_date" {:name "checkin_date"
@@ -610,8 +611,8 @@
;; handling them gets delegated to the functions in `metabase.query-processor.parameters`, which is fully-tested :D
(datasets/expect-with-drivers @sql-parameters-engines
[0]
- (first-row
- (format-rows-by [int]
+ (qp.test/first-row
+ (qp.test/format-rows-by [int]
(process-native
:native {:query (format "SELECT COUNT(*) FROM %s WHERE {{checkin_date}}" (checkins-identifier))
:template-tags {"checkin_date" {:name "checkin_date"
@@ -624,8 +625,8 @@
;; test that multiple filters applied to the same variable combine into `AND` clauses (#3539)
(datasets/expect-with-drivers @sql-parameters-engines
[4]
- (first-row
- (format-rows-by [int]
+ (qp.test/first-row
+ (qp.test/format-rows-by [int]
(process-native
:native {:query (format "SELECT COUNT(*) FROM %s WHERE {{checkin_date}}" (checkins-identifier))
:template-tags {"checkin_date" {:name "checkin_date"
@@ -648,13 +649,13 @@
(= :snowflake driver/*driver*)
"2018-04-16T17:00:00.000-07:00"
- (qpt/supports-report-timezone? driver/*driver*)
+ (qp.test/supports-report-timezone? driver/*driver*)
"2018-04-18T00:00:00.000-07:00"
:else
"2018-04-18T00:00:00.000Z")]
(tu/with-temporary-setting-values [report-timezone "America/Los_Angeles"]
- (first-row
+ (qp.test/first-row
(process-native
:native {:query (case driver/*driver*
:bigquery
@@ -731,7 +732,7 @@
:display-name "X"
:type :text
:required true
- :default "%Toucan%"}}},
+ :default "%Toucan%"}}}
:parameters [{:type "category", :target [:variable [:template-tag "x"]]}]}))
(expect
diff --git a/test/metabase/query_processor/test_util.clj b/test/metabase/query_processor/test_util.clj
index 6f10e6105318240ea8758ee603d3f7cb9b364ecb..91c0414c9a80d859852483ee81204b537e9c7902 100644
--- a/test/metabase/query_processor/test_util.clj
+++ b/test/metabase/query_processor/test_util.clj
@@ -5,12 +5,22 @@
The various QP Store functions & macros in this namespace are primarily meant to help write QP Middleware tests, so
you can test a given piece of middleware without having to worry about putting things in the QP Store
yourself (since this is usually done by other middleware in the first place)."
- (:require [metabase.mbql.util :as mbql.u]
- [metabase.query-processor :as qp]
+ (:require [metabase
+ [query-processor :as qp]
+ [util :as u]]
+ [metabase.mbql.util :as mbql.u]
+ [metabase.models
+ [field :refer [Field]]
+ [table :refer [Table]]]
+ [metabase.query-processor.middleware.add-implicit-joins :as add-implicit-joins]
[metabase.query-processor.store :as qp.store]
[metabase.test.data :as data]
[metabase.util.schema :as su]
- [schema.core :as s]))
+ [schema.core :as s]
+ [toucan.db :as db]))
+
+;; TODO - I don't think we different QP test util namespaces? We should roll this namespace into
+;; `metabase.query-processor-test`
(s/defn ^:private everything-store-table [table-id :- (s/maybe su/IntGreaterThanZero)]
(or (get-in @@#'qp.store/*store* [:tables table-id])
@@ -89,3 +99,13 @@
(throw (ex-info "Query failure" results)))]
{:dataset_query query
:result_metadata metadata}))
+
+(defn fk-table-alias-name
+ "Get the name that will be used for the alias for an implicit join (i.e., a join added as a result of using an `:fk->`
+ clause somewhere in the query.)
+
+ (fk-table-alias-name (data/id :categories) (data/id :venues :category_id)) ;; -> \"CATEGORIES__via__CATEGORY_ID\""
+ [table-or-id field-or-id]
+ (#'add-implicit-joins/join-alias
+ (db/select-one-field :name Table :id (u/get-id table-or-id))
+ (db/select-one-field :name Field :id (u/get-id field-or-id))))
diff --git a/test/metabase/query_processor_test.clj b/test/metabase/query_processor_test.clj
index adede61ef21a87a47647b167ca3d34a5d53361a4..67f15f87a8705af845477a80ebef21e1815b13a9 100644
--- a/test/metabase/query_processor_test.clj
+++ b/test/metabase/query_processor_test.clj
@@ -90,8 +90,10 @@
(col-defaults)
(db/select-one [Field :id :table_id :special_type :base_type :name :display_name :fingerprint]
:id (data/id table-kw field-kw))
+ {:field_ref [:field-id (data/id table-kw field-kw)]}
(when (= field-kw :last_login)
- {:unit :default})))
+ {:unit :default
+ :field_ref [:datetime-field [:field-id (data/id table-kw field-kw)] :default]})))
(defn- expected-column-names
"Get a sequence of keyword names of Fields belonging to a Table in the order they'd normally appear in QP results."
diff --git a/test/metabase/query_processor_test/aggregation_test.clj b/test/metabase/query_processor_test/aggregation_test.clj
index a457245103c09ca4b22539c620a2f5ee8eecc70d..7a5eeb7c858dff47546b29da8184b90f32a506af 100644
--- a/test/metabase/query_processor_test/aggregation_test.clj
+++ b/test/metabase/query_processor_test/aggregation_test.clj
@@ -142,7 +142,7 @@
;; so we can use it with Mongo
(datasets/expect-with-drivers (disj qp.test/non-timeseries-drivers :mongo)
[(qp.test/aggregate-col :count)
- (assoc (qp.test/aggregate-col :count) :name "count_2")]
+ (assoc (qp.test/aggregate-col :count) :name "count_2", :field_ref [:aggregation 1])]
(qp.test/cols
(data/run-mbql-query venues
{:aggregation [[:count] [:count]]})))
diff --git a/test/metabase/query_processor_test/breakout_test.clj b/test/metabase/query_processor_test/breakout_test.clj
index 3f614a834f0511fac13973f5c53f367bf91fe1e4..4ed5b0a9128681338d4e346b0597255c1e2ff1ed 100644
--- a/test/metabase/query_processor_test/breakout_test.clj
+++ b/test/metabase/query_processor_test/breakout_test.clj
@@ -207,7 +207,9 @@
;;Validate binning info is returned with the binning-strategy
(datasets/expect-with-drivers (qp.test/non-timeseries-drivers-with-feature :binning)
(assoc (qp.test/breakout-col :venues :latitude)
- :binning_info {:min_value 10.0, :max_value 50.0, :num_bins 4, :bin_width 10.0, :binning_strategy :bin-width})
+ :binning_info {:min_value 10.0, :max_value 50.0, :num_bins 4, :bin_width 10.0, :binning_strategy :bin-width}
+ :field_ref [:binning-strategy (data/$ids venues $latitude) :bin-width nil
+ {:min-value 10.0, :max-value 50.0, :num-bins 4, :bin-width 10.0}])
(-> (data/run-mbql-query venues
{:aggregation [[:count]]
:breakout [[:binning-strategy $latitude :default]]})
@@ -216,7 +218,9 @@
(datasets/expect-with-drivers (qp.test/non-timeseries-drivers-with-feature :binning)
(assoc (qp.test/breakout-col :venues :latitude)
- :binning_info {:min_value 7.5, :max_value 45.0, :num_bins 5, :bin_width 7.5, :binning_strategy :num-bins})
+ :binning_info {:min_value 7.5, :max_value 45.0, :num_bins 5, :bin_width 7.5, :binning_strategy :num-bins}
+ :field_ref [:binning-strategy (data/$ids venues $latitude) :num-bins 5
+ {:min-value 7.5, :max-value 45.0, :num-bins 5, :bin-width 7.5}])
(-> (data/run-mbql-query venues
{:aggregation [[:count]]
:breakout [[:binning-strategy $latitude :num-bins 5]]})
diff --git a/test/metabase/query_processor_test/remapping_test.clj b/test/metabase/query_processor_test/remapping_test.clj
index 1664465a075913c9218fb79cdf9b7b271fa8ac02..d4878f69be9fa8c7347217da203845145b0069d6 100644
--- a/test/metabase/query_processor_test/remapping_test.clj
+++ b/test/metabase/query_processor_test/remapping_test.clj
@@ -7,6 +7,7 @@
[dimension :refer [Dimension]]
[field :refer [Field]]]
[metabase.query-processor.middleware.add-dimension-projections :as add-dimension-projections]
+ [metabase.query-processor.test-util :as qp.test-util]
[metabase.test.data :as data]
[metabase.test.data.datasets :as datasets]
[toucan.db :as db]))
@@ -52,11 +53,15 @@
["800 Degrees Neapolitan Pizzeria" 2 "Pizza"]]
:cols [(qp.test/col :venues :name)
(qp.test/col :venues :price)
- (assoc (qp.test/col :categories :name)
- :fk_field_id (data/id :venues :category_id)
- :display_name "Foo"
- :name (data/format-name "name_2")
- :remapped_from (data/format-name "category_id"))]}
+ (data/$ids venues
+ (assoc (qp.test/col :categories :name)
+ :fk_field_id %category_id
+ :display_name "Foo"
+ :name (data/format-name "name_2")
+ :remapped_from (data/format-name "category_id")
+ :field_ref [:joined-field
+ (qp.test-util/fk-table-alias-name $$categories %category_id)
+ $categories.name]))]}
(data/with-temp-objects
(data/create-venue-category-fk-remapping "Foo")
(select-columns (set (map data/format-name ["name" "price" "name_2"]))
@@ -73,11 +78,15 @@
["800 Degrees Neapolitan Pizzeria" 2 "Pizza"]]
:cols [(qp.test/col :venues :name)
(qp.test/col :venues :price)
- (assoc (qp.test/col :categories :name)
- :fk_field_id (data/id :venues :category_id)
- :display_name "Foo"
- :name (data/format-name "name_2")
- :remapped_from (data/format-name "category_id"))]}
+ (data/$ids venues
+ (assoc (qp.test/col :categories :name)
+ :fk_field_id %category_id
+ :display_name "Foo"
+ :name (data/format-name "name_2")
+ :remapped_from (data/format-name "category_id")
+ :field_ref [:joined-field
+ (qp.test-util/fk-table-alias-name $$categories %category_id)
+ $categories.name]))]}
(data/with-temp-objects
(data/create-venue-category-fk-remapping "Foo")
(select-columns (set (map data/format-name ["name" "price" "name_2"]))
diff --git a/test/metabase/test/data/impl.clj b/test/metabase/test/data/impl.clj
index a8582bee4a6997d91409741fb75d1407f4d254a0..cfec5be1c5c33f41a90562802a32aa7e26745114 100644
--- a/test/metabase/test/data/impl.clj
+++ b/test/metabase/test/data/impl.clj
@@ -96,7 +96,8 @@
;; make sure we're returing an up-to-date copy of the DB
(Database (u/get-id db)))
(catch Throwable e
- (log/error e (format "Failed to create %s '%s' test database" driver database-name))
+ (printf "Failed to create %s '%s' test database:\n" driver database-name)
+ (println e)
(when config/is-test?
(System/exit -1)))))
diff --git a/test/metabase/test/data/interface.clj b/test/metabase/test/data/interface.clj
index 95ed770e47d283f4d2e43d56e229a9f18dab195f..07622ab36b8763035595c2a417b17416b4ffbd03 100644
--- a/test/metabase/test/data/interface.clj
+++ b/test/metabase/test/data/interface.clj
@@ -352,7 +352,8 @@
:special_type :type/Number
:name "count"
:display_name "count"
- :source :aggregation})
+ :source :aggregation
+ :field_ref [:aggregation 0]})
([driver aggregation-type {field-id :id, :keys [base_type special_type table_id]}]
{:pre [base_type special_type]}
@@ -362,7 +363,8 @@
(qp.store/fetch-and-store-fields! [field-id])
(merge
(annotate/col-info-for-aggregation-clause {} [aggregation-type [:field-id field-id]])
- {:source :aggregation}
+ {:source :aggregation
+ :field_ref [:aggregation 0]}
(when (#{:count :cum-count} aggregation-type)
{:base_type :type/Integer, :special_type :type/Number}))))))