From 1afb4077e82e5dc23e2475c16842d729a33da538 Mon Sep 17 00:00:00 2001
From: Cam Saul <1455846+camsaul@users.noreply.github.com>
Date: Tue, 14 Jan 2020 14:33:19 -0800
Subject: [PATCH] Return Postgres MONEY columns correctly [ci postgres]
(#11716)
Works around upstream bug in Postgres JDBC driver pgjdbc/pgjdbc#425
Also includes a lot of test reformatting/cleanup
---
dev/src/dev.clj | 1 -
src/metabase/driver/postgres.clj | 18 +-
src/metabase/driver/sql_jdbc.clj | 28 +-
src/metabase/util.clj | 57 +-
test/metabase/driver/postgres_test.clj | 766 ++++++++++++-------------
test/metabase/test.clj | 6 +-
test/metabase/util_test.clj | 505 ++++++++--------
7 files changed, 660 insertions(+), 721 deletions(-)
diff --git a/dev/src/dev.clj b/dev/src/dev.clj
index d2d79a049aa..4d91a986f67 100644
--- a/dev/src/dev.clj
+++ b/dev/src/dev.clj
@@ -96,7 +96,6 @@
{:read-columns (partial sql-jdbc.execute/read-columns driver)
:set-parameters (partial sql-jdbc.execute/set-parameters driver)})))
-
([driver-or-driver+dataset sql-args options]
(let [[driver dataset] (u/one-or-many driver-or-driver+dataset)]
(driver/with-driver driver
diff --git a/src/metabase/driver/postgres.clj b/src/metabase/driver/postgres.clj
index f1577df57e9..023d84f59df 100644
--- a/src/metabase/driver/postgres.clj
+++ b/src/metabase/driver/postgres.clj
@@ -8,8 +8,10 @@
[clojure.tools.logging :as log]
[honeysql.core :as hsql]
[java-time :as t]
+ [metabase
+ [driver :as driver]
+ [util :as u]]
[metabase.db.spec :as db.spec]
- [metabase.driver :as driver]
[metabase.driver.common :as driver.common]
[metabase.driver.sql-jdbc
[common :as sql-jdbc.common]
@@ -112,10 +114,10 @@
;;; | metabase.driver.sql impls |
;;; +----------------------------------------------------------------------------------------------------------------+
-(defmethod sql.qp/unix-timestamp->timestamp [:postgres :seconds] [_ _ expr]
+(defmethod sql.qp/unix-timestamp->timestamp [:postgres :seconds]
+ [_ _ expr]
(hsql/call :to_timestamp expr))
-
(defn- date-trunc [unit expr] (hsql/call :date_trunc (hx/literal unit) (hx/->timestamp expr)))
(defn- extract [unit expr] (hsql/call :extract unit (hx/->timestamp expr)))
@@ -145,7 +147,6 @@
(defmethod sql.qp/date [:postgres :quarter-of-year] [_ _ expr] (extract-integer :quarter expr))
(defmethod sql.qp/date [:postgres :year] [_ _ expr] (date-trunc :year expr))
-
(defmethod sql.qp/->honeysql [:postgres :value]
[driver value]
(let [[_ value {base-type :base_type, database-type :database_type}] value]
@@ -302,6 +303,15 @@
(log/tracef "Error in Postgres JDBC driver reading TIME value, fetching as string '%s'" s)
(u.date/parse s))))))
+;; The postgres JDBC driver cannot properly read MONEY columns — see https://github.com/pgjdbc/pgjdbc/issues/425. Work
+;; around this by checking whether the column type name is `money`, and reading it out as a String and parsing to a
+;; BigDecimal if so; otherwise, proceeding as normal
+(defmethod sql-jdbc.execute/read-column [:postgres Types/DOUBLE]
+ [driver _ ^ResultSet rs ^ResultSetMetaData rsmeta ^Integer i]
+ (if (= (.getColumnTypeName rsmeta i) "money")
+ (some-> (.getString rs i) u/parse-currency)
+ (.getObject rs i)))
+
;; Postgres doesn't support OffsetTime
(defmethod sql-jdbc.execute/set-parameter [:postgres OffsetTime]
[driver prepared-statement i t]
diff --git a/src/metabase/driver/sql_jdbc.clj b/src/metabase/driver/sql_jdbc.clj
index 48e2b58d667..fbe2f75032d 100644
--- a/src/metabase/driver/sql_jdbc.clj
+++ b/src/metabase/driver/sql_jdbc.clj
@@ -22,6 +22,7 @@
([driver database honeysql-form]
(jdbc/query (sql-jdbc.conn/db->pooled-connection-spec database)
(sql.qp/format-honeysql driver honeysql-form)))
+
([driver database table honeysql-form]
(query driver database (merge {:from [(sql.qp/->honeysql driver (hx/identifier :table (:schema table) (:name table)))]}
honeysql-form))))
@@ -31,34 +32,43 @@
;;; | Default SQL JDBC metabase.driver impls |
;;; +----------------------------------------------------------------------------------------------------------------+
-(defmethod driver/can-connect? :sql-jdbc [driver details]
+(defmethod driver/can-connect? :sql-jdbc
+ [driver details]
(sql-jdbc.conn/can-connect? driver details))
-(defmethod driver/table-rows-seq :sql-jdbc [driver database table]
+(defmethod driver/table-rows-seq :sql-jdbc
+ [driver database table]
(query driver database table {:select [:*]}))
-(defmethod driver/supports? [:sql-jdbc :set-timezone] [driver _]
+(defmethod driver/supports? [:sql-jdbc :set-timezone]
+ [driver _]
(boolean (seq (sql-jdbc.execute/set-timezone-sql driver))))
-(defmethod driver/execute-query :sql-jdbc [driver query]
+(defmethod driver/execute-query :sql-jdbc
+ [driver query]
(sql-jdbc.execute/execute-query driver query))
-(defmethod driver/notify-database-updated :sql-jdbc [driver database]
+(defmethod driver/notify-database-updated :sql-jdbc
+ [driver database]
(sql-jdbc.conn/notify-database-updated driver database))
-(defmethod driver/describe-database :sql-jdbc [driver database]
+(defmethod driver/describe-database :sql-jdbc
+ [driver database]
(sql-jdbc.sync/describe-database driver database))
-(defmethod driver/describe-table :sql-jdbc [driver database table]
+(defmethod driver/describe-table :sql-jdbc
+ [driver database table]
(sql-jdbc.sync/describe-table driver database table))
-(defmethod driver/describe-table-fks :sql-jdbc [driver database table]
+(defmethod driver/describe-table-fks :sql-jdbc
+ [driver database table]
(sql-jdbc.sync/describe-table-fks driver database table))
;; `:sql-jdbc` drivers almost certainly don't need to override this method, and instead can implement
;; `unprepare/unprepare-value` for specific classes, or, in extereme cases, `unprepare/unprepare` itself.
-(defmethod driver/splice-parameters-into-native-query :sql-jdbc [driver {:keys [params], sql :query, :as query}]
+(defmethod driver/splice-parameters-into-native-query :sql-jdbc
+ [driver {:keys [params], sql :query, :as query}]
(cond-> query
(seq params)
(merge {:params nil
diff --git a/src/metabase/util.clj b/src/metabase/util.clj
index b2779edc09c..baf6d1afed7 100644
--- a/src/metabase/util.clj
+++ b/src/metabase/util.clj
@@ -635,15 +635,6 @@
(when (pred x) i))
coll)))
-
-(defn is-java-9-or-higher?
- "Are we running on Java 9 or above?"
- ([]
- (is-java-9-or-higher? (System/getProperty "java.version")))
- ([java-version-str]
- (when-let [[_ java-major-version-str] (re-matches #"^(?:1\.)?(\d+).*$" java-version-str)]
- (>= (Integer/parseInt java-major-version-str) 9))))
-
(defn hexadecimal-string?
"Returns truthy if `new-value` is a hexadecimal-string"
[new-value]
@@ -716,29 +707,6 @@
[& body]
`(do-with-us-locale (fn [] ~@body)))
-(defn xor
- "Exclusive or. (Because this is implemented as a function, rather than a macro, it is not short-circuting the way `or`
- is.)"
- [x y & more]
- (loop [[x y & more] (into [x y] more)]
- (cond
- (and x y)
- false
-
- (seq more)
- (recur (cons (or x y) more))
-
- :else
- (or x y))))
-
-(defn xor-pred
- "Takes a set of predicates and returns a function that is true if *exactly one* of its composing predicates returns a
- logically true value. Compare to `every-pred`."
- [& preds]
- (fn [& args]
- (apply xor (for [pred preds]
- (apply pred args)))))
-
(defn topological-sort
"Topologically sorts vertexs in graph g. Graph is a map of vertexs to edges. Optionally takes an
additional argument `edge-fn`, a function used to extract edges. Returns data in the same shape
@@ -856,3 +824,28 @@
"Convert `minutes` to milliseconds. More readable than doing this math inline."
[minutes]
(-> minutes minutes->seconds seconds->ms))
+
+(defn parse-currency
+ "Parse a currency String to a BigDecimal. Handles a variety of different formats, such as:
+
+ $1,000.00
+ -£127.54
+ -127,54 €
+ kr-127,54
+ € 127,54-
+ ¥200"
+ ^java.math.BigDecimal [^String s]
+ (when-not (str/blank? s)
+ (bigdec
+ (reduce
+ (partial apply str/replace)
+ s
+ [
+ ;; strip out any current symbols
+ [#"[^\d,.-]+" ""]
+ ;; now strip out any thousands separators
+ [#"(?<=\d)[,.](\d{3})" "$1"]
+ ;; now replace a comma decimal seperator with a period
+ [#"," "."]
+ ;; move minus sign at end to front
+ [#"(^[^-]+)-$" "-$1"]]))))
diff --git a/test/metabase/driver/postgres_test.clj b/test/metabase/driver/postgres_test.clj
index 76045a0e941..b6ca1de0077 100644
--- a/test/metabase/driver/postgres_test.clj
+++ b/test/metabase/driver/postgres_test.clj
@@ -2,13 +2,12 @@
"Tests for features/capabilities specific to PostgreSQL driver, such as support for Postgres UUID or enum types."
(:require [clojure.java.jdbc :as jdbc]
[clojure.test :refer :all]
- [expectations :refer [expect]]
[honeysql.core :as hsql]
[metabase
[driver :as driver]
[query-processor :as qp]
- [query-processor-test :refer [rows rows+column-names]]
[sync :as sync]
+ [test :as mt]
[util :as u]]
[metabase.driver.postgres :as postgres]
[metabase.driver.sql-jdbc
@@ -20,130 +19,109 @@
[field :refer [Field]]
[table :refer [Table]]]
[metabase.sync.sync-metadata :as sync-metadata]
- [metabase.test
- [data :as data]
- [util :as tu]]
- [metabase.test.data
- [datasets :as datasets]
- [interface :as tx]]
- [metabase.test.util.log :as tu.log]
- [toucan.db :as db]
- [toucan.util.test :as tt]))
-
-;; Check that SSL params get added the connection details in the way we'd like # no SSL -- this should *not* include
-;; the key :ssl (regardless of its value) since that will cause the PG driver to use SSL anyway
-(expect
- {:classname "org.postgresql.Driver"
- :subprotocol "postgresql"
- :subname "//localhost:5432/bird_sightings"
- :OpenSourceSubProtocolOverride true
- :user "camsaul"
- :sslmode "disable"}
- (sql-jdbc.conn/connection-details->spec :postgres
- {:ssl false
- :host "localhost"
- :port 5432
- :dbname "bird_sightings"
- :user "camsaul"}))
-
-;; ## ssl - check that expected params get added
-(expect
- {:classname "org.postgresql.Driver"
- :subprotocol "postgresql"
- :subname "//localhost:5432/bird_sightings"
- :OpenSourceSubProtocolOverride true
- :user "camsaul"
- :ssl true
- :sslmode "require"
- :sslfactory "org.postgresql.ssl.NonValidatingFactory"}
- (sql-jdbc.conn/connection-details->spec :postgres
- {:ssl true
- :host "localhost"
- :port 5432
- :dbname "bird_sightings"
- :user "camsaul"}))
-
-;; Verify that we identify JSON columns and mark metadata properly during sync
-(datasets/expect-with-driver :postgres
- :type/SerializedJSON
- (data/dataset (tx/dataset-definition "Postgres with a JSON Field"
- ["venues"
- [{:field-name "address", :base-type {:native "json"}}]
- [[(hsql/raw "to_json('{\"street\": \"431 Natoma\", \"city\": \"San Francisco\", \"state\": \"CA\", \"zip\": 94103}'::text)")]]])
- (db/select-one-field :special_type Field, :id (data/id :venues :address))))
-
-
-;;; # UUID Support
-(tx/defdataset ^:private with-uuid
- [["users"
- [{:field-name "user_id", :base-type :type/UUID}]
- [[#uuid "4f01dcfd-13f7-430c-8e6f-e505c0851027"]
- [#uuid "4652b2e7-d940-4d55-a971-7e484566663e"]
- [#uuid "da1d6ecc-e775-4008-b366-c38e7a2e8433"]
- [#uuid "7a5ce4a2-0958-46e7-9685-1a4eaa3bd08a"]
- [#uuid "84ed434e-80b4-41cf-9c88-e334427104ae"]]]])
+ [toucan.db :as db]))
-;; Check that we can load a Postgres Database with a :type/UUID
-(datasets/expect-with-driver :postgres
- [{:name "id", :base_type :type/Integer}
- {:name "user_id", :base_type :type/UUID}]
- (->> (data/dataset with-uuid
- (data/run-mbql-query users))
- :data
- :cols
- (mapv (u/rpartial select-keys [:name :base_type]))))
-
-
-;; Check that we can filter by a UUID Field
-(datasets/expect-with-driver :postgres
- [[2 #uuid "4652b2e7-d940-4d55-a971-7e484566663e"]]
- (rows (data/dataset with-uuid
- (data/run-mbql-query users
- {:filter [:= $user_id "4652b2e7-d940-4d55-a971-7e484566663e"]}))))
-
-;; check that a nil value for a UUID field doesn't barf (#2152)
-(datasets/expect-with-driver :postgres
- []
- (rows (data/dataset with-uuid
- (data/run-mbql-query users
- {:filter [:= $user_id nil]}))))
-
-;; Check that we can filter by a UUID for SQL Field filters (#7955)
-(datasets/expect-with-driver :postgres
- [[#uuid "4f01dcfd-13f7-430c-8e6f-e505c0851027" 1]]
- (data/dataset with-uuid
- (rows (qp/process-query {:database (data/id)
- :type :native
- :native {:query "SELECT * FROM users WHERE {{user}}"
- :template-tags {:user {:name "user"
- :display_name "User ID"
- :type "dimension"
- :dimension ["field-id" (data/id :users :user_id)]}}}
- :parameters [{:type "text"
- :target ["dimension" ["template-tag" "user"]]
- :value "4f01dcfd-13f7-430c-8e6f-e505c0851027"}]}))))
-
-
-;; Make sure that Tables / Fields with dots in their names get escaped properly
-(tx/defdataset ^:private dots-in-names
+(defn- drop-if-exists-and-create-db!
+ "Drop a Postgres database named `db-name` if it already exists; then create a new empty one with that name."
+ [db-name]
+ (let [spec (sql-jdbc.conn/connection-details->spec :postgres (mt/dbdef->connection-details :postgres :server nil))]
+ ;; kill any open connections
+ (jdbc/query spec ["SELECT pg_terminate_backend(pg_stat_activity.pid)
+ FROM pg_stat_activity
+ WHERE pg_stat_activity.datname = ?;" db-name])
+ ;; create the DB
+ (jdbc/execute! spec [(format "DROP DATABASE IF EXISTS \"%s\";
+ CREATE DATABASE \"%s\";"
+ db-name db-name)]
+ {:transaction? false})))
+
+
+;;; ----------------------------------------------- Connection Details -----------------------------------------------
+
+(deftest connection-details->spec-test
+ (testing (str "Check that SSL params get added the connection details in the way we'd like # no SSL -- this should "
+ "*not* include the key :ssl (regardless of its value) since that will cause the PG driver to use SSL "
+ "anyway")
+ (is (= {:classname "org.postgresql.Driver"
+ :subprotocol "postgresql"
+ :subname "//localhost:5432/bird_sightings"
+ :OpenSourceSubProtocolOverride true
+ :user "camsaul"
+ :sslmode "disable"}
+ (sql-jdbc.conn/connection-details->spec :postgres
+ {:ssl false
+ :host "localhost"
+ :port 5432
+ :dbname "bird_sightings"
+ :user "camsaul"}))))
+ (testing "ssl - check that expected params get added"
+ (is (= {:classname "org.postgresql.Driver"
+ :subprotocol "postgresql"
+ :subname "//localhost:5432/bird_sightings"
+ :OpenSourceSubProtocolOverride true
+ :user "camsaul"
+ :ssl true
+ :sslmode "require"
+ :sslfactory "org.postgresql.ssl.NonValidatingFactory"}
+ (sql-jdbc.conn/connection-details->spec :postgres
+ {:ssl true
+ :host "localhost"
+ :port 5432
+ :dbname "bird_sightings"
+ :user "camsaul"}))))
+ (testing "make sure connection details w/ extra params work as expected"
+ (is (= {:classname "org.postgresql.Driver"
+ :subprotocol "postgresql"
+ :subname "//localhost:5432/cool?prepareThreshold=0"
+ :OpenSourceSubProtocolOverride true
+ :sslmode "disable"}
+ (sql-jdbc.conn/connection-details->spec :postgres
+ {:host "localhost"
+ :port "5432"
+ :dbname "cool"
+ :additional-options "prepareThreshold=0"})))))
+
+
+;;; ------------------------------------------- Tests for sync edge cases --------------------------------------------
+
+(mt/defdataset ^:private dots-in-names
[["objects.stuff"
[{:field-name "dotted.name", :base-type :type/Text}]
[["toucan_cage"]
["four_loko"]
["ouija_board"]]]])
-(datasets/expect-with-driver :postgres
- {:columns ["id" "dotted.name"]
- :rows [[1 "toucan_cage"]
- [2 "four_loko"]
- [3 "ouija_board"]]}
- (-> (data/dataset metabase.driver.postgres-test/dots-in-names
- (data/run-mbql-query objects.stuff))
- rows+column-names))
-
-
-;; Make sure that duplicate column names (e.g. caused by using a FK) still return both columns
-(tx/defdataset ^:private duplicate-names
+(deftest edge-case-identifiers-test
+ (mt/test-driver :postgres
+ (testing "Make sure that Tables / Fields with dots in their names get escaped properly"
+ (mt/dataset dots-in-names
+ (= {:columns ["id" "dotted.name"]
+ :rows [[1 "toucan_cage"]
+ [2 "four_loko"]
+ [3 "ouija_board"]]}
+ (mt/rows+column-names (mt/run-mbql-query objects.stuff)))))
+ (testing "make sure schema/table/field names with hyphens in them work correctly (#8766)"
+ (let [details (mt/dbdef->connection-details :postgres :db {:database-name "hyphen-names-test"})
+ spec (sql-jdbc.conn/connection-details->spec :postgres details)
+ exec! #(doseq [statement %]
+ (jdbc/execute! spec [statement]))]
+ ;; create the postgres DB
+ (drop-if-exists-and-create-db! "hyphen-names-test")
+ ;; create the DB object
+ (mt/with-temp Database [database {:engine :postgres, :details (assoc details :dbname "hyphen-names-test")}]
+ (let [sync! #(sync/sync-database! database)]
+ ;; populate the DB and create a view
+ (exec! ["CREATE SCHEMA \"x-mas\";"
+ "CREATE TABLE \"x-mas\".\"presents-and-gifts\" (\"gift-description\" TEXT NOT NULL);"
+ "INSERT INTO \"x-mas\".\"presents-and-gifts\" (\"gift-description\") VALUES ('Bird Hat');;"])
+ (sync!)
+ (is (= [["Bird Hat"]]
+ (mt/rows (qp/process-query
+ {:database (u/get-id database)
+ :type :query
+ :query {:source-table (db/select-one-id Table :name "presents-and-gifts")}}))))))))))
+
+(mt/defdataset ^:private duplicate-names
[["birds"
[{:field-name "name", :base-type :type/Text}]
[["Rasta"]
@@ -153,73 +131,40 @@
{:field-name "bird_id", :base-type :type/Integer, :fk :birds}]
[["Cam" 1]]]])
-(datasets/expect-with-driver :postgres
- {:columns ["name" "name_2"]
- :rows [["Cam" "Rasta"]]}
- (-> (data/dataset metabase.driver.postgres-test/duplicate-names
- (data/run-mbql-query people
- {:fields [$name $bird_id->birds.name]}))
- rows+column-names))
-
-
-;;; Check support for `inet` columns
-(tx/defdataset ^:private ip-addresses
- [["addresses"
- [{:field-name "ip", :base-type {:native "inet"}}]
- [[(hsql/raw "'192.168.1.1'::inet")]
- [(hsql/raw "'10.4.4.15'::inet")]]]])
-
-;; Filtering by inet columns should add the appropriate SQL cast, e.g. `cast('192.168.1.1' AS inet)` (otherwise this
-;; wouldn't work)
-(datasets/expect-with-driver :postgres
- [[1]]
- (rows (data/dataset metabase.driver.postgres-test/ip-addresses
- (data/run-mbql-query addresses
- {:aggregation [[:count]]
- :filter [:= $ip "192.168.1.1"]}))))
-
-
-;;; Util Fns
-
-(defn drop-if-exists-and-create-db!
- "Drop a Postgres database named `db-name` if it already exists; then create a new empty one with that name."
- [db-name]
- (let [spec (sql-jdbc.conn/connection-details->spec :postgres (tx/dbdef->connection-details :postgres :server nil))]
- ;; kill any open connections
- (jdbc/query spec ["SELECT pg_terminate_backend(pg_stat_activity.pid)
- FROM pg_stat_activity
- WHERE pg_stat_activity.datname = ?;" db-name])
- ;; create the DB
- (jdbc/execute! spec [(format "DROP DATABASE IF EXISTS \"%s\";
- CREATE DATABASE \"%s\";"
- db-name db-name)]
- {:transaction? false})))
+(deftest duplicate-names-test
+ (mt/test-driver :postgres
+ (testing "Make sure that duplicate column names (e.g. caused by using a FK) still return both columns"
+ (mt/dataset duplicate-names
+ (is (= {:columns ["name" "name_2"]
+ :rows [["Cam" "Rasta"]]}
+ (mt/rows+column-names
+ (mt/run-mbql-query people
+ {:fields [$name $bird_id->birds.name]}))))))))
(defn- default-table-result [table-name]
{:name table-name, :schema "public", :description nil})
-;; Check that we properly fetch materialized views.
-;; As discussed in #2355 they don't come back from JDBC `DatabaseMetadata` so we have to fetch them manually.
-(datasets/expect-with-driver :postgres
- {:tables #{(default-table-result "test_mview")}}
- (do
- (drop-if-exists-and-create-db! "materialized_views_test")
- (let [details (tx/dbdef->connection-details :postgres :db {:database-name "materialized_views_test"})]
- (jdbc/execute! (sql-jdbc.conn/connection-details->spec :postgres details)
- ["DROP MATERIALIZED VIEW IF EXISTS test_mview;
+(deftest materialized-views-test
+ (mt/test-driver :postgres
+ (testing (str "Check that we properly fetch materialized views. As discussed in #2355 they don't come back from "
+ "JDBC `DatabaseMetadata` so we have to fetch them manually.")
+ (drop-if-exists-and-create-db! "materialized_views_test")
+ (let [details (mt/dbdef->connection-details :postgres :db {:database-name "materialized_views_test"})]
+ (jdbc/execute! (sql-jdbc.conn/connection-details->spec :postgres details)
+ ["DROP MATERIALIZED VIEW IF EXISTS test_mview;
CREATE MATERIALIZED VIEW test_mview AS
SELECT 'Toucans are the coolest type of bird.' AS true_facts;"])
- (tt/with-temp Database [database {:engine :postgres, :details (assoc details :dbname "materialized_views_test")}]
- (driver/describe-database :postgres database)))))
-
-;; Check that we properly fetch foreign tables.
-(datasets/expect-with-driver :postgres
- {:tables (set (map default-table-result ["foreign_table" "local_table"]))}
- (do
- (drop-if-exists-and-create-db! "fdw_test")
- (let [details (tx/dbdef->connection-details :postgres :db {:database-name "fdw_test"})]
- (jdbc/execute! (sql-jdbc.conn/connection-details->spec :postgres details)
- [(str "CREATE EXTENSION IF NOT EXISTS postgres_fdw;
+ (mt/with-temp Database [database {:engine :postgres, :details (assoc details :dbname "materialized_views_test")}]
+ (is (= {:tables #{(default-table-result "test_mview")}}
+ (driver/describe-database :postgres database))))))))
+
+(deftest foreign-tables-test
+ (mt/test-driver :postgres
+ (testing "Check that we properly fetch foreign tables."
+ (drop-if-exists-and-create-db! "fdw_test")
+ (let [details (mt/dbdef->connection-details :postgres :db {:database-name "fdw_test"})]
+ (jdbc/execute! (sql-jdbc.conn/connection-details->spec :postgres details)
+ [(str "CREATE EXTENSION IF NOT EXISTS postgres_fdw;
CREATE SERVER foreign_server
FOREIGN DATA WRAPPER postgres_fdw
OPTIONS (host '" (:host details) "', port '" (:port details) "', dbname 'fdw_test');
@@ -227,125 +172,123 @@
CREATE FOREIGN TABLE foreign_table (data text)
SERVER foreign_server
OPTIONS (schema_name 'public', table_name 'local_table');")])
- (tt/with-temp Database [database {:engine :postgres, :details (assoc details :dbname "fdw_test")}]
- (driver/describe-database :postgres database)))))
-
-;; make sure that if a view is dropped and recreated that the original Table object is marked active rather than a new
-;; one being created (#3331)
-(datasets/expect-with-driver :postgres
- [{:name "angry_birds", :active true}]
- (let [details (tx/dbdef->connection-details :postgres :db {:database-name "dropped_views_test"})
- spec (sql-jdbc.conn/connection-details->spec :postgres details)
- exec! #(doseq [statement %]
- (jdbc/execute! spec [statement]))]
- ;; create the postgres DB
- (drop-if-exists-and-create-db! "dropped_views_test")
- ;; create the DB object
- (tt/with-temp Database [database {:engine :postgres, :details (assoc details :dbname "dropped_views_test")}]
- (let [sync! #(sync/sync-database! database)]
- ;; populate the DB and create a view
- (exec! ["CREATE table birds (name VARCHAR UNIQUE NOT NULL);"
- "INSERT INTO birds (name) VALUES ('Rasta'), ('Lucky'), ('Kanye Nest');"
- "CREATE VIEW angry_birds AS SELECT upper(name) AS name FROM birds;"])
- ;; now sync the DB
- (sync!)
- ;; drop the view
- (exec! ["DROP VIEW angry_birds;"])
- ;; sync again
- (sync!)
- ;; recreate the view
- (exec! ["CREATE VIEW angry_birds AS SELECT upper(name) AS name FROM birds;"])
- ;; sync one last time
- (sync!)
- ;; now take a look at the Tables in the database related to the view. THERE SHOULD BE ONLY ONE!
- (map (partial into {}) (db/select [Table :name :active] :db_id (u/get-id database), :name "angry_birds"))))))
-
-;;; timezone tests
-
-(defn- server-spec []
- (sql-jdbc.conn/connection-details->spec :postgres (tx/dbdef->connection-details :postgres :server nil)))
-
-(def ^:private current-timezone-query
- {:query "SELECT current_setting('TIMEZONE') AS timezone;"})
-
-(defn- get-timezone-with-report-timezone [report-timezone]
- (-> (#'sql-jdbc.execute/run-query-with-timezone :postgres report-timezone (server-spec) current-timezone-query)
- :rows
- ffirst))
-
-;; check that if we set report-timezone to US/Pacific that the session timezone is in fact US/Pacific
-(datasets/expect-with-driver :postgres
- "US/Pacific"
- (get-timezone-with-report-timezone "US/Pacific"))
-
-;; check that we can set it to something else: America/Chicago
-(datasets/expect-with-driver :postgres
- "America/Chicago"
- (get-timezone-with-report-timezone "America/Chicago"))
-
-;; ok, check that if we try to put in a fake timezone that the query still reëxecutes without a custom timezone. This
-;; should give us the same result as if we didn't try to set a timezone at all
-(datasets/expect-with-driver :postgres
- (-> (#'sql-jdbc.execute/run-query-without-timezone :postgres nil (server-spec) current-timezone-query) :rows ffirst)
- (tu.log/suppress-output
- (get-timezone-with-report-timezone "Crunk Burger")))
-
-
-;; make sure connection details w/ extra params work as expected
-(expect
- {:classname "org.postgresql.Driver"
- :subprotocol "postgresql"
- :subname "//localhost:5432/cool?prepareThreshold=0"
- :OpenSourceSubProtocolOverride true
- :sslmode "disable"}
- (sql-jdbc.conn/connection-details->spec :postgres
- {:host "localhost"
- :port "5432"
- :dbname "cool"
- :additional-options "prepareThreshold=0"}))
-
-(datasets/expect-with-driver :postgres
- "UTC"
- (tu/db-timezone-id))
-
-;; Make sure we're able to fingerprint TIME fields (#5911)
-(deftest fingerprint-time-fields-test
- (datasets/test-driver :postgres
- (drop-if-exists-and-create-db! "time_field_test")
- (let [details (tx/dbdef->connection-details :postgres :db {:database-name "time_field_test"})]
- (jdbc/execute! (sql-jdbc.conn/connection-details->spec :postgres details)
- [(str "CREATE TABLE toucan_sleep_schedule ("
- " start_time TIME WITHOUT TIME ZONE NOT NULL, "
- " end_time TIME WITHOUT TIME ZONE NOT NULL, "
- " reason VARCHAR(256) NOT NULL"
- ");"
- "INSERT INTO toucan_sleep_schedule (start_time, end_time, reason) "
- " VALUES ('22:00'::time, '9:00'::time, 'Beauty Sleep');")])
- (tt/with-temp Database [database {:engine :postgres, :details (assoc details :dbname "time_field_test")}]
- (sync/sync-database! database)
- (is (= {"start_time" {:global {:distinct-count 1
- :nil% 0.0}
- :type {:type/DateTime {:earliest "22:00:00"
- :latest "22:00:00"}}}
- "end_time" {:global {:distinct-count 1
- :nil% 0.0}
- :type {:type/DateTime {:earliest "09:00:00"
- :latest "09:00:00"}}}
- "reason" {:global {:distinct-count 1
- :nil% 0.0}
- :type {:type/Text {:percent-json 0.0
- :percent-url 0.0
- :percent-email 0.0
- :average-length 12.0}}}}
- (db/select-field->field :name :fingerprint Field
- :table_id (db/select-one-id Table :db_id (u/get-id database)))))))))
-
-
-;;; +----------------------------------------------------------------------------------------------------------------+
-;;; | POSTGRES ENUM SUPPORT |
-;;; +----------------------------------------------------------------------------------------------------------------+
-
-(defn- enums-test-db-details [] (tx/dbdef->connection-details :postgres :db {:database-name "enums_test"}))
+ (mt/with-temp Database [database {:engine :postgres, :details (assoc details :dbname "fdw_test")}]
+ (is (= {:tables (set (map default-table-result ["foreign_table" "local_table"]))}
+ (driver/describe-database :postgres database))))))))
+
+(deftest recreated-views-test
+ (mt/test-driver :postgres
+ (testing (str "make sure that if a view is dropped and recreated that the original Table object is marked active "
+ "rather than a new one being created (#3331)")
+ (let [details (mt/dbdef->connection-details :postgres :db {:database-name "dropped_views_test"})
+ spec (sql-jdbc.conn/connection-details->spec :postgres details)
+ exec! #(doseq [statement %]
+ (jdbc/execute! spec [statement]))]
+ ;; create the postgres DB
+ (drop-if-exists-and-create-db! "dropped_views_test")
+ ;; create the DB object
+ (mt/with-temp Database [database {:engine :postgres, :details (assoc details :dbname "dropped_views_test")}]
+ (let [sync! #(sync/sync-database! database)]
+ ;; populate the DB and create a view
+ (exec! ["CREATE table birds (name VARCHAR UNIQUE NOT NULL);"
+ "INSERT INTO birds (name) VALUES ('Rasta'), ('Lucky'), ('Kanye Nest');"
+ "CREATE VIEW angry_birds AS SELECT upper(name) AS name FROM birds;"])
+ ;; now sync the DB
+ (sync!)
+ ;; drop the view
+ (exec! ["DROP VIEW angry_birds;"])
+ ;; sync again
+ (sync!)
+ ;; recreate the view
+ (exec! ["CREATE VIEW angry_birds AS SELECT upper(name) AS name FROM birds;"])
+ ;; sync one last time
+ (sync!)
+ ;; now take a look at the Tables in the database related to the view. THERE SHOULD BE ONLY ONE!
+ (is (= [{:name "angry_birds", :active true}]
+ (map (partial into {})
+ (db/select [Table :name :active] :db_id (u/get-id database), :name "angry_birds"))))))))))
+
+
+;;; ----------------------------------------- Tests for exotic column types ------------------------------------------
+
+(deftest json-columns-test
+ (mt/test-driver :postgres
+ (testing "Verify that we identify JSON columns and mark metadata properly during sync"
+ (mt/dataset (mt/dataset-definition "Postgres with a JSON Field"
+ ["venues"
+ [{:field-name "address", :base-type {:native "json"}}]
+ [[(hsql/raw "to_json('{\"street\": \"431 Natoma\", \"city\": \"San Francisco\", \"state\": \"CA\", \"zip\": 94103}'::text)")]]])
+ (is (= :type/SerializedJSON
+ (db/select-one-field :special_type Field, :id (mt/id :venues :address))))))))
+
+(mt/defdataset ^:private with-uuid
+ [["users"
+ [{:field-name "user_id", :base-type :type/UUID}]
+ [[#uuid "4f01dcfd-13f7-430c-8e6f-e505c0851027"]
+ [#uuid "4652b2e7-d940-4d55-a971-7e484566663e"]
+ [#uuid "da1d6ecc-e775-4008-b366-c38e7a2e8433"]
+ [#uuid "7a5ce4a2-0958-46e7-9685-1a4eaa3bd08a"]
+ [#uuid "84ed434e-80b4-41cf-9c88-e334427104ae"]]]])
+
+(deftest uuid-columns-test
+ (mt/test-driver :postgres
+ (mt/dataset with-uuid
+ (testing "Check that we can load a Postgres Database with a :type/UUID"
+ (is (= [{:name "id", :base_type :type/Integer}
+ {:name "user_id", :base_type :type/UUID}]
+ (map #(select-keys % [:name :base_type])
+ (mt/cols (mt/run-mbql-query users))))))
+ (testing "Check that we can filter by a UUID Field"
+ (is (= [[2 #uuid "4652b2e7-d940-4d55-a971-7e484566663e"]]
+ (mt/rows (mt/run-mbql-query users
+ {:filter [:= $user_id "4652b2e7-d940-4d55-a971-7e484566663e"]})))))
+ (testing "check that a nil value for a UUID field doesn't barf (#2152)"
+ (is (= []
+ (mt/rows (mt/run-mbql-query users
+ {:filter [:= $user_id nil]})))))
+ (testing "Check that we can filter by a UUID for SQL Field filters (#7955)"
+ (is (= [[#uuid "4f01dcfd-13f7-430c-8e6f-e505c0851027" 1]]
+ (mt/rows (qp/process-query {:database (mt/id)
+ :type :native
+ :native {:query "SELECT * FROM users WHERE {{user}}"
+ :template-tags {:user {:name "user"
+ :display_name "User ID"
+ :type "dimension"
+ :dimension ["field-id" (mt/id :users :user_id)]}}}
+ :parameters [{:type "text"
+ :target ["dimension" ["template-tag" "user"]]
+ :value "4f01dcfd-13f7-430c-8e6f-e505c0851027"}]}))))))))
+
+
+(mt/defdataset ^:private ip-addresses
+ [["addresses"
+ [{:field-name "ip", :base-type {:native "inet"}}]
+ [[(hsql/raw "'192.168.1.1'::inet")]
+ [(hsql/raw "'10.4.4.15'::inet")]]]])
+
+(deftest inet-columns-test
+ (mt/test-driver :postgres
+ (testing (str "Filtering by inet columns should add the appropriate SQL cast, e.g. `cast('192.168.1.1' AS inet)` "
+ "(otherwise this wouldn't work)")
+ (mt/dataset ip-addresses
+ (is (= [[1]]
+ (mt/rows (mt/run-mbql-query addresses
+ {:aggregation [[:count]]
+ :filter [:= $ip "192.168.1.1"]}))))))))
+
+(deftest money-columns-test
+ (mt/test-driver :postgres
+ (testing "We should support the Postgres MONEY type"
+ (testing "It should be possible to return money column results (#3754)"
+ (mt/with-log-level :trace
+ (is (= [{:money 1000.00M}]
+ (jdbc/query
+ (sql-jdbc.conn/connection-details->spec :postgres (mt/dbdef->connection-details :postgres :server nil))
+ "SELECT 1000::money AS \"money\";"
+ {:read-columns (partial sql-jdbc.execute/read-columns :postgres)
+ :set-parameters (partial sql-jdbc.execute/set-parameters :postgres)}))))))))
+
+(defn- enums-test-db-details [] (mt/dbdef->connection-details :postgres :db {:database-name "enums_test"}))
(defn- create-enums-db!
"Create a Postgres database called `enums_test` that has a couple of enum types and a couple columns of those types.
@@ -369,106 +312,131 @@
(defn- do-with-enums-db {:style/indent 0} [f]
(create-enums-db!)
- (tt/with-temp Database [database {:engine :postgres, :details (enums-test-db-details)}]
+ (mt/with-temp Database [database {:engine :postgres, :details (enums-test-db-details)}]
(sync-metadata/sync-db-metadata! database)
(f database)))
-;; check that we can actually fetch the enum types from a DB
-(datasets/expect-with-driver :postgres
- #{(keyword "bird type") :bird_status}
- (do-with-enums-db
- (fn [db]
- (#'postgres/enum-types :postgres db))))
-
-;; check that describe-table properly describes the database & base types of the enum fields
-(datasets/expect-with-driver :postgres
- {:name "birds"
- :fields #{{:name "name",
- :database-type "varchar"
- :base-type :type/Text
- :pk? true}
- {:name "status"
- :database-type "bird_status"
- :base-type :type/PostgresEnum}
- {:name "type"
- :database-type "bird type"
- :base-type :type/PostgresEnum}}}
- (do-with-enums-db
- (fn [db]
- (driver/describe-table :postgres db {:name "birds"}))))
-
-;; check that when syncing the DB the enum types get recorded appropriately
-(datasets/expect-with-driver :postgres
- #{{:name "name", :database_type "varchar", :base_type :type/Text}
- {:name "type", :database_type "bird type", :base_type :type/PostgresEnum}
- {:name "status", :database_type "bird_status", :base_type :type/PostgresEnum}}
- (do-with-enums-db
- (fn [db]
- (let [table-id (db/select-one-id Table :db_id (u/get-id db), :name "birds")]
- (set (map (partial into {})
- (db/select [Field :name :database_type :base_type] :table_id table-id)))))))
-
-
-;; check that values for enum types get wrapped in appropriate CAST() fn calls in `->honeysql`
-(datasets/expect-with-driver :postgres
- {:name :cast, :args ["toucan" (keyword "bird type")]}
- (sql.qp/->honeysql :postgres [:value "toucan" {:database_type "bird type", :base_type :type/PostgresEnum}]))
-
-;; End-to-end check: make sure everything works as expected when we run an actual query
-(datasets/expect-with-driver :postgres
- {:rows [["Rasta" "good bird" "toucan"]]
- :native_form {:query (str "SELECT \"public\".\"birds\".\"name\" AS \"name\","
- " \"public\".\"birds\".\"status\" AS \"status\","
- " \"public\".\"birds\".\"type\" AS \"type\" "
- "FROM \"public\".\"birds\" "
- "WHERE \"public\".\"birds\".\"type\" = CAST('toucan' AS \"bird type\") "
- "LIMIT 10")
- :params nil}}
- (do-with-enums-db
- (fn [db]
- (let [table-id (db/select-one-id Table :db_id (u/get-id db), :name "birds")
- bird-type-field-id (db/select-one-id Field :table_id table-id, :name "type")]
- (-> (qp/process-query
- {:database (u/get-id db)
- :type :query
- :query {:source-table table-id
- :filter [:= [:field-id (u/get-id bird-type-field-id)] "toucan"]
- :limit 10}})
- :data
- (select-keys [:rows :native_form]))))))
-
-;; make sure schema/table/field names with hyphens in them work correctly (#8766)
-(datasets/expect-with-driver :postgres
- [["Bird Hat"]]
- (metabase.driver/with-driver :postgres
- [{:name "angry_birds", :active true}]
- (let [details (tx/dbdef->connection-details :postgres :db {:database-name "hyphen-names-test"})
- spec (sql-jdbc.conn/connection-details->spec :postgres details)
- exec! #(doseq [statement %]
- (jdbc/execute! spec [statement]))]
- ;; create the postgres DB
- (drop-if-exists-and-create-db! "hyphen-names-test")
- ;; create the DB object
- (tt/with-temp Database [database {:engine :postgres, :details (assoc details :dbname "hyphen-names-test")}]
- (let [sync! #(sync/sync-database! database)]
- ;; populate the DB and create a view
- (exec! ["CREATE SCHEMA \"x-mas\";"
- "CREATE TABLE \"x-mas\".\"presents-and-gifts\" (\"gift-description\" TEXT NOT NULL);"
- "INSERT INTO \"x-mas\".\"presents-and-gifts\" (\"gift-description\") VALUES ('Bird Hat');;"])
- (sync!)
- (-> (qp/process-query
- {:database (u/get-id database)
- :type :query
- :query {:source-table (db/select-one-id Table :name "presents-and-gifts")}})
- rows))))))
-
-;; If the DB throws an exception, is it properly returned by the query processor? Is it status :failed? (#9942)
-(datasets/expect-with-driver :postgres
- {:status :failed
- :class org.postgresql.util.PSQLException
- :error "ERROR: column \"adsasdasd\" does not exist\n Position: 20"}
- (-> (qp/process-query
- {:database (data/id)
- :type :native
- :native {:query "SELECT adsasdasd;"}})
- (select-keys [:status :class :error])))
+(deftest enums-test
+ (mt/test-driver :postgres
+ (testing "check that values for enum types get wrapped in appropriate CAST() fn calls in `->honeysql`"
+ (is (= (hsql/call :cast "toucan" (keyword "bird type"))
+ (sql.qp/->honeysql :postgres [:value "toucan" {:database_type "bird type", :base_type :type/PostgresEnum}]))))
+ (do-with-enums-db
+ (fn [db]
+ (testing "check that we can actually fetch the enum types from a DB"
+ (is (= #{(keyword "bird type") :bird_status}
+ (#'postgres/enum-types :postgres db))))
+ (testing "check that describe-table properly describes the database & base types of the enum fields"
+ (is (= {:name "birds"
+ :fields #{{:name "name",
+ :database-type "varchar"
+ :base-type :type/Text
+ :pk? true}
+ {:name "status"
+ :database-type "bird_status"
+ :base-type :type/PostgresEnum}
+ {:name "type"
+ :database-type "bird type"
+ :base-type :type/PostgresEnum}}}
+ (driver/describe-table :postgres db {:name "birds"}))))
+ (testing "check that when syncing the DB the enum types get recorded appropriately"
+ (let [table-id (db/select-one-id Table :db_id (u/get-id db), :name "birds")]
+ (is (= #{{:name "name", :database_type "varchar", :base_type :type/Text}
+ {:name "type", :database_type "bird type", :base_type :type/PostgresEnum}
+ {:name "status", :database_type "bird_status", :base_type :type/PostgresEnum}}
+ (set (map (partial into {})
+ (db/select [Field :name :database_type :base_type] :table_id table-id)))))))
+ (testing "End-to-end check: make sure everything works as expected when we run an actual query"
+ (let [table-id (db/select-one-id Table :db_id (u/get-id db), :name "birds")
+ bird-type-field-id (db/select-one-id Field :table_id table-id, :name "type")]
+ (is (= {:rows [["Rasta" "good bird" "toucan"]]
+ :native_form {:query (str "SELECT \"public\".\"birds\".\"name\" AS \"name\","
+ " \"public\".\"birds\".\"status\" AS \"status\","
+ " \"public\".\"birds\".\"type\" AS \"type\" "
+ "FROM \"public\".\"birds\" "
+ "WHERE \"public\".\"birds\".\"type\" = CAST('toucan' AS \"bird type\") "
+ "LIMIT 10")
+ :params nil}}
+ (-> (qp/process-query
+ {:database (u/get-id db)
+ :type :query
+ :query {:source-table table-id
+ :filter [:= [:field-id (u/get-id bird-type-field-id)] "toucan"]
+ :limit 10}})
+ :data
+ (select-keys [:rows :native_form]))))))))))
+
+
+;;; ------------------------------------------------ Timezone-related ------------------------------------------------
+
+(deftest timezone-test
+ (mt/test-driver :postgres
+ (let [current-timezone-query {:query "SELECT current_setting('TIMEZONE') AS timezone;"}
+ server-spec (sql-jdbc.conn/connection-details->spec :postgres
+ (mt/dbdef->connection-details :postgres :server nil))
+ get-timezone-with-report-timezone (fn [report-timezone]
+ (-> (#'sql-jdbc.execute/run-query-with-timezone
+ :postgres report-timezone server-spec current-timezone-query)
+ :rows
+ ffirst))]
+ (testing "check that if we set report-timezone to US/Pacific that the session timezone is in fact US/Pacific"
+ (is (= "US/Pacific"
+ (get-timezone-with-report-timezone "US/Pacific"))))
+ (testing "check that we can set it to something else: America/Chicago"
+ (is (= "America/Chicago"
+ (get-timezone-with-report-timezone "America/Chicago"))))
+ (testing (str "ok, check that if we try to put in a fake timezone that the query still reëxecutes without a "
+ "custom timezone. This should give us the same result as if we didn't try to set a timezone at all")
+ (mt/suppress-output
+ (is (= (-> (#'sql-jdbc.execute/run-query-without-timezone :postgres nil server-spec current-timezone-query)
+ :rows
+ ffirst)
+ (get-timezone-with-report-timezone "Crunk Burger"))))))))
+
+(deftest fingerprint-time-fields-test
+ (mt/test-driver :postgres
+ (testing "Make sure we're able to fingerprint TIME fields (#5911)"
+ (drop-if-exists-and-create-db! "time_field_test")
+ (let [details (mt/dbdef->connection-details :postgres :db {:database-name "time_field_test"})]
+ (jdbc/execute! (sql-jdbc.conn/connection-details->spec :postgres details)
+ [(str "CREATE TABLE toucan_sleep_schedule ("
+ " start_time TIME WITHOUT TIME ZONE NOT NULL, "
+ " end_time TIME WITHOUT TIME ZONE NOT NULL, "
+ " reason VARCHAR(256) NOT NULL"
+ ");"
+ "INSERT INTO toucan_sleep_schedule (start_time, end_time, reason) "
+ " VALUES ('22:00'::time, '9:00'::time, 'Beauty Sleep');")])
+ (mt/with-temp Database [database {:engine :postgres, :details (assoc details :dbname "time_field_test")}]
+ (sync/sync-database! database)
+ (is (= {"start_time" {:global {:distinct-count 1
+ :nil% 0.0}
+ :type {:type/DateTime {:earliest "22:00:00"
+ :latest "22:00:00"}}}
+ "end_time" {:global {:distinct-count 1
+ :nil% 0.0}
+ :type {:type/DateTime {:earliest "09:00:00"
+ :latest "09:00:00"}}}
+ "reason" {:global {:distinct-count 1
+ :nil% 0.0}
+ :type {:type/Text {:percent-json 0.0
+ :percent-url 0.0
+ :percent-email 0.0
+ :average-length 12.0}}}}
+ (db/select-field->field :name :fingerprint Field
+ :table_id (db/select-one-id Table :db_id (u/get-id database))))))))))
+
+
+;;; ----------------------------------------------------- Other ------------------------------------------------------
+
+(deftest exception-test
+ (mt/test-driver :postgres
+ (testing (str "If the DB throws an exception, is it properly returned by the query processor? Is it status "
+ ":failed? (#9942)")
+ (is (= {:status :failed
+ :class org.postgresql.util.PSQLException
+ :error "ERROR: column \"adsasdasd\" does not exist\n Position: 20"}
+ (-> (qp/process-query
+ {:database (mt/id)
+ :type :native
+ :native {:query "SELECT adsasdasd;"}})
+ (select-keys [:status :class :error])))))))
diff --git a/test/metabase/test.clj b/test/metabase/test.clj
index c93b561b9fe..421d51f93d8 100644
--- a/test/metabase/test.clj
+++ b/test/metabase/test.clj
@@ -1,5 +1,8 @@
(ns metabase.test
- "The stuff you need to write almost every test, all in one place. Nice!"
+ "The stuff you need to write almost every test, all in one place. Nice!
+
+ (Prefer using `metabase.test` to requiring bits and pieces from these various namespaces going forward, since it
+ reduces the cognitive load required to write tests.)"
(:require [clojure.test :refer :all]
[java-time :as t]
[metabase
@@ -141,6 +144,7 @@
db-test-env-var
db-test-env-var-or-throw
dbdef->connection-details
+ defdataset
dispatch-on-driver-with-test-extensions
get-dataset-definition
has-questionable-timezone-support?
diff --git a/test/metabase/util_test.clj b/test/metabase/util_test.clj
index dc2cf53ae22..ee0d8224347 100644
--- a/test/metabase/util_test.clj
+++ b/test/metabase/util_test.clj
@@ -1,92 +1,101 @@
(ns metabase.util-test
"Tests for functions in `metabase.util`."
(:require [clojure.test :refer :all]
- [expectations :refer [expect]]
+ [clojure.tools.macro :as tools.macro]
[flatland.ordered.map :refer [ordered-map]]
[metabase.util :as u])
(:import java.util.Locale))
-;;; `host-up?` and `host-port-up?`
-
-(expect
- (u/host-up? "localhost"))
-
-(expect
- false
- (u/host-up? "nosuchhost"))
-
-(expect
- false
- (u/host-port-up? "nosuchhost" 8005))
-
-
-;;; `url?`
-(expect true (u/url? "http://google.com"))
-(expect true (u/url? "https://google.com"))
-(expect true (u/url? "https://amazon.co.uk"))
-(expect true (u/url? "http://google.com?q=my-query&etc"))
-(expect true (u/url? "http://www.cool.com"))
-(expect true (u/url? "http://localhost/"))
-(expect true (u/url? "http://localhost:3000"))
-(expect true (u/url? "https://www.mapbox.com/help/data/stations.geojson"))
-(expect true (u/url? "http://www.cool.com:3000"))
-(expect true (u/url? "http://localhost:3000/auth/reset_password/144_f98987de-53ca-4335-81da-31bb0de8ea2b#new"))
-(expect true (u/url? "http://192.168.1.10/"))
-(expect true (u/url? "http://metabase.intranet/"))
-(expect false (u/url? "google.com")) ; missing protocol
-(expect false (u/url? "ftp://metabase.com")) ; protocol isn't HTTP/HTTPS
-(expect false (u/url? "http://.com")) ; no domain
-(expect false (u/url? "http://google.")) ; no TLD
-(expect false (u/url? "http://")) ; no domain or tld
-(expect false (u/url? "http:/")) ; nil .getAuthority needs to be handled or NullPointerException
-
-
-;;; `qualified-name`
-(expect
- "keyword"
- (u/qualified-name :keyword))
-
-(expect
- "namespace/keyword"
- (u/qualified-name :namespace/keyword))
-
-;; `qualified-name` should return strings as-is
-(expect
- "some string"
- (u/qualified-name "some string"))
-
-;; `qualified-name` should work on anything that implements `clojure.lang.Named`
-(expect
- "namespace/name"
- (u/qualified-name (reify clojure.lang.Named
- (getName [_] "name")
- (getNamespace [_] "namespace"))))
-
-;; `qualified-name` shouldn't throw an NPE (unlike `name`)
-(expect
- nil
- (u/qualified-name nil))
-
-;; we shouldn't ignore non-nil values -- `u/qualified-name` should throw an Exception if `name` would
-(expect
- ClassCastException
- (u/qualified-name false))
-
-;;; `rpartial`
-(expect
- 3
- ((u/rpartial - 5) 8))
-
-(expect -7
- ((u/rpartial - 5 10) 8))
-
-
-;;; `key-by`
-(expect
- {1 {:id 1, :name "Rasta"}
- 2 {:id 2, :name "Lucky"}}
- (u/key-by :id [{:id 1, :name "Rasta"}
- {:id 2, :name "Lucky"}]))
+(defn- are+-message [expr arglist args]
+ (pr-str
+ (second
+ (macroexpand-1
+ (list
+ `tools.macro/symbol-macrolet
+ (vec (apply concat (map-indexed (fn [i arg]
+ [arg (nth args i)])
+ arglist)))
+ expr)))))
+
+(defmacro ^:private are+
+ "Like `clojure.test/are` but includes a message for easier test failure debugging. (Also this is somewhat more
+ efficient since it generates far less code  it uses `doseq` rather than repeating the entire test each time.)
+
+ TODO Â if this macro proves useful, we should consider moving it to somewhere more general, such as
+ `metabase.test`."
+ {:style/indent 2}
+ [argv expr & args]
+ `(doseq [args# ~(mapv vec (partition (count argv) args))
+ :let [~argv args#]]
+ (is ~expr
+ (are+-message '~expr '~argv args#))))
+
+(deftest host-up?-test
+ (testing "host-up?"
+ (are+ [s expected] (= expected
+ (u/host-up? s))
+ "localhost" true
+ "nosuchhost" false))
+ (testing "host-port-up?"
+ (is (= false
+ (u/host-port-up? "nosuchhost" 8005)))))
+
+(deftest url?-test
+ (are+ [s expected] (= expected
+ (u/url? s))
+ "http://google.com" true
+ "https://google.com" true
+ "https://amazon.co.uk" true
+ "http://google.com?q=my-query&etc" true
+ "http://www.cool.com" true
+ "http://localhost/" true
+ "http://localhost:3000" true
+ "https://www.mapbox.com/help/data/stations.geojson" true
+ "http://www.cool.com:3000" true
+ "http://localhost:3000/auth/reset_password/144_f98987de-53ca-4335-81da-31bb0de8ea2b#new" true
+ "http://192.168.1.10/" true
+ "http://metabase.intranet/" true
+ ;; missing protocol
+ "google.com" false
+ ;; protocol isn't HTTP/HTTPS
+ "ftp://metabase.com" false
+ ;; no domain
+ "http://.com" false
+ ;; no TLD
+ "http://google." false
+ ;; no domain or tld
+ "http://" false
+ ;; nil .getAuthority needs to be handled or NullPointerException
+ "http:/" false))
+
+(deftest qualified-name-test
+ (are+ [k expected] (= expected
+ (u/qualified-name k))
+ :keyword "keyword"
+ :namespace/keyword "namespace/keyword"
+ ;; `qualified-name` should return strings as-is
+ "some string" "some string"
+ ;; `qualified-name` should work on anything that implements `clojure.lang.Named`
+ (reify clojure.lang.Named
+ (getName [_] "name")
+ (getNamespace [_] "namespace")) "namespace/name"
+ ;; `qualified-name` shouldn't throw an NPE (unlike `name`)
+ nil nil)
+ (testing "we shouldn't ignore non-nil values -- `u/qualified-name` should throw an Exception if `name` would"
+ (is (thrown? ClassCastException
+ (u/qualified-name false)))))
+
+(deftest rpartial-test
+ (is (= 3
+ ((u/rpartial - 5) 8)))
+ (is (= -7
+ ((u/rpartial - 5 10) 8))))
+
+(deftest key-by-test
+ (is (= {1 {:id 1, :name "Rasta"}
+ 2 {:id 2, :name "Lucky"}}
+ (u/key-by :id [{:id 1, :name "Rasta"}
+ {:id 2, :name "Lucky"}]))))
(deftest remove-diacritical-marks-test
(doseq [[s expected] {"üuuü" "uuuu"
@@ -98,8 +107,6 @@
(is (= expected
(u/remove-diacritical-marks s))))))
-
-;;; `slugify`
(deftest slugify-test
(doseq [[group s->expected]
{nil
@@ -119,200 +126,148 @@
(is (= expected
(u/slugify s))))))))
-;;; `select-nested-keys`
-(expect
- {:a 100, :b {:d 300}}
- (u/select-nested-keys {:a 100, :b {:c 200, :d 300}} [:a [:b :d] :c]))
-
-(expect
- {:b {:c 200, :d 300}}
- (u/select-nested-keys {:a 100, :b {:c 200, :d 300}} [:b]))
-
-(expect
- {:b {:c 200, :d 300}}
- (u/select-nested-keys {:a 100, :b {:c 200, :d 300}} [[:b :c :d]]))
-
-(expect
- {:b {:d {:e 300}}}
- (u/select-nested-keys {:a 100, :b {:c 200, :d {:e 300}}} [[:b [:d :e]]]))
-
-(expect
- {:b {:d {:e 300}}}
- (u/select-nested-keys {:a 100, :b {:c 200, :d {:e 300}}} [[:b :d]]))
-
-(expect
- {:a {:b 100}, :d {:e 300}}
- (u/select-nested-keys {:a {:b 100, :c 200}, :d {:e 300, :f 400}} [[:a :b] [:d :e]]))
-
-(expect
- {:a 100}
- (u/select-nested-keys {:a 100, :b {:c 200, :d 300}} [[:a]]))
-
-(expect
- {}
- (u/select-nested-keys {:a 100, :b {:c 200, :d 300}} [:c]))
-
-(expect
- {}
- (u/select-nested-keys nil [:c]))
-
-(expect
- {}
- (u/select-nested-keys {} nil))
-
-(expect
- {}
- (u/select-nested-keys {:a 100, :b {:c 200, :d 300}} []))
-
-(expect
- {}
- (u/select-nested-keys {} [:c]))
-
-
-;;; `base64-string?`
-(expect (u/base64-string? "ABc"))
-(expect (u/base64-string? "ABc/+asdasd=="))
-(expect false (u/base64-string? 100))
-(expect false (u/base64-string? "<<>>"))
-(expect false (u/base64-string? "{\"a\": 10}"))
-
-
-;;; `occurances-of-substring`
-;; return nil if one or both strings are nil or empty
-(expect nil (u/occurances-of-substring nil nil))
-(expect nil (u/occurances-of-substring nil ""))
-(expect nil (u/occurances-of-substring "" nil))
-(expect nil (u/occurances-of-substring "" ""))
-(expect nil (u/occurances-of-substring "ABC" ""))
-(expect nil (u/occurances-of-substring "" " ABC"))
-
-(expect 1 (u/occurances-of-substring "ABC" "A"))
-(expect 2 (u/occurances-of-substring "ABA" "A"))
-(expect 3 (u/occurances-of-substring "AAA" "A"))
-
-(expect 0 (u/occurances-of-substring "ABC" "{{id}}"))
-(expect 1 (u/occurances-of-substring "WHERE ID = {{id}}" "{{id}}"))
-(expect 2 (u/occurances-of-substring "WHERE ID = {{id}} OR USER_ID = {{id}}" "{{id}}"))
-(expect 3 (u/occurances-of-substring "WHERE ID = {{id}} OR USER_ID = {{id}} OR TOUCAN_ID = {{id}} OR BIRD_ID = {{bird}}" "{{id}}"))
-
-
-;;; tests for `select-non-nil-keys` and `select-keys-when`
-(expect
- {:a 100}
- (u/select-non-nil-keys {:a 100, :b nil} #{:a :b :c}))
-
-(expect
- {:a 100, :b nil, :d 200}
- (u/select-keys-when {:a 100, :b nil, :d 200, :e nil}
- :present #{:a :b :c}
- :non-nil #{:d :e :f}))
-
-
-;;; tests for `order-of-magnitude`
-(expect -2 (u/order-of-magnitude 0.01))
-(expect -1 (u/order-of-magnitude 0.5))
-(expect 0 (u/order-of-magnitude 4))
-(expect 1 (u/order-of-magnitude 12))
-(expect 2 (u/order-of-magnitude 444))
-(expect 3 (u/order-of-magnitude 1023))
-(expect 0 (u/order-of-magnitude 0))
-(expect 3 (u/order-of-magnitude -1444))
-
-
-;;; `update-when` and `update-in-when`
-(expect {:foo 2} (u/update-when {:foo 2} :bar inc))
-(expect {:foo 2 :bar 3} (u/update-when {:foo 2 :bar 2} :bar inc))
-
-(expect {:foo 2} (u/update-in-when {:foo 2} [:foo :bar] inc))
-(expect {:foo {:bar 3}} (u/update-in-when {:foo {:bar 2}} [:foo :bar] inc))
-
-
-;;; `index-of`
-(expect 2 (u/index-of pos? [-1 0 2 3]))
-(expect nil (u/index-of pos? [-1 0 -2 -3]))
-(expect nil (u/index-of pos? nil))
-(expect nil (u/index-of pos? []))
-
-
-;; `is-java-9-or-higher?`
-(expect
- false
- (u/is-java-9-or-higher? "1.8.0_141"))
-
-(expect
- (u/is-java-9-or-higher? "1.9.0_141"))
-
-(expect
- (u/is-java-9-or-higher? "10.0.1"))
-
-(expect
- (u/is-java-9-or-higher? "11.0.1"))
-
-;; make sure we can parse wacky version strings like `9-internal`: See #8282
-(expect
- (u/is-java-9-or-higher? "9-internal"))
-
-;; `snake-keys`
-(expect
- {:num_cans 2, :lisp_case? {:nested_maps? true}}
- (u/snake-keys {:num-cans 2, :lisp-case? {:nested-maps? true}}))
-
-
-;; `xor` & `xor-pred`
-(expect false (u/xor false false))
-(expect true (u/xor false true))
-(expect true (u/xor true false))
-(expect false (u/xor true true))
-
-(expect false (u/xor false false false))
-(expect true (u/xor false true false))
-(expect true (u/xor true false false))
-(expect false (u/xor true true false))
-
-(expect true (u/xor false false true))
-(expect false (u/xor false true true))
-(expect false (u/xor true false true))
-(expect false (u/xor true true true))
-
-(expect false (boolean ((u/xor-pred :a :b :c) {})))
-(expect true (boolean ((u/xor-pred :a :b :c) {:a 1})))
-(expect true (boolean ((u/xor-pred :a :b :c) {:b 1})))
-(expect true (boolean ((u/xor-pred :a :b :c) {:c 1})))
-(expect false (boolean ((u/xor-pred :a :b :c) {:a 1, :b 1})))
-(expect false (boolean ((u/xor-pred :a :b :c) {:a 1, :c 1})))
-(expect false (boolean ((u/xor-pred :a :b :c) {:b 1, :c 1})))
-(expect false (boolean ((u/xor-pred :a :b :c) {:a 1, :b 1, :c 1})))
-
-
-(expect nil (u/one-or-many nil))
-(expect [nil] (u/one-or-many [nil]))
-(expect [42] (u/one-or-many 42))
-(expect [42] (u/one-or-many [42]))
-
-
-(expect
- (ordered-map :a [] :b [] :c [:a] :d [:a :b :c] :e [:d])
- (u/topological-sort identity {:b []
- :c [:a]
- :e [:d]
- :d [:a :b :c]
- :a []}))
-
-(expect
- nil
- (u/topological-sort identity {}))
-
-(expect
- nil
- (u/topological-sort identity nil))
-
-;; `lower-case-en`
-(expect
- "id"
+(deftest select-nested-keys-test
+ (are+ [m keyseq expected] (= expected
+ (u/select-nested-keys m keyseq))
+ {:a 100, :b {:c 200, :d 300}} [:a [:b :d] :c] {:a 100, :b {:d 300}}
+ {:a 100, :b {:c 200, :d 300}} [:b] {:b {:c 200, :d 300}}
+ {:a 100, :b {:c 200, :d 300}} [[:b :c :d]] {:b {:c 200, :d 300}}
+ {:a 100, :b {:c 200, :d {:e 300}}} [[:b [:d :e]]] {:b {:d {:e 300}}}
+ {:a 100, :b {:c 200, :d {:e 300}}} [[:b :d]] {:b {:d {:e 300}}}
+ {:a {:b 100, :c 200}, :d {:e 300, :f 400}} [[:a :b] [:d :e]] {:a {:b 100}, :d {:e 300}}
+ {:a 100, :b {:c 200, :d 300}} [[:a]] {:a 100}
+ {:a 100, :b {:c 200, :d 300}} [:c] {}
+ nil [:c] {}
+ {} nil {}
+ {:a 100, :b {:c 200, :d 300}} [] {}
+ {} [:c] {}))
+
+(deftest base64-string?-test
+ (are+ [s expected] (= expected
+ (u/base64-string? s))
+ "ABc" true
+ "ABc/+asdasd==" true
+ 100 false
+ "<<>>" false
+ "{\"a\": 10}" false))
+
+(deftest occurances-of-substring-test
+ (testing "should return nil if one or both strings are nil or empty"
+ (are+ [s substr expected] (= expected
+ (u/occurances-of-substring s substr))
+ nil nil nil
+ nil "" nil
+ "" nil nil
+ "" "" nil
+ "ABC" "" nil
+ "" " ABC" nil
+ ;; non-empty strings
+ "ABC" "A" 1
+ "ABA" "A" 2
+ "AAA" "A" 3
+ "ABC" "{{id}}" 0
+ "WHERE ID = {{id}}" "{{id}}" 1
+ "WHERE ID = {{id}} OR USER_ID = {{id}}" "{{id}}" 2
+ "WHERE ID = {{id}} OR USER_ID = {{id}} OR TOUCAN_ID = {{id}} OR BIRD_ID = {{bird}}" "{{id}}" 3)))
+
+(deftest select-keys-test
+ (testing "select-non-nil-keys"
+ (is (= {:a 100}
+ (u/select-non-nil-keys {:a 100, :b nil} #{:a :b :c}))))
+ (testing "select-keys-when"
+ (is (= {:a 100, :b nil, :d 200}
+ (u/select-keys-when {:a 100, :b nil, :d 200, :e nil}
+ :present #{:a :b :c}
+ :non-nil #{:d :e :f})))))
+
+(deftest order-of-magnitude-test
+ (are+ [n expected] (= expected
+ (u/order-of-magnitude n))
+ 0.01 -2
+ 0.5 -1
+ 4 0
+ 12 1
+ 444 2
+ 1023 3
+ 0 0
+ -1444 3))
+
+(deftest update-when-test
+ (testing "update-when"
+ (are [m expected] (= expected
+ (u/update-when m :bar inc))
+ {:foo 2} {:foo 2}
+ {:foo 2 :bar 2} {:foo 2 :bar 3}))
+ (testing "update-in-when"
+ (are [m expected] (= expected
+ (u/update-in-when m [:foo :bar] inc))
+ {:foo 2} {:foo 2}
+ {:foo {:bar 2}} {:foo {:bar 3}})))
+
+(deftest index-of-test
+ (are [input expected] (= expected
+ (u/index-of pos? input))
+ [-1 0 2 3] 2
+ [-1 0 -2 -3] nil
+ nil nil
+ [] nil))
+
+(deftest snake-key-test
+ (is (= {:num_cans 2, :lisp_case? {:nested_maps? true}}
+ (u/snake-keys {:num-cans 2, :lisp-case? {:nested-maps? true}}))))
+
+(deftest one-or-many-test
+ (are+ [input expected] (= expected
+ (u/one-or-many input))
+ nil nil
+ [nil] [nil]
+ 42 [42]
+ [42] [42]))
+
+(deftest topological-sort-test
+ (are+ [input expected] (= expected
+ (u/topological-sort identity input))
+ {:b []
+ :c [:a]
+ :e [:d]
+ :d [:a :b :c]
+ :a []}
+ (ordered-map :a [] :b [] :c [:a] :d [:a :b :c] :e [:d])
+
+ {} nil
+ nil nil))
+
+(deftest lower-case-en-test
(let [original-locale (Locale/getDefault)]
(try
(Locale/setDefault (Locale/forLanguageTag "tr"))
;; `(str/lower-case "ID")` returns "ıd" in the Turkish locale
- (u/lower-case-en "ID")
+ (is (= "id"
+ (u/lower-case-en "ID")))
(finally
(Locale/setDefault original-locale)))))
+
+(deftest parse-currency-test
+ (are+ [s expected] (= expected
+ (u/parse-currency s))
+ nil nil
+ "" nil
+ " " nil
+ "$1,000" 1000.0M
+ "$1,000,000" 1000000.0M
+ "$1,000.00" 1000.0M
+ "€1.000" 1000.0M
+ "€1.000,00" 1000.0M
+ "€1.000.000,00" 1000000.0M
+ "-£127.54" -127.54M
+ "-127,54 €" -127.54M
+ "kr-127,54" -127.54M
+ "€ 127,54-" -127.54M
+ "Â¥200" 200.0M
+ "Â¥200." 200.0M
+ "$.05" 0.05M
+ "0.05" 0.05M))
+
+;; Local Variables:
+;; eval: (add-to-list (make-local-variable 'clojure-align-cond-forms) "are+")
+;; End:
--
GitLab