diff --git a/src/metabase/driver/sql_jdbc/execute.clj b/src/metabase/driver/sql_jdbc/execute.clj
index ea49bb7e11d19f7513df6c0db0c4d806a721f2a1..954d571ee426947ba5d18c780d0ee2cfad843175 100644
--- a/src/metabase/driver/sql_jdbc/execute.clj
+++ b/src/metabase/driver/sql_jdbc/execute.clj
@@ -180,10 +180,7 @@
;; This is what does the real work of canceling the query. We aren't checking the result of
;; `query-future` but this will cause an exception to be thrown, saying the query has been cancelled.
(.cancel stmt)
- (throw e))
- (catch Exception e
- (u/ignore-exceptions (.cancel stmt))
- e)))))
+ (throw e))))))
(defn- run-query
"Run the query itself."
@@ -248,8 +245,9 @@
(defn- set-timezone!
"Set the timezone for the current connection."
- [driver settings connection]
- (let [timezone (u/prog1 (:report-timezone settings)
+ {:arglists '([driver settings connection])}
+ [driver {:keys [report-timezone]} connection]
+ (let [timezone (u/prog1 report-timezone
(assert (re-matches #"[A-Za-z\/_]+" <>)))
format-string (set-timezone-sql driver)
sql (format format-string (str \' timezone \'))]
diff --git a/test/metabase/driver/postgres_test.clj b/test/metabase/driver/postgres_test.clj
index c49aa40faff483b96b4526d6a52b3b035260704b..138666efa1f2c637ffda14e75403d0832eccc9b0 100644
--- a/test/metabase/driver/postgres_test.clj
+++ b/test/metabase/driver/postgres_test.clj
@@ -23,7 +23,7 @@
[data :as data]
[util :as tu]]
[metabase.test.data
- [datasets :refer [expect-with-driver]]
+ [datasets :as datasets]
[interface :as tx]]
[toucan.db :as db]
[toucan.util.test :as tt]))
@@ -62,7 +62,7 @@
:user "camsaul"}))
;; Verify that we identify JSON columns and mark metadata properly during sync
-(expect-with-driver :postgres
+(datasets/expect-with-driver :postgres
:type/SerializedJSON
(data/with-temp-db
[_
@@ -84,7 +84,7 @@
[#uuid "84ed434e-80b4-41cf-9c88-e334427104ae"]]]])
;; Check that we can load a Postgres Database with a :type/UUID
-(expect-with-driver :postgres
+(datasets/expect-with-driver :postgres
[{:name "id", :base_type :type/Integer}
{:name "user_id", :base_type :type/UUID}]
(->> (data/dataset metabase.driver.postgres-test/with-uuid
@@ -95,21 +95,21 @@
;; Check that we can filter by a UUID Field
-(expect-with-driver :postgres
+(datasets/expect-with-driver :postgres
[[2 #uuid "4652b2e7-d940-4d55-a971-7e484566663e"]]
(rows (data/dataset metabase.driver.postgres-test/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)
-(expect-with-driver :postgres
+(datasets/expect-with-driver :postgres
[]
(rows (data/dataset metabase.driver.postgres-test/with-uuid
(data/run-mbql-query users
{:filter [:= $user_id nil]}))))
;; Check that we can filter by a UUID for SQL Field filters (#7955)
-(expect-with-driver :postgres
+(datasets/expect-with-driver :postgres
[[#uuid "4f01dcfd-13f7-430c-8e6f-e505c0851027" 1]]
(data/dataset metabase.driver.postgres-test/with-uuid
(rows (qp/process-query {:database (data/id)
@@ -132,7 +132,7 @@
["four_loko"]
["ouija_board"]]]])
-(expect-with-driver :postgres
+(datasets/expect-with-driver :postgres
{:columns ["id" "dotted.name"]
:rows [[1 "toucan_cage"]
[2 "four_loko"]
@@ -153,7 +153,7 @@
{:field-name "bird_id", :base-type :type/Integer, :fk :birds}]
[["Cam" 1]]]])
-(expect-with-driver :postgres
+(datasets/expect-with-driver :postgres
{:columns ["name" "name_2"]
:rows [["Cam" "Rasta"]]}
(-> (data/dataset metabase.driver.postgres-test/duplicate-names
@@ -171,7 +171,7 @@
;; Filtering by inet columns should add the appropriate SQL cast, e.g. `cast('192.168.1.1' AS inet)` (otherwise this
;; wouldn't work)
-(expect-with-driver :postgres
+(datasets/expect-with-driver :postgres
[[1]]
(rows (data/dataset metabase.driver.postgres-test/ip-addresses
(data/run-mbql-query addresses
@@ -200,7 +200,7 @@
;; 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.
-(expect-with-driver :postgres
+(datasets/expect-with-driver :postgres
{:tables #{(default-table-result "test_mview")}}
(do
(drop-if-exists-and-create-db! "materialized_views_test")
@@ -213,7 +213,7 @@
(driver/describe-database :postgres database)))))
;; Check that we properly fetch foreign tables.
-(expect-with-driver :postgres
+(datasets/expect-with-driver :postgres
{:tables (set (map default-table-result ["foreign_table" "local_table"]))}
(do
(drop-if-exists-and-create-db! "fdw_test")
@@ -232,7 +232,7 @@
;; 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)
-(expect-with-driver :postgres
+(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)
@@ -270,18 +270,18 @@
{:query "SELECT current_setting('TIMEZONE') AS timezone;"}))))
;; check that if we set report-timezone to US/Pacific that the session timezone is in fact US/Pacific
-(expect-with-driver :postgres
+(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
-(expect-with-driver :postgres
+(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
-(expect-with-driver :postgres
+(datasets/expect-with-driver :postgres
(get-timezone-with-report-timezone nil)
(get-timezone-with-report-timezone "Crunk Burger"))
@@ -299,12 +299,12 @@
:dbname "cool"
:additional-options "prepareThreshold=0"}))
-(expect-with-driver :postgres
+(datasets/expect-with-driver :postgres
"UTC"
(tu/db-timezone-id))
;; Make sure we're able to fingerprint TIME fields (#5911)
-(expect-with-driver :postgres
+(datasets/expect-with-driver :postgres
#{#metabase.models.field.FieldInstance{:name "start_time", :fingerprint {:global {:distinct-count 1
:nil% 0.0}
:type {:type/DateTime {:earliest "1970-01-01T22:00:00.000Z", :latest "1970-01-01T22:00:00.000Z"}}}}
@@ -366,14 +366,14 @@
(f database)))
;; check that we can actually fetch the enum types from a DB
-(expect-with-driver :postgres
+(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
-(expect-with-driver :postgres
+(datasets/expect-with-driver :postgres
{:name "birds"
:fields #{{:name "name",
:database-type "varchar"
@@ -390,7 +390,7 @@
(driver/describe-table :postgres db {:name "birds"}))))
;; check that when syncing the DB the enum types get recorded appropriately
-(expect-with-driver :postgres
+(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}}
@@ -402,12 +402,12 @@
;; check that values for enum types get wrapped in appropriate CAST() fn calls in `->honeysql`
-(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}]))
+(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
-(expect-with-driver :postgres
+(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\","
@@ -430,7 +430,7 @@
(select-keys [:rows :native_form]))))))
;; make sure schema/table/field names with hyphens in them work correctly (#8766)
-(expect-with-driver :postgres
+(datasets/expect-with-driver :postgres
[["Bird Hat"]]
(metabase.driver/with-driver :postgres
[{:name "angry_birds", :active true}]
@@ -453,3 +453,14 @@
: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])))
diff --git a/test/metabase/driver/sql_jdbc/execute_test.clj b/test/metabase/driver/sql_jdbc/execute_test.clj
index 6c201b7900e8fbefd67557844d01e7ad8399ef46..af3d81165f457b524adab54b55a335174deb311f 100644
--- a/test/metabase/driver/sql_jdbc/execute_test.clj
+++ b/test/metabase/driver/sql_jdbc/execute_test.clj
@@ -7,11 +7,16 @@
[query-processor :as qp]
[util :as u]]
[metabase.driver.sql-jdbc-test :as sql-jdbc-test]
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
+ [metabase.query-processor
+ [store :as qp.store]
+ [test-util :as qp.tu]]
[metabase.test
[data :as data]
[util :as tu]]
[metabase.test.data.datasets :as datasets]
- [metabase.test.util.log :as tu.log])
+ [metabase.test.util.log :as tu.log]
+ [schema.core :as s])
(:import [java.sql PreparedStatement ResultSet]))
(defn- do-with-max-rows [f]
@@ -185,3 +190,60 @@
(query-thunk))
(catch Throwable e
(throw e)))))))))
+
+;; If the DB throws an exception, is it properly returned by the query processor? Is it status :failed? (#9942)
+(tu/expect-schema
+ {:status (s/eq :failed)
+ :class (s/eq java.lang.Exception)
+ :error (s/eq "Column \"ADSASDASD\" not found")
+ :stacktrace [s/Str]
+ :query s/Any}
+ (qp/process-query
+ {:database (data/id)
+ :type :native
+ :native {:query "SELECT adsasdasd;"}}))
+
+;; do we run query with a timezone if one is present in the Settings?
+(defn- ran-with-timezone? [driver query]
+ (let [ran-with-timezone? (promise)
+ timezone (promise)]
+ (with-redefs [sql-jdbc.execute/run-query-without-timezone
+ (let [orig @#'sql-jdbc.execute/run-query-without-timezone]
+ (fn [& args]
+ (deliver ran-with-timezone? false)
+ (deliver timezone nil)
+ (apply orig args)))
+
+ sql-jdbc.execute/run-query-with-timezone
+ (let [orig @#'sql-jdbc.execute/run-query-with-timezone]
+ (fn [& args]
+ (deliver ran-with-timezone? true)
+ (apply orig args)))
+
+ sql-jdbc.execute/set-timezone!
+ (let [orig @#'sql-jdbc.execute/set-timezone!]
+ (fn [driver {:keys [report-timezone], :as settings} connection]
+ (deliver timezone report-timezone)
+ (orig driver settings connection)))]
+ (qp.tu/with-everything-store
+ (qp.store/store-database! (data/db))
+ (sql-jdbc.execute/execute-query driver query))
+ {:ran-with-timezone? (u/deref-with-timeout ran-with-timezone? 1000)
+ :timezone (u/deref-with-timeout timezone 1000)})))
+
+(expect
+ {:ran-with-timezone? false, :timezone nil}
+ (ran-with-timezone?
+ :h2
+ {:database (data/id)
+ :type :native
+ :native {:query "SELECT * FROM VENUES LIMIT 1;"}}))
+
+(expect
+ {:ran-with-timezone? true, :timezone "US/Pacific"}
+ (ran-with-timezone?
+ :h2
+ {:database (data/id)
+ :type :native
+ :native {:query "SELECT * FROM VENUES LIMIT 1;"}
+ :settings {:report-timezone "US/Pacific"}}))
diff --git a/test/metabase/query_processor/test_util.clj b/test/metabase/query_processor/test_util.clj
index fa3f21389487bfd1cdf3e7db5ad3c157d1d3c9a9..8de6987a1e08725e1c7fd6ad362fd15b9403f13b 100644
--- a/test/metabase/query_processor/test_util.clj
+++ b/test/metabase/query_processor/test_util.clj
@@ -12,16 +12,19 @@
[f]
(with-redefs [qp.store/table (fn [table-id]
(or (get-in @@#'qp.store/*store* [:tables table-id])
- (db/select-one (vec (cons Table qp.store/table-columns-to-fetch)), :id table-id)))
+ (db/select-one (into [Table] qp.store/table-columns-to-fetch), :id table-id)))
qp.store/field (fn [field-id]
(or (get-in @@#'qp.store/*store* [:fields field-id])
- (db/select-one (vec (cons Field qp.store/field-columns-to-fetch)), :id field-id)))]
+ (db/select-one (into [Field] qp.store/field-columns-to-fetch), :id field-id)))]
(qp.store/with-store
(f))))
(defmacro with-everything-store
"When testing a specific piece of middleware, you often need to load things into the QP store, but doing so can be
tedious. This macro swaps out the normal QP store backend with one that fetches Tables and Fields from the DB
- on-demand, making tests a lot nicer to write."
+ on-demand, making tests a lot nicer to write.
+
+ You still need to store the Database yourself, because the getter function takes no args; we don't know which DB
+ you'd want to fetch."
[& body]
`(do-with-everything-store (fn [] ~@body)))