diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn
index d14e7a0f7e7aa54512c41ea4fe3e7071000dd85f..f8fdabc586bc64b1d3a6c73b5d1592c12a88b66f 100644
--- a/.clj-kondo/config.edn
+++ b/.clj-kondo/config.edn
@@ -123,49 +123,52 @@
schema.core/either {:namespaces ["metabase.*"]}}}
:discouraged-var
- {clojure.core/print {:message "Use clojure.tools.logging instead of clojure.core/print"}
- clojure.core/println {:message "Use clojure.tools.logging instead of clojure.core/println"}
- clojure.core/printf {:message "Use clojure.tools.logging instead of clojure.core/printf"}
- clojure.core/prn {:message "Use clojure.tools.logging instead of clojure.core/prn"}
- clojure.core/pr {:message "Use clojure.tools.logging instead of clojure.core/pr"}
- clojure.pprint/pprint {:message "Use clojure.tools.logging instead of clojure.pprint/pprint"}
- clojure.string/lower-case {:message "Use metabase.util/lower-case-en instead of clojure.string/lower-case"}
- clojure.string/upper-case {:message "Use metabase.util/upper-case-en instead of clojure.string/upper-case"}
- honeysql.core/call {:message "Use hx/call instead because it is Honey SQL 2 friendly"}
- honeysql.core/raw {:message "Use hx/raw instead because it is Honey SQL 2 friendly"}
- java-time/with-clock {:message "Use mt/with-clock"}
- metabase.test/with-temp {:message "Use t2.with-temp/with-temp instead of metabase.test/with-temp"}
- ;; metabase.test/with-temp* {:message "Use t2.with-temp/with-temp instead of metabase.test/with-temp*"}
- toucan.db/select {:message "Use t2/select instead of toucan.db/select"}
- toucan.db/select-one {:message "Use t2/select-one instead of toucan.db/select-one"}
- toucan.db/select-one-id {:message "Use t2/select-one-pk instead of toucan.db/select-one-id"}
- toucan.db/select-one-field {:message "Use t2/select-one-fn instead of toucan.db/select-one-field"}
- toucan.db/select-ids {:message "Use t2/select-pks-set instead of toucan.db/select-ids"}
- toucan.db/select-field {:message "Use t2/select-fn instead of toucan.db/select-field"}
- toucan.db/select-field->field {:message "Use t2/select-fn->fn instead of toucan.db/select-field->field"}
- toucan.db/select-field->id {:message "Use t2/select-fn->pk instead of toucan.db/select-field->id"}
- toucan.db/select-id->field {:message "Use t2/select-pk->field instead of toucan.db/select-id->field"}
- toucan.db/simple-insert! {:message "Use t2/insert-returning-pks! with table name instead of toucan.db/simple-insert!"}
- toucan.db/simple-insert-many! {:message "Use t2/insert-returning-pks! with table name instead of toucan.db/simple-insert-many!"}
- toucan.db/simple-delete! {:message "Use t2/delete! with table name instead of toucan.db/simple-delete!"}
- toucan.db/delete! {:message "Use t2/delete! instead of toucan.db/delete!"}
- toucan.db/insert! {:message "Use t2/insert-returning-instances! instead of toucan.db/insert!"}
- toucan.db/update! {:message "Use t2/update! instead of toucan.db/update!"}
- toucan.db/update-where! {:message "Use t2/update! instead of toucan.db/update-where!"}
- toucan.db/query {:message "Use mdb.query/query instead of toucan.db/query"}
- toucan.db/count {:message "Use t2/count instead of toucan.db/count"}
- toucan.db/exists? {:message "Use t2/exists? instead of toucan.db/exists?"}
- toucan.db/transaction {:message "Use t2/with-transaction instead of toucan.db/transaction"}
- toucan.db/with-call-counting {:message "Use t2/with-call-count instead of toucan.db/with-call-counting"}
- toucan.db/execute! {:message "Use t2/query-one instead of toucan.db/execute!"}
- toucan.db/reducible-query {:message "Use mdb.query/reducible-query instead of toucan.db/reducible-query"}
- toucan.db/resolve-model {:message "Use metabase.db.util/resolve-model instead of toucan.db/resolve-model"}
- toucan.models/primary-key {:message "Use metabase.db.util/primary-key instead of toucan.models/primary-key"}
- toucan.models/model? {:message "Use mdb.u/toucan-model? instead of toucan.models/model?"}
- toucan.hydrate/hydrate {:message "Use t2/hydrate instead of toucan.hydrate/hydrate"}
- toucan.util.test/with-temp-defaults {:message "Use t2.with-temp/with-temp-defaults instead of toucan.util.test/with-temp-defaults"}
- toucan.util.test/with-temp {:message "Use t2.with-temp/with-temp instead of toucan.util.test/with-temp"}
- toucan.util.test/with-temp* {:message "Use t2.with-temp/with-temp instead of toucan.util.test/with-temp*"}}
+ {clojure.core/print {:message "Use clojure.tools.logging instead of clojure.core/print"}
+ clojure.core/println {:message "Use clojure.tools.logging instead of clojure.core/println"}
+ clojure.core/printf {:message "Use clojure.tools.logging instead of clojure.core/printf"}
+ clojure.core/prn {:message "Use clojure.tools.logging instead of clojure.core/prn"}
+ clojure.core/pr {:message "Use clojure.tools.logging instead of clojure.core/pr"}
+ clojure.java.jdbc/get-connection {:message "Use sql-jdbc.execute/do-with-connection-with-options (for drivers) or t2/with-connection (for the app DB) instead of jdbc/get-connection"}
+ clojure.java.jdbc/with-db-connection {:message "Use sql-jdbc.execute/do-with-connection-with-options (for drivers) or t2/with-connection (for the app DB) instead of jdbc/with-db-connection"}
+ clojure.java.jdbc/with-db-metadata {:message "Use sql-jdbc.execute/do-with-connection-with-options + .getMetaData instead of jdbc/with-db-metadata"}
+ clojure.pprint/pprint {:message "Use clojure.tools.logging instead of clojure.pprint/pprint"}
+ clojure.string/lower-case {:message "Use metabase.util/lower-case-en instead of clojure.string/lower-case"}
+ clojure.string/upper-case {:message "Use metabase.util/upper-case-en instead of clojure.string/upper-case"}
+ honeysql.core/call {:message "Use hx/call instead because it is Honey SQL 2 friendly"}
+ honeysql.core/raw {:message "Use hx/raw instead because it is Honey SQL 2 friendly"}
+ java-time/with-clock {:message "Use mt/with-clock"}
+ metabase.test/with-temp {:message "Use t2.with-temp/with-temp instead of metabase.test/with-temp"}
+ ;; metabase.test/with-temp* {:message "Use t2.with-temp/with-temp instead of metabase.test/with-temp*"}
+ toucan.db/select {:message "Use t2/select instead of toucan.db/select"}
+ toucan.db/select-one {:message "Use t2/select-one instead of toucan.db/select-one"}
+ toucan.db/select-one-id {:message "Use t2/select-one-pk instead of toucan.db/select-one-id"}
+ toucan.db/select-one-field {:message "Use t2/select-one-fn instead of toucan.db/select-one-field"}
+ toucan.db/select-ids {:message "Use t2/select-pks-set instead of toucan.db/select-ids"}
+ toucan.db/select-field {:message "Use t2/select-fn instead of toucan.db/select-field"}
+ toucan.db/select-field->field {:message "Use t2/select-fn->fn instead of toucan.db/select-field->field"}
+ toucan.db/select-field->id {:message "Use t2/select-fn->pk instead of toucan.db/select-field->id"}
+ toucan.db/select-id->field {:message "Use t2/select-pk->field instead of toucan.db/select-id->field"}
+ toucan.db/simple-insert! {:message "Use t2/insert-returning-pks! with table name instead of toucan.db/simple-insert!"}
+ toucan.db/simple-insert-many! {:message "Use t2/insert-returning-pks! with table name instead of toucan.db/simple-insert-many!"}
+ toucan.db/simple-delete! {:message "Use t2/delete! with table name instead of toucan.db/simple-delete!"}
+ toucan.db/delete! {:message "Use t2/delete! instead of toucan.db/delete!"}
+ toucan.db/insert! {:message "Use t2/insert-returning-instances! instead of toucan.db/insert!"}
+ toucan.db/update! {:message "Use t2/update! instead of toucan.db/update!"}
+ toucan.db/update-where! {:message "Use t2/update! instead of toucan.db/update-where!"}
+ toucan.db/query {:message "Use mdb.query/query instead of toucan.db/query"}
+ toucan.db/count {:message "Use t2/count instead of toucan.db/count"}
+ toucan.db/exists? {:message "Use t2/exists? instead of toucan.db/exists?"}
+ toucan.db/transaction {:message "Use t2/with-transaction instead of toucan.db/transaction"}
+ toucan.db/with-call-counting {:message "Use t2/with-call-count instead of toucan.db/with-call-counting"}
+ toucan.db/execute! {:message "Use t2/query-one instead of toucan.db/execute!"}
+ toucan.db/reducible-query {:message "Use mdb.query/reducible-query instead of toucan.db/reducible-query"}
+ toucan.db/resolve-model {:message "Use metabase.db.util/resolve-model instead of toucan.db/resolve-model"}
+ toucan.models/primary-key {:message "Use metabase.db.util/primary-key instead of toucan.models/primary-key"}
+ toucan.models/model? {:message "Use mdb.u/toucan-model? instead of toucan.models/model?"}
+ toucan.hydrate/hydrate {:message "Use t2/hydrate instead of toucan.hydrate/hydrate"}
+ toucan.util.test/with-temp-defaults {:message "Use t2.with-temp/with-temp-defaults instead of toucan.util.test/with-temp-defaults"}
+ toucan.util.test/with-temp {:message "Use t2.with-temp/with-temp instead of toucan.util.test/with-temp"}
+ toucan.util.test/with-temp* {:message "Use t2.with-temp/with-temp instead of toucan.util.test/with-temp*"}}
:discouraged-namespace
{camel-snake-kebab.core {:message "CSK is not Turkish-safe, use the versions in metabase.util instead."}
diff --git a/dev/src/dev.clj b/dev/src/dev.clj
index 9ba4951a9a89a75e5a47a36830d0741176f5093d..c9f233d89f9c44873fcaa0aa6e55972e6c06edd6 100644
--- a/dev/src/dev.clj
+++ b/dev/src/dev.clj
@@ -142,12 +142,17 @@
(try
(driver/with-driver driver
(letfn [(thunk []
- (with-open [conn (sql-jdbc.execute/connection-with-timezone driver (mt/db) (qp.timezone/report-timezone-id-if-supported driver (mt/db)))
- stmt (sql-jdbc.execute/prepared-statement driver conn sql params)
- rs (sql-jdbc.execute/execute-prepared-statement! driver stmt)]
- (let [rsmeta (.getMetaData rs)]
- {:cols (sql-jdbc.execute/column-metadata driver rsmeta)
- :rows (reduce conj [] (sql-jdbc.execute/reducible-rows driver rs rsmeta canceled-chan))})))]
+ (let [db (mt/db)]
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ db
+ {:session-timezone (qp.timezone/report-timezone-id-if-supported driver db)}
+ (fn [conn]
+ (with-open [stmt (sql-jdbc.execute/prepared-statement driver conn sql params)
+ rs (sql-jdbc.execute/execute-prepared-statement! driver stmt)]
+ (let [rsmeta (.getMetaData rs)]
+ {:cols (sql-jdbc.execute/column-metadata driver rsmeta)
+ :rows (reduce conj [] (sql-jdbc.execute/reducible-rows driver rs rsmeta canceled-chan))}))))))]
(if dataset
(data.impl/do-with-dataset (data.impl/resolve-dataset-definition *ns* dataset) thunk)
(thunk))))
diff --git a/docs/developers-guide/driver-changelog.md b/docs/developers-guide/driver-changelog.md
index bb0074963aa1a21f9a371e6750b5cba74d39fcf2..2de54e04b3e82a5db49633fa6d663fe4d4bfdff4 100644
--- a/docs/developers-guide/driver-changelog.md
+++ b/docs/developers-guide/driver-changelog.md
@@ -15,7 +15,15 @@ title: Driver interface changelog
An implemention of the multimethod `metabase.driver/database-supports?` for `:schemas` is required only if the
database doesn't store tables in schemas.
-- The multimethod `metabase.driver/supports?` has been deprecated in favor of `metabase.driver/database-supports?`. The existing default implementation of `database-supports?` currently calls `supports?`, but it will be removed in 0.55.0.
+- The multimethod `metabase.driver/supports?` has been deprecated in favor of `metabase.driver/database-supports?`.
+ The existing default implementation of `database-supports?` currently calls `supports?`, but it will be removed in
+ 0.50.0.
+
+- `metabase.driver.sql-jdbc.execute/connection-with-timezone` has been marked deprecated and is scheduled for removal
+ in Metabase 0.50.0. The new method `metabase.driver.sql-jdbc.execute/do-with-connection-with-options` replaces it.
+ Migration to the new method is straightforward. See PR [#22166](https://github.com/metabase/metabase/pull/22166) for
+ more information. You should use `metabase.driver.sql-jdbc.execute/do-with-connection-with-options` instead of
+ `clojure.java.jdbc/with-db-connection` or `clojure.java.jdbc/get-connection` going forward.
## Metabase 0.46.0
diff --git a/modules/drivers/athena/src/metabase/driver/athena.clj b/modules/drivers/athena/src/metabase/driver/athena.clj
index 6f29ac3730480dfca0e1d2a7bb2bbbfbce359806..326070bdfde6cf4c3dbf40cdfe8465c9e0e585fe 100644
--- a/modules/drivers/athena/src/metabase/driver/athena.clj
+++ b/modules/drivers/athena/src/metabase/driver/athena.clj
@@ -22,7 +22,7 @@
[metabase.util.i18n :refer [trs]]
[metabase.util.log :as log])
(:import
- (java.sql DatabaseMetaData)
+ (java.sql Connection DatabaseMetaData)
(java.time OffsetDateTime ZonedDateTime)))
(set! *warn-on-reflection* true)
@@ -368,12 +368,17 @@
(defmethod driver/describe-table :athena
[driver {{:keys [catalog]} :details, :as database} table]
- (jdbc/with-db-metadata [metadata (sql-jdbc.conn/db->pooled-connection-spec database)]
- (assoc (select-keys table [:name :schema])
- :fields (try
- (describe-table-fields metadata database driver table catalog)
- (catch Throwable _
- (set nil))))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ database
+ nil
+ (fn [^Connection conn]
+ (let [metadata (.getMetaData conn)]
+ (assoc (select-keys table [:name :schema])
+ :fields (try
+ (describe-table-fields metadata database driver table catalog)
+ (catch Throwable _
+ (set nil))))))))
(defn- get-tables
"Athena can query EXTERNAL and MANAGED tables."
@@ -424,8 +429,13 @@
; If we want to limit the initial connection to a specific database/schema, I think we'd have to do that here...
(defmethod driver/describe-database :athena
[driver {details :details, :as database}]
- {:tables (jdbc/with-db-metadata [metadata (sql-jdbc.conn/db->pooled-connection-spec database)]
- (fast-active-tables driver metadata details))})
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ database
+ nil
+ (fn [^Connection conn]
+ (let [metadata (.getMetaData conn)]
+ {:tables (fast-active-tables driver metadata details)}))))
; Unsure if this is the right way to approach building the parameterized query...but it works
(defn- prepare-query [driver {query :native, :as outer-query}]
diff --git a/modules/drivers/athena/test/metabase/test/data/athena.clj b/modules/drivers/athena/test/metabase/test/data/athena.clj
index 392030490a76dac3dd1ba7615e8d3ad582d6527f..6335b15f383b4426b75e4b59ce203639499c2dc3 100644
--- a/modules/drivers/athena/test/metabase/test/data/athena.clj
+++ b/modules/drivers/athena/test/metabase/test/data/athena.clj
@@ -7,6 +7,7 @@
[metabase.driver.athena :as athena]
[metabase.driver.ddl.interface :as ddl.i]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.driver.sql-jdbc.sync :as sql-jdbc.sync]
[metabase.driver.sql.util.unprepare :as unprepare]
[metabase.test.data.interface :as tx]
@@ -211,10 +212,14 @@
(defn- existing-databases
"Set of databases that already exist in our S3 bucket, so we don't try to create them a second time."
[]
- (jdbc/with-db-connection [conn (server-connection-spec)]
- (let [dbs (into #{} (map :database_name) (jdbc/query conn ["SHOW DATABASES;"]))]
- (log/infof "The following Athena databases have already been created: %s" (pr-str (sort dbs)))
- dbs)))
+ (sql-jdbc.execute/do-with-connection-with-options
+ :athena
+ (server-connection-spec)
+ nil
+ (fn [^java.sql.Connection conn]
+ (let [dbs (into #{} (map :database_name) (jdbc/query {:connection conn} ["SHOW DATABASES;"]))]
+ (log/infof "The following Athena databases have already been created: %s" (pr-str (sort dbs)))
+ dbs))))
(def ^:private ^:dynamic *allow-database-creation*
"Whether to allow database creation. This is normally disabled to prevent people from accidentally loading duplicate
diff --git a/modules/drivers/presto-jdbc/src/metabase/driver/presto_jdbc.clj b/modules/drivers/presto-jdbc/src/metabase/driver/presto_jdbc.clj
index 1c3462e6b7741b7dc07493d48a2a79e55549f47c..04f810dfae474d451cd2d43194f6ae5a018764c1 100644
--- a/modules/drivers/presto-jdbc/src/metabase/driver/presto_jdbc.clj
+++ b/modules/drivers/presto-jdbc/src/metabase/driver/presto_jdbc.clj
@@ -35,19 +35,8 @@
(:import
(com.facebook.presto.jdbc PrestoConnection)
(com.mchange.v2.c3p0 C3P0ProxyConnection)
- (java.sql
- Connection
- PreparedStatement
- ResultSet
- ResultSetMetaData
- Time
- Types)
- (java.time
- LocalDateTime
- LocalTime
- OffsetDateTime
- OffsetTime
- ZonedDateTime)
+ (java.sql Connection PreparedStatement ResultSet ResultSetMetaData Time Types)
+ (java.time LocalDateTime LocalTime OffsetDateTime OffsetTime ZonedDateTime)
(java.time.format DateTimeFormatter)
(java.time.temporal ChronoField Temporal)))
@@ -587,28 +576,34 @@
(defmethod driver/describe-database :presto-jdbc
[driver {{:keys [catalog schema] :as _details} :details :as database}]
- (with-open [conn (-> (sql-jdbc.conn/db->pooled-connection-spec database)
- jdbc/get-connection)]
- (let [schemas (if schema #{(describe-schema driver conn catalog schema)}
- (all-schemas driver conn catalog))]
- {:tables (reduce set/union schemas)})))
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ database
+ nil
+ (fn [^Connection conn]
+ (let [schemas (if schema #{(describe-schema driver conn catalog schema)}
+ (all-schemas driver conn catalog))]
+ {:tables (reduce set/union schemas)}))))
(defmethod driver/describe-table :presto-jdbc
[driver {{:keys [catalog] :as _details} :details :as database} {schema :schema, table-name :name}]
- (with-open [conn (-> (sql-jdbc.conn/db->pooled-connection-spec database)
- jdbc/get-connection)]
- (let [sql (describe-table-sql driver catalog schema table-name)]
- (log/trace (trs "Running statement in describe-table: {0}" sql))
- {:schema schema
- :name table-name
- :fields (into
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ database
+ nil
+ (fn [^Connection conn]
+ (let [sql (describe-table-sql driver catalog schema table-name)]
+ (log/trace (trs "Running statement in describe-table: {0}" sql))
+ {:schema schema
+ :name table-name
+ :fields (into
#{}
(map-indexed (fn [idx {:keys [column type] :as _col}]
- {:name column
- :database-type type
+ {:name column
+ :database-type type
:base-type (presto-type->base-type type)
:database-position idx}))
- (jdbc/reducible-query {:connection conn} sql))})))
+ (jdbc/reducible-query {:connection conn} sql))}))))
;;; The Presto JDBC driver DOES NOT support the `.getImportedKeys` method so just return `nil` here so the `:sql-jdbc`
;;; implementation doesn't try to use it.
@@ -657,26 +652,47 @@
^PrestoConnection [^C3P0ProxyConnection pooled-conn]
(.unwrap pooled-conn PrestoConnection))
-(defmethod sql-jdbc.execute/connection-with-timezone :presto-jdbc
- [driver database ^String timezone-id]
- ;; Presto supports setting the session timezone via a `PrestoConnection` instance method. Under the covers,
- ;; this is equivalent to the `X-Presto-Time-Zone` header in the HTTP request.
- (let [conn (.getConnection (sql-jdbc.execute/datasource-with-diagnostic-info! driver database))
- underlying-conn (pooled-conn->presto-conn conn)]
- (try
- (sql-jdbc.execute/set-best-transaction-level! driver conn)
- (when-not (str/blank? timezone-id)
- ;; set session time zone if defined
- (.setTimeZoneId underlying-conn timezone-id))
+;;; for some insane reason Presto's JDBC driver does not seem to work properly when reusing a Connection to execute
+;;; multiple PreparedStatements... this breaks the test-data loading code. To work around that, we'll track the original
+;;; Connection spec from the top-level call, and if the `:presto-jdbc/force-fresh?` is passed in to recursive calls
+;;; we'll create a NEW connection using the original spec every time. See for example the code
+;;; in [[metabase.test.data.presto-jdbc]]
+(def ^:dynamic ^:private *original-connection-spec* nil)
+
+(defn- set-connection-options! [driver ^java.sql.Connection conn {:keys [^String session-timezone write?], :as _options}]
+ (let [underlying-conn (pooled-conn->presto-conn conn)]
+ (sql-jdbc.execute/set-best-transaction-level! driver conn)
+ (when-not (str/blank? session-timezone)
+ ;; set session time zone if defined
+ (.setTimeZoneId underlying-conn session-timezone))
+ ;; as with statement and prepared-statement, cannot set holdability on the connection level
+ (let [read-only? (not write?)]
(try
- (.setReadOnly conn true)
+ (.setReadOnly conn read-only?)
(catch Throwable e
- (log/debug e (trs "Error setting connection to read-only"))))
- ;; as with statement and prepared-statement, cannot set holdability on the connection level
- conn
- (catch Throwable e
- (.close conn)
- (throw e)))))
+ (log/debug e (trs "Error setting connection read-only to {0}" (pr-str read-only?))))))))
+
+(defmethod sql-jdbc.execute/do-with-connection-with-options :presto-jdbc
+ [driver db-or-id-or-spec options f]
+ ;; Presto supports setting the session timezone via a `PrestoConnection` instance method. Under the covers,
+ ;; this is equivalent to the `X-Presto-Time-Zone` header in the HTTP request.
+ (cond
+ (nil? *original-connection-spec*)
+ (binding [*original-connection-spec* db-or-id-or-spec]
+ (sql-jdbc.execute/do-with-connection-with-options driver db-or-id-or-spec options f))
+
+ (:presto-jdbc/force-fresh? options)
+ (sql-jdbc.execute/do-with-connection-with-options driver *original-connection-spec* (dissoc options :presto-jdbc/force-fresh?) f)
+
+ :else
+ (sql-jdbc.execute/do-with-resolved-connection
+ driver
+ db-or-id-or-spec
+ (dissoc options :session-timezone)
+ (fn [^java.sql.Connection conn]
+ (when-not (sql-jdbc.execute/recursive-connection?)
+ (set-connection-options! driver conn options))
+ (f conn)))))
(defn- date-time->substitution [ts-str]
(sql.params.substitution/make-stmt-subs "from_iso8601_timestamp(?)" [ts-str]))
diff --git a/modules/drivers/presto-jdbc/test/metabase/driver/presto_jdbc_test.clj b/modules/drivers/presto-jdbc/test/metabase/driver/presto_jdbc_test.clj
index 9bd821b290b9d3d0f8c2cd70ca4c110f8e9e05d9..7974e9883723e61430122f94853f50f50fc73eff 100644
--- a/modules/drivers/presto-jdbc/test/metabase/driver/presto_jdbc_test.clj
+++ b/modules/drivers/presto-jdbc/test/metabase/driver/presto_jdbc_test.clj
@@ -1,6 +1,5 @@
(ns metabase.driver.presto-jdbc-test
(:require
- [clojure.java.jdbc :as jdbc]
[clojure.test :refer :all]
[honeysql.format :as hformat]
[java-time :as t]
@@ -9,6 +8,7 @@
[metabase.driver :as driver]
[metabase.driver.presto-jdbc :as presto-jdbc]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.driver.sql.query-processor :as sql.qp]
[metabase.models.database :refer [Database]]
[metabase.models.field :refer [Field]]
@@ -185,11 +185,14 @@
(defn- execute-ddl! [ddl-statements]
(mt/with-driver :presto-jdbc
- (let [jdbc-spec (sql-jdbc.conn/connection-details->spec :presto-jdbc (clone-db-details))]
- (with-open [conn (jdbc/get-connection jdbc-spec)]
- (doseq [ddl-stmt ddl-statements]
- (with-open [stmt (.prepareStatement conn ddl-stmt)]
- (.executeUpdate stmt)))))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ :presto-jdbc
+ (sql-jdbc.conn/connection-details->spec :presto-jdbc (clone-db-details))
+ {:write? true}
+ (fn [^java.sql.Connection conn]
+ (doseq [ddl-stmt ddl-statements]
+ (with-open [stmt (.prepareStatement conn ddl-stmt)]
+ (.executeUpdate stmt)))))))
(deftest specific-schema-sync-test
(mt/test-driver :presto-jdbc
diff --git a/modules/drivers/presto-jdbc/test/metabase/test/data/presto_jdbc.clj b/modules/drivers/presto-jdbc/test/metabase/test/data/presto_jdbc.clj
index f7bdfe26d50da6aa743076106a38ff9d1badf9cb..36abfdb311b4a61dc171a847f7ce2a4b0688aa31 100644
--- a/modules/drivers/presto-jdbc/test/metabase/test/data/presto_jdbc.clj
+++ b/modules/drivers/presto-jdbc/test/metabase/test/data/presto_jdbc.clj
@@ -4,7 +4,6 @@
[clojure.string :as str]
[clojure.test :refer :all]
[metabase.config :as config]
- [metabase.connection-pool :as connection-pool]
[metabase.driver :as driver]
[metabase.driver.ddl.interface :as ddl.i]
[metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
@@ -17,7 +16,7 @@
[metabase.test.data.sql.ddl :as ddl]
[metabase.util.log :as log])
(:import
- (java.sql Connection DriverManager PreparedStatement)))
+ (java.sql Connection PreparedStatement)))
(set! *warn-on-reflection* true)
@@ -117,29 +116,25 @@
[_ dbdef tabledef]
(load-data dbdef tabledef))
-(defn- jdbc-spec->connection
- "This is to work around some weird interplay between clojure.java.jdbc caching behavior of connections based on URL,
- combined with the fact that the Presto driver apparently closes the connection when it closes a prepare statement.
- Therefore, create a fresh connection from the DriverManager."
- ^Connection [jdbc-spec]
- (DriverManager/getConnection (format "jdbc:%s:%s" (:subprotocol jdbc-spec) (:subname jdbc-spec))
- (connection-pool/map->properties (select-keys jdbc-spec [:user :SSL]))))
-
(defmethod load-data/do-insert! :presto-jdbc
[driver spec table-identifier row-or-rows]
(let [statements (ddl/insert-rows-ddl-statements driver table-identifier row-or-rows)]
- (with-open [conn (jdbc-spec->connection spec)]
- (doseq [[^String sql & params] statements]
- (try
- (with-open [^PreparedStatement stmt (.prepareStatement conn sql)]
- (sql-jdbc.execute/set-parameters! driver stmt params)
- (let [tbl-nm ((comp last :components) (into {} table-identifier))
- rows-affected (.executeUpdate stmt)]
- (log/infof "[%s] Inserted %d rows into %s." driver rows-affected tbl-nm)))
- (catch Throwable e
- (throw (ex-info (format "[%s] Error executing SQL: %s" driver (ex-message e))
- {:driver driver, :sql sql, :params params}
- e))))))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ spec
+ {:write? true, :presto-jdbc/force-fresh? true}
+ (fn [^Connection conn]
+ (doseq [[^String sql & params] statements]
+ (try
+ (with-open [^PreparedStatement stmt (.prepareStatement conn sql)]
+ (sql-jdbc.execute/set-parameters! driver stmt params)
+ (let [tbl-nm ((comp last :components) (into {} table-identifier))
+ rows-affected (.executeUpdate stmt)]
+ (log/infof "[%s] Inserted %d rows into %s." driver rows-affected tbl-nm)))
+ (catch Throwable e
+ (throw (ex-info (format "[%s] Error executing SQL: %s" driver (ex-message e))
+ {:driver driver, :sql sql, :params params}
+ e)))))))))
(defmethod sql.tx/drop-db-if-exists-sql :presto-jdbc [_ _] nil)
(defmethod sql.tx/create-db-sql :presto-jdbc [_ _] nil)
diff --git a/modules/drivers/redshift/src/metabase/driver/redshift.clj b/modules/drivers/redshift/src/metabase/driver/redshift.clj
index ab5afe7b98541aee68ff7f181a3b40697307ea6b..ccf939d8898152955291afd00e8835a0fd45df94 100644
--- a/modules/drivers/redshift/src/metabase/driver/redshift.clj
+++ b/modules/drivers/redshift/src/metabase/driver/redshift.clj
@@ -130,22 +130,23 @@
[_]
"SET TIMEZONE TO %s;")
-;; This impl is basically the same as the default impl in `sql-jdbc.execute`, but doesn't attempt to make the
-;; connection read-only, because that seems to be causing problems for people
-(defmethod sql-jdbc.execute/connection-with-timezone :redshift
- [driver database ^String timezone-id]
- (let [conn (.getConnection (sql-jdbc.execute/datasource-with-diagnostic-info! driver database))]
- (try
- (sql-jdbc.execute/set-best-transaction-level! driver conn)
- (sql-jdbc.execute/set-time-zone-if-supported! driver conn timezone-id)
- (try
- (.setHoldability conn ResultSet/CLOSE_CURSORS_AT_COMMIT)
- (catch Throwable e
- (log/debug e (trs "Error setting default holdability for connection"))))
- conn
- (catch Throwable e
- (.close ^Connection conn)
- (throw e)))))
+;; This impl is basically the same as the default impl in [[metabase.driver.sql-jdbc.execute]], but doesn't attempt to
+;; make the connection read-only, because that seems to be causing problems for people
+(defmethod sql-jdbc.execute/do-with-connection-with-options :redshift
+ [driver db-or-id-or-spec {:keys [^String session-timezone], :as options} f]
+ (sql-jdbc.execute/do-with-resolved-connection
+ driver
+ db-or-id-or-spec
+ options
+ (fn [^Connection conn]
+ (when-not (sql-jdbc.execute/recursive-connection?)
+ (sql-jdbc.execute/set-best-transaction-level! driver conn)
+ (sql-jdbc.execute/set-time-zone-if-supported! driver conn session-timezone)
+ (try
+ (.setHoldability conn ResultSet/CLOSE_CURSORS_AT_COMMIT)
+ (catch Throwable e
+ (log/debug e (trs "Error setting default holdability for connection")))))
+ (f conn))))
(defn- prepare-statement ^PreparedStatement [^Connection conn sql]
(.prepareStatement conn
@@ -167,10 +168,13 @@
(defn- quote-literal-for-database
"This function invokes quote-literal-for-connection with a connection for the given database. See its docstring for
more detail."
- [database s]
- (let [jdbc-spec (sql-jdbc.conn/db->pooled-connection-spec database)]
- (with-open [conn (jdbc/get-connection jdbc-spec)]
- (quote-literal-for-connection conn s))))
+ [driver database s]
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ database
+ nil
+ (fn [conn]
+ (quote-literal-for-connection conn s))))
(defmethod sql.qp/->honeysql [:redshift :regex-match-first]
[driver [_ arg pattern]]
@@ -179,7 +183,7 @@
;; the parameter to REGEXP_SUBSTR can only be a string literal; neither prepared statement parameters nor encoding/
;; decoding functions seem to work (fails with java.sql.SQLExcecption: "The pattern must be a valid UTF-8 literal
;; character expression"), hence we will use a different function to safely escape it before splicing here
- [:raw (quote-literal-for-database (qp.store/database) pattern)]])
+ [:raw (quote-literal-for-database driver (qp.store/database) pattern)]])
(defmethod sql.qp/->honeysql [:redshift :replace]
[driver [_ arg pattern replacement]]
diff --git a/modules/drivers/redshift/test/metabase/driver/redshift_test.clj b/modules/drivers/redshift/test/metabase/driver/redshift_test.clj
index 3dd46b6c8dfef89b6885b55d7d975b862452ca87..bdab08c1caaef9d5faf95c01e23fce290762ec21 100644
--- a/modules/drivers/redshift/test/metabase/driver/redshift_test.clj
+++ b/modules/drivers/redshift/test/metabase/driver/redshift_test.clj
@@ -311,18 +311,22 @@
(try
(binding [redshift.test/*use-original-filtered-syncable-schemas-impl?* true]
(t2.with-temp/with-temp [Database db {:engine :redshift, :details (assoc db-det :user temp-username :password user-pw)}]
- (with-open [conn (jdbc/get-connection (sql-jdbc.conn/db->pooled-connection-spec db))]
- (let [schemas (reduce conj
- #{}
- (sql-jdbc.sync/filtered-syncable-schemas :redshift
- conn
- (.getMetaData conn)
- nil
- nil))]
- (testing "filtered-syncable-schemas for the user should contain the newly created random schema"
- (is (contains? schemas random-schema)))
- (testing "should not contain the current session-schema name (since that was never granted)"
- (is (not (contains? schemas (redshift.test/unique-session-schema)))))))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ :redshift
+ db
+ nil
+ (fn [^java.sql.Connection conn]
+ (let [schemas (reduce conj
+ #{}
+ (sql-jdbc.sync/filtered-syncable-schemas :redshift
+ conn
+ (.getMetaData conn)
+ nil
+ nil))]
+ (testing "filtered-syncable-schemas for the user should contain the newly created random schema"
+ (is (contains? schemas random-schema)))
+ (testing "should not contain the current session-schema name (since that was never granted)"
+ (is (not (contains? schemas (redshift.test/unique-session-schema))))))))))
(finally
(execute! (str "REVOKE USAGE ON SCHEMA %s FROM %s;%n"
"DROP USER IF EXISTS %s;%n"
@@ -341,19 +345,22 @@
(eduction
cat
[(orig metadata) [fake-schema-name]])))]
- (let [jdbc-spec (sql-jdbc.conn/db->pooled-connection-spec (mt/db))]
- (with-open [conn (jdbc/get-connection jdbc-spec)]
- (letfn [(schemas []
- (reduce
- conj
- #{}
- (sql-jdbc.sync/filtered-syncable-schemas :redshift conn (.getMetaData conn) nil nil)))]
- (testing "if schemas-with-usage-permissions is disabled, the ::fake-schema should come back"
- (with-redefs [redshift/reducible-schemas-with-usage-permissions (fn [_ reducible]
- reducible)]
- (is (contains? (schemas) fake-schema-name))))
- (testing "normally, ::fake-schema should be filtered out (because it does not exist)"
- (is (not (contains? (schemas) fake-schema-name)))))))))))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ :redshift
+ (mt/db)
+ nil
+ (fn [^java.sql.Connection conn]
+ (letfn [(schemas []
+ (reduce
+ conj
+ #{}
+ (sql-jdbc.sync/filtered-syncable-schemas :redshift conn (.getMetaData conn) nil nil)))]
+ (testing "if schemas-with-usage-permissions is disabled, the ::fake-schema should come back"
+ (with-redefs [redshift/reducible-schemas-with-usage-permissions (fn [_ reducible]
+ reducible)]
+ (is (contains? (schemas) fake-schema-name))))
+ (testing "normally, ::fake-schema should be filtered out (because it does not exist)"
+ (is (not (contains? (schemas) fake-schema-name)))))))))))))
(mt/defdataset numeric-unix-timestamps
[["timestamps"
diff --git a/modules/drivers/redshift/test/metabase/test/data/redshift.clj b/modules/drivers/redshift/test/metabase/test/data/redshift.clj
index d9b88d4e37c6d48b7ad9fcf4fcbd3f985b03bac4..6150e213a68117fe833f61ce251e965490bffd59 100644
--- a/modules/drivers/redshift/test/metabase/test/data/redshift.clj
+++ b/modules/drivers/redshift/test/metabase/test/data/redshift.clj
@@ -1,9 +1,9 @@
(ns metabase.test.data.redshift
(:require
- [clojure.java.jdbc :as jdbc]
[clojure.string :as str]
[java-time :as t]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.driver.sql-jdbc.sync :as sql-jdbc.sync]
[metabase.driver.sql.test-util.unique-prefix :as sql.tu.unique-prefix]
[metabase.test.data.interface :as tx]
@@ -165,11 +165,14 @@
(.execute stmt sql))))
(defmethod tx/before-run :redshift
- [_driver]
- (with-open [conn (jdbc/get-connection
- (sql-jdbc.conn/connection-details->spec :redshift @db-connection-details))]
- (delete-old-schemas! conn)
- (create-session-schema! conn)))
+ [driver]
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ (sql-jdbc.conn/connection-details->spec driver @db-connection-details)
+ {:write? true}
+ (fn [conn]
+ (delete-old-schemas! conn)
+ (create-session-schema! conn))))
(defonce ^:private ^{:arglists '([driver connection metadata _ _])}
original-filtered-syncable-schemas
diff --git a/modules/drivers/snowflake/src/metabase/driver/snowflake.clj b/modules/drivers/snowflake/src/metabase/driver/snowflake.clj
index a166d59405d523a850db3dd9ee7b2090e0e9fd6b..590bbed066be52bd81207e310acca65e12addf93 100644
--- a/modules/drivers/snowflake/src/metabase/driver/snowflake.clj
+++ b/modules/drivers/snowflake/src/metabase/driver/snowflake.clj
@@ -388,42 +388,49 @@
(sql.qp/with-driver-honey-sql-version driver
(sql.qp/->honeysql driver table)))]]}))
-(defmethod driver/describe-database :snowflake [driver database]
+(defmethod driver/describe-database :snowflake
+ [driver database]
(let [db-name (db-name database)
excluded-schemas (set (sql-jdbc.sync/excluded-schemas driver))]
(qp.store/with-store
(qp.store/fetch-and-store-database! (u/the-id database))
- (let [spec (sql-jdbc.conn/db->pooled-connection-spec database)
- sql (format "SHOW OBJECTS IN DATABASE \"%s\"" db-name)
+ (let [sql (format "SHOW OBJECTS IN DATABASE \"%s\"" db-name)
schema-patterns (driver.s/db-details->schema-filter-patterns "schema-filters" database)
[inclusion-patterns exclusion-patterns] schema-patterns]
(log/tracef "[Snowflake] %s" sql)
- (with-open [conn (jdbc/get-connection spec)]
- {:tables (into
- #{}
- (comp (filter (fn [{schema :schema_name, table-name :name}]
- (and (not (contains? excluded-schemas schema))
- (driver.s/include-schema? inclusion-patterns
- exclusion-patterns
- schema)
- (sql-jdbc.sync/have-select-privilege? driver conn schema table-name))))
- (map (fn [{schema :schema_name, table-name :name, remark :comment}]
- {:name table-name
- :schema schema
- :description (not-empty remark)})))
- (try
- (jdbc/reducible-query {:connection conn} sql)
- (catch Throwable e
- (throw (ex-info (trs "Error executing query: {0}" (ex-message e)) {:sql sql} e)))))})))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ database
+ nil
+ (fn [^Connection conn]
+ {:tables (into
+ #{}
+ (comp (filter (fn [{schema :schema_name, table-name :name}]
+ (and (not (contains? excluded-schemas schema))
+ (driver.s/include-schema? inclusion-patterns
+ exclusion-patterns
+ schema)
+ (sql-jdbc.sync/have-select-privilege? driver conn schema table-name))))
+ (map (fn [{schema :schema_name, table-name :name, remark :comment}]
+ {:name table-name
+ :schema schema
+ :description (not-empty remark)})))
+ (try
+ (jdbc/reducible-query {:connection conn} sql)
+ (catch Throwable e
+ (throw (ex-info (trs "Error executing query: {0}" (ex-message e)) {:sql sql} e)))))}))))))
(defmethod driver/describe-table :snowflake
[driver database table]
- (let [spec (sql-jdbc.conn/db->pooled-connection-spec database)]
- (with-open [conn (jdbc/get-connection spec)]
- (->> (assoc (select-keys table [:name :schema])
- :fields (sql-jdbc.sync/describe-table-fields driver conn table (db-name database)))
- ;; find PKs and mark them
- (sql-jdbc.sync/add-table-pks driver conn (db-name database))))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ database
+ nil
+ (fn [^Connection conn]
+ (->> (assoc (select-keys table [:name :schema])
+ :fields (sql-jdbc.sync/describe-table-fields driver conn table (db-name database)))
+ ;; find PKs and mark them
+ (sql-jdbc.sync/add-table-pks driver conn (db-name database))))))
(defn- escape-name-for-metadata [entity-name]
(when entity-name
@@ -471,9 +478,12 @@
[driver db-or-id-or-spec-or-conn table & [db-name-or-nil]]
(if (instance? Connection db-or-id-or-spec-or-conn)
(describe-table-fks* driver db-or-id-or-spec-or-conn table db-name-or-nil)
- (let [spec (sql-jdbc.conn/db->pooled-connection-spec db-or-id-or-spec-or-conn)]
- (with-open [conn (jdbc/get-connection spec)]
- (describe-table-fks* driver conn table db-name-or-nil)))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ db-or-id-or-spec-or-conn
+ nil
+ (fn [conn]
+ (describe-table-fks* driver conn table db-name-or-nil)))))
(defmethod driver/describe-table-fks :snowflake
[driver database table]
diff --git a/modules/drivers/snowflake/test/metabase/test/data/snowflake.clj b/modules/drivers/snowflake/test/metabase/test/data/snowflake.clj
index edde99360ac8c03a9a289d450d3a367f71064c01..92c89259849271b9f59d6be01785d93a45941cfb 100644
--- a/modules/drivers/snowflake/test/metabase/test/data/snowflake.clj
+++ b/modules/drivers/snowflake/test/metabase/test/data/snowflake.clj
@@ -3,6 +3,7 @@
[clojure.java.jdbc :as jdbc]
[clojure.string :as str]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.driver.sql.test-util.unique-prefix :as sql.tu.unique-prefix]
[metabase.driver.sql.util.unprepare :as unprepare]
[metabase.test.data.interface :as tx]
@@ -87,18 +88,22 @@
(defn- old-dataset-names
"Return a collection of all dataset names that are old -- prefixed with a date two days ago or older?"
[]
- (with-open [conn (jdbc/get-connection (no-db-connection-spec))]
- (let [metadata (.getMetaData conn)]
- (with-open [rset (.getCatalogs metadata)]
- (loop [acc []]
- (if-not (.next rset)
- acc
- ;; for whatever dumb reason the Snowflake JDBC driver always returns these as uppercase despite us making
- ;; them all lower-case
- (let [catalog (u/lower-case-en (.getString rset "TABLE_CAT"))
- acc (cond-> acc
- (sql.tu.unique-prefix/old-dataset-name? catalog) (conj catalog))]
- (recur acc))))))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ :snowflake
+ (no-db-connection-spec)
+ {:write? true}
+ (fn [^java.sql.Connection conn]
+ (let [metadata (.getMetaData conn)]
+ (with-open [rset (.getCatalogs metadata)]
+ (loop [acc []]
+ (if-not (.next rset)
+ acc
+ ;; for whatever dumb reason the Snowflake JDBC driver always returns these as uppercase despite us making
+ ;; them all lower-case
+ (let [catalog (u/lower-case-en (.getString rset "TABLE_CAT"))
+ acc (cond-> acc
+ (sql.tu.unique-prefix/old-dataset-name? catalog) (conj catalog))]
+ (recur acc)))))))))
(defn- delete-old-datasets!
"Delete any datasets prefixed by a date that is two days ago or older. See comments above."
@@ -109,19 +114,23 @@
#_{:clj-kondo/ignore [:discouraged-var]}
(println "[Snowflake] deleting old datasets...")
(when-let [old-datasets (not-empty (old-dataset-names))]
- (with-open [conn (jdbc/get-connection (no-db-connection-spec))
- stmt (.createStatement conn)]
- (doseq [dataset-name old-datasets]
- #_{:clj-kondo/ignore [:discouraged-var]}
- (println "[Snowflake] Deleting old dataset:" dataset-name)
- (try
- (.execute stmt (format "DROP DATABASE \"%s\";" dataset-name))
- ;; if this fails for some reason it's probably just because some other job tried to delete the dataset at the
- ;; same time. No big deal. Just log this and carry on trying to delete the other datasets. If we don't end up
- ;; deleting anything it's not the end of the world because it won't affect our ability to run our tests
- (catch Throwable e
- #_{:clj-kondo/ignore [:discouraged-var]}
- (println "[Snowflake] Error deleting old dataset:" (ex-message e))))))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ :snowflake
+ (no-db-connection-spec)
+ {:write? true}
+ (fn [^java.sql.Connection conn]
+ (with-open [stmt (.createStatement conn)]
+ (doseq [dataset-name old-datasets]
+ #_{:clj-kondo/ignore [:discouraged-var]}
+ (println "[Snowflake] Deleting old dataset:" dataset-name)
+ (try
+ (.execute stmt (format "DROP DATABASE \"%s\";" dataset-name))
+ ;; if this fails for some reason it's probably just because some other job tried to delete the dataset at the
+ ;; same time. No big deal. Just log this and carry on trying to delete the other datasets. If we don't end up
+ ;; deleting anything it's not the end of the world because it won't affect our ability to run our tests
+ (catch Throwable e
+ #_{:clj-kondo/ignore [:discouraged-var]}
+ (println "[Snowflake] Error deleting old dataset:" (ex-message e))))))))))
(defonce ^:private deleted-old-datasets?
(atom false))
diff --git a/modules/drivers/sparksql/src/metabase/driver/sparksql.clj b/modules/drivers/sparksql/src/metabase/driver/sparksql.clj
index c167d436740c5cbecda2a31715fa5343a27c5168..463038a71cacc35e428ca9825f466b6e56c5e147 100644
--- a/modules/drivers/sparksql/src/metabase/driver/sparksql.clj
+++ b/modules/drivers/sparksql/src/metabase/driver/sparksql.clj
@@ -114,14 +114,18 @@
;; workaround for SPARK-9686 Spark Thrift server doesn't return correct JDBC metadata
(defmethod driver/describe-database :sparksql
- [_ database]
+ [driver database]
{:tables
- (with-open [conn (jdbc/get-connection (sql-jdbc.conn/db->pooled-connection-spec database))]
- (set
- (for [{:keys [database tablename tab_name], table-namespace :namespace} (jdbc/query {:connection conn} ["show tables"])]
- {:name (or tablename tab_name) ; column name differs depending on server (SparkSQL, hive, Impala)
- :schema (or (not-empty database)
- (not-empty table-namespace))})))})
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ database
+ nil
+ (fn [^Connection conn]
+ (set
+ (for [{:keys [database tablename tab_name], table-namespace :namespace} (jdbc/query {:connection conn} ["show tables"])]
+ {:name (or tablename tab_name) ; column name differs depending on server (SparkSQL, hive, Impala)
+ :schema (or (not-empty database)
+ (not-empty table-namespace))}))))})
;; Hive describe table result has commented rows to distinguish partitions
(defn- valid-describe-table-row? [{:keys [col_name data_type]}]
@@ -135,19 +139,23 @@
{:name table-name
:schema schema
:fields
- (with-open [conn (jdbc/get-connection (sql-jdbc.conn/db->pooled-connection-spec database))]
- (let [results (jdbc/query {:connection conn} [(format
- "describe %s"
- (sql.u/quote-name driver :table
- (dash-to-underscore schema)
- (dash-to-underscore table-name)))])]
- (set
- (for [[idx {col-name :col_name, data-type :data_type, :as result}] (m/indexed results)
- :when (valid-describe-table-row? result)]
- {:name col-name
- :database-type data-type
- :base-type (sql-jdbc.sync/database-type->base-type :hive-like (keyword data-type))
- :database-position idx}))))})
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ database
+ nil
+ (fn [^Connection conn]
+ (let [results (jdbc/query {:connection conn} [(format
+ "describe %s"
+ (sql.u/quote-name driver :table
+ (dash-to-underscore schema)
+ (dash-to-underscore table-name)))])]
+ (set
+ (for [[idx {col-name :col_name, data-type :data_type, :as result}] (m/indexed results)
+ :when (valid-describe-table-row? result)]
+ {:name col-name
+ :database-type data-type
+ :base-type (sql-jdbc.sync/database-type->base-type :hive-like (keyword data-type))
+ :database-position idx})))))})
;; bound variables are not supported in Spark SQL (maybe not Hive either, haven't checked)
(defmethod driver/execute-reducible-query :sparksql
@@ -167,15 +175,16 @@
;; 2. SparkSQL doesn't support session timezones (at least our driver doesn't support it)
;; 3. SparkSQL doesn't support making connections read-only
;; 4. SparkSQL doesn't support setting the default result set holdability
-(defmethod sql-jdbc.execute/connection-with-timezone :sparksql
- [driver database _timezone-id]
- (let [conn (.getConnection (sql-jdbc.execute/datasource-with-diagnostic-info! driver database))]
- (try
- (.setTransactionIsolation conn Connection/TRANSACTION_READ_UNCOMMITTED)
- conn
- (catch Throwable e
- (.close conn)
- (throw e)))))
+(defmethod sql-jdbc.execute/do-with-connection-with-options :sparksql
+ [driver db-or-id-or-spec options f]
+ (sql-jdbc.execute/do-with-resolved-connection
+ driver
+ db-or-id-or-spec
+ options
+ (fn [^Connection conn]
+ (when-not (sql-jdbc.execute/recursive-connection?)
+ (.setTransactionIsolation conn Connection/TRANSACTION_READ_UNCOMMITTED))
+ (f conn))))
;; 1. SparkSQL doesn't support setting holdability type to `CLOSE_CURSORS_AT_COMMIT`
(defmethod sql-jdbc.execute/prepared-statement :sparksql
diff --git a/modules/drivers/sparksql/test/metabase/test/data/sparksql.clj b/modules/drivers/sparksql/test/metabase/test/data/sparksql.clj
index 7dbb67c8732acd635a1a5e52070affc47ac2bf09..7385e657a64e4e08d5e8f4c12c0697064fb6ee5d 100644
--- a/modules/drivers/sparksql/test/metabase/test/data/sparksql.clj
+++ b/modules/drivers/sparksql/test/metabase/test/data/sparksql.clj
@@ -5,6 +5,7 @@
[metabase.config :as config]
[metabase.driver :as driver]
[metabase.driver.ddl.interface :as ddl.i]
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.driver.sql.util :as sql.u]
[metabase.driver.sql.util.unprepare :as unprepare]
[metabase.test.data.interface :as tx]
@@ -84,15 +85,19 @@
(defmethod load-data/do-insert! :sparksql
[driver spec table-identifier row-or-rows]
(let [statements (ddl/insert-rows-ddl-statements driver table-identifier row-or-rows)]
- (with-open [conn (jdbc/get-connection spec)]
- (try
- (.setAutoCommit conn false)
- (doseq [sql+args statements]
- (jdbc/execute! {:connection conn} sql+args {:transaction? false}))
- (catch java.sql.SQLException e
- (log/infof "Error inserting data: %s" (u/pprint-to-str 'red statements))
- (jdbc/print-sql-exception-chain e)
- (throw e))))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ spec
+ {:write? true}
+ (fn [^java.sql.Connection conn]
+ (try
+ (.setAutoCommit conn false)
+ (doseq [sql+args statements]
+ (jdbc/execute! {:connection conn} sql+args {:transaction? false}))
+ (catch java.sql.SQLException e
+ (log/infof "Error inserting data: %s" (u/pprint-to-str 'red statements))
+ (jdbc/print-sql-exception-chain e)
+ (throw e)))))))
(defmethod load-data/load-data! :sparksql [& args]
(apply load-data/load-data-add-ids! args))
diff --git a/modules/drivers/sqlite/src/metabase/driver/sqlite.clj b/modules/drivers/sqlite/src/metabase/driver/sqlite.clj
index f0f1a87cc90766b7a7f0012f91fd81da43c075c3..21846773c8b66ba17a573522ba0eaba555b8b506 100644
--- a/modules/drivers/sqlite/src/metabase/driver/sqlite.clj
+++ b/modules/drivers/sqlite/src/metabase/driver/sqlite.clj
@@ -409,15 +409,16 @@
;; SQLite's JDBC driver is fussy and won't let you change connections to read-only after you create them. So skip that
;; step. SQLite doesn't have a notion of session timezones so don't do that either. The only thing we're doing here from
;; the default impl is setting the transaction isolation level
-(defmethod sql-jdbc.execute/connection-with-timezone :sqlite
- [driver database _timezone-id]
- (let [conn (.getConnection (sql-jdbc.execute/datasource-with-diagnostic-info! driver database))]
- (try
- (sql-jdbc.execute/set-best-transaction-level! driver conn)
- conn
- (catch Throwable e
- (.close conn)
- (throw e)))))
+(defmethod sql-jdbc.execute/do-with-connection-with-options :sqlite
+ [driver db-or-id-or-spec options f]
+ (sql-jdbc.execute/do-with-resolved-connection
+ driver
+ db-or-id-or-spec
+ options
+ (fn [^Connection conn]
+ (when-not (sql-jdbc.execute/recursive-connection?)
+ (sql-jdbc.execute/set-best-transaction-level! driver conn))
+ (f conn))))
;; SQLite's JDBC driver is dumb and complains if you try to call `.setFetchDirection` on the Connection
(defmethod sql-jdbc.execute/prepared-statement :sqlite
diff --git a/modules/drivers/sqlserver/test/metabase/driver/sqlserver_test.clj b/modules/drivers/sqlserver/test/metabase/driver/sqlserver_test.clj
index 09ce34524ee20ba814705088a0496f5d0f8b4312..05b670f4d5807d98942dca1f815d2a311d622198 100644
--- a/modules/drivers/sqlserver/test/metabase/driver/sqlserver_test.clj
+++ b/modules/drivers/sqlserver/test/metabase/driver/sqlserver_test.clj
@@ -190,26 +190,30 @@
;; we're doing things here with low-level calls to HoneySQL (emulating what the QP does) instead of using normal
;; QP pathways because `SET LANGUAGE` doesn't seem to persist to subsequent executions so to test that things
;; are working we need to add to in from of the query we're trying to check
- (with-open [conn (sql-jdbc.execute/connection-with-timezone :sqlserver (mt/db) (qp.timezone/report-timezone-id-if-supported :sqlserver (mt/db)))]
- (.setAutoCommit conn false)
- (try
- (doseq [[sql & params] [["DROP TABLE IF EXISTS temp;"]
- ["CREATE TABLE temp (d DATETIME2);"]
- ["INSERT INTO temp (d) VALUES (?)" #t "2019-02-08T00:00:00Z"]
- ["SET LANGUAGE Italian;"]]]
- (with-open [stmt (sql-jdbc.execute/prepared-statement :sqlserver conn sql params)]
- (.execute stmt)))
- (let [[sql & params] (hsql/format {:select [[(sql.qp/date :sqlserver :month :temp.d) :my-date]]
- :from [:temp]}
- :quoting :ansi, :allow-dashed-names? true)]
- (with-open [stmt (sql-jdbc.execute/prepared-statement :sqlserver conn sql params)
- rs (sql-jdbc.execute/execute-prepared-statement! :sqlserver stmt)]
- (let [row-thunk (sql-jdbc.execute/row-thunk :sqlserver rs (.getMetaData rs))]
- (is (= [#t "2019-02-01"]
- (row-thunk))))))
- ;; rollback transaction so `temp` table gets discarded
- (finally
- (.rollback conn)))))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ :sqlserver
+ (mt/db)
+ {:session-timezone (qp.timezone/report-timezone-id-if-supported :sqlserver (mt/db))}
+ (fn [^java.sql.Connection conn]
+ (.setAutoCommit conn false)
+ (try
+ (doseq [[sql & params] [["DROP TABLE IF EXISTS temp;"]
+ ["CREATE TABLE temp (d DATETIME2);"]
+ ["INSERT INTO temp (d) VALUES (?)" #t "2019-02-08T00:00:00Z"]
+ ["SET LANGUAGE Italian;"]]]
+ (with-open [stmt (sql-jdbc.execute/prepared-statement :sqlserver conn sql params)]
+ (.execute stmt)))
+ (let [[sql & params] (hsql/format {:select [[(sql.qp/date :sqlserver :month :temp.d) :my-date]]
+ :from [:temp]}
+ :quoting :ansi, :allow-dashed-names? true)]
+ (with-open [stmt (sql-jdbc.execute/prepared-statement :sqlserver conn sql params)
+ rs (sql-jdbc.execute/execute-prepared-statement! :sqlserver stmt)]
+ (let [row-thunk (sql-jdbc.execute/row-thunk :sqlserver rs (.getMetaData rs))]
+ (is (= [#t "2019-02-01"]
+ (row-thunk))))))
+ ;; rollback transaction so `temp` table gets discarded
+ (finally
+ (.rollback conn))))))))
(deftest unprepare-test
(mt/test-driver :sqlserver
@@ -232,13 +236,17 @@
#_{:clj-kondo/ignore [:discouraged-var]}
(testing (format "Convert %s to SQL literal" (colorize/magenta (with-out-str (pr t))))
(let [sql (format "SELECT %s AS t;" (unprepare/unprepare-value :sqlserver t))]
- (with-open [conn (sql-jdbc.execute/connection-with-timezone :sqlserver (mt/db) nil)
- stmt (sql-jdbc.execute/prepared-statement :sqlserver conn sql nil)
- rs (sql-jdbc.execute/execute-prepared-statement! :sqlserver stmt)]
- (let [row-thunk (sql-jdbc.execute/row-thunk :sqlserver rs (.getMetaData rs))]
- (is (= [expected]
- (row-thunk))
- (format "SQL %s should return %s" (colorize/blue (pr-str sql)) (colorize/green expected))))))))))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ :sqlserver
+ (mt/db)
+ nil
+ (fn [^java.sql.Connection conn]
+ (with-open [stmt (sql-jdbc.execute/prepared-statement :sqlserver conn sql nil)
+ rs (sql-jdbc.execute/execute-prepared-statement! :sqlserver stmt)]
+ (let [row-thunk (sql-jdbc.execute/row-thunk :sqlserver rs (.getMetaData rs))]
+ (is (= [expected]
+ (row-thunk))
+ (format "SQL %s should return %s" (colorize/blue (pr-str sql)) (colorize/green expected))))))))))))))
(defn- pretty-sql [s]
(str/replace s #"\"" ""))
diff --git a/modules/drivers/vertica/test/metabase/test/data/vertica.clj b/modules/drivers/vertica/test/metabase/test/data/vertica.clj
index cb6e9e0d4f72a7393f83dd519bf2c735af2429dc..798254f385c0e873599eb7fca015ccf1fbc5c868 100644
--- a/modules/drivers/vertica/test/metabase/test/data/vertica.clj
+++ b/modules/drivers/vertica/test/metabase/test/data/vertica.clj
@@ -8,6 +8,7 @@
[java-time :as t]
[medley.core :as m]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.test :as mt]
[metabase.test.data.interface :as tx]
[metabase.test.data.sql :as sql.tx]
@@ -123,42 +124,46 @@
(defn- load-rows-from-csv!
"Load rows from a CSV file into a Table."
- [_driver {:keys [database-name], :as _dbdef} {:keys [table-name rows], :as _tabledef} filename]
- (let [table-identifier (sql.tx/qualify-and-quote :vertica database-name table-name)]
- (with-open [conn (jdbc/get-connection (dbspec))]
- (letfn [(execute! [sql]
- (try
- (jdbc/execute! {:connection conn} sql)
- (catch Throwable e
- (throw (ex-info "Error executing SQL" {:sql sql, :spec (dbspec)} e)))))
- (actual-rows []
- (u/ignore-exceptions
- (jdbc/query {:connection conn}
- (format "SELECT * FROM %s ORDER BY id ASC;" table-identifier))))]
- (try
- ;; make sure the Table is empty
- (execute! (format "TRUNCATE TABLE %s" table-identifier))
- ;; load the rows from the CSV file
- (let [[num-rows-inserted] (execute! (format "COPY %s FROM LOCAL '%s' DELIMITER ','"
- table-identifier
- filename))]
- ;; it should return the number of rows inserted; make sure this matches what we expected
- (when-not (= num-rows-inserted (count rows))
- (throw (ex-info (format "Expected %d rows to be inserted, but only %d were" (count rows) num-rows-inserted)
- {:inserted-rows (take 100 (actual-rows))}))))
- ;; make sure SELECT COUNT(*) matches as well
- (let [[{actual-num-rows :count}] (jdbc/query {:connection conn}
- (format "SELECT count(*) FROM %s;" table-identifier))]
- (when-not (= actual-num-rows (count rows))
- (throw (ex-info (format "Expected count(*) to return %d, but only got %d" (count rows) actual-num-rows)
- {:inserted-rows (take 100 (actual-rows))}))))
- ;; success!
- :ok
- (catch Throwable e
- (throw (ex-info "Error loading rows from CSV file"
- {:filename filename
- :rows (take 10 (str/split-lines (slurp filename)))}
- e))))))))
+ [driver {:keys [database-name], :as _dbdef} {:keys [table-name rows], :as _tabledef} filename]
+ (let [table-identifier (sql.tx/qualify-and-quote driver database-name table-name)]
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ (dbspec)
+ {:write? true}
+ (fn [^java.sql.Connection conn]
+ (letfn [(execute! [sql]
+ (try
+ (jdbc/execute! {:connection conn} sql)
+ (catch Throwable e
+ (throw (ex-info "Error executing SQL" {:sql sql, :spec (dbspec)} e)))))
+ (actual-rows []
+ (u/ignore-exceptions
+ (jdbc/query {:connection conn}
+ (format "SELECT * FROM %s ORDER BY id ASC;" table-identifier))))]
+ (try
+ ;; make sure the Table is empty
+ (execute! (format "TRUNCATE TABLE %s" table-identifier))
+ ;; load the rows from the CSV file
+ (let [[num-rows-inserted] (execute! (format "COPY %s FROM LOCAL '%s' DELIMITER ','"
+ table-identifier
+ filename))]
+ ;; it should return the number of rows inserted; make sure this matches what we expected
+ (when-not (= num-rows-inserted (count rows))
+ (throw (ex-info (format "Expected %d rows to be inserted, but only %d were" (count rows) num-rows-inserted)
+ {:inserted-rows (take 100 (actual-rows))}))))
+ ;; make sure SELECT COUNT(*) matches as well
+ (let [[{actual-num-rows :count}] (jdbc/query {:connection conn}
+ (format "SELECT count(*) FROM %s;" table-identifier))]
+ (when-not (= actual-num-rows (count rows))
+ (throw (ex-info (format "Expected count(*) to return %d, but only got %d" (count rows) actual-num-rows)
+ {:inserted-rows (take 100 (actual-rows))}))))
+ ;; success!
+ :ok
+ (catch Throwable e
+ (throw (ex-info "Error loading rows from CSV file"
+ {:filename filename
+ :rows (take 10 (str/split-lines (slurp filename)))}
+ e)))))))))
(defmethod load-data/load-data! :vertica
[driver dbdef {:keys [rows], :as tabledef}]
diff --git a/src/metabase/db/liquibase.clj b/src/metabase/db/liquibase.clj
index 4b3ec2763830271c22e7148cd17294a6e0606193..2d9e1e521a44770046d2d2d84550964377941f2c 100644
--- a/src/metabase/db/liquibase.clj
+++ b/src/metabase/db/liquibase.clj
@@ -194,7 +194,7 @@
[db-type :- s/Keyword
conn :- java.sql.Connection]
(let [liquibase-table-name (changelog-table-name db-type)
- fresh-install? (jdbc/with-db-metadata [meta {:connection conn}] ; don't migrate on fresh install
+ fresh-install? (let [meta (.getMetaData conn)] ; don't migrate on fresh install
(empty? (jdbc/metadata-query
(.getTables meta nil nil liquibase-table-name (u/varargs String ["TABLE"])))))
statement (format "UPDATE %s SET FILENAME = ?" liquibase-table-name)]
diff --git a/src/metabase/db/setup.clj b/src/metabase/db/setup.clj
index df471fe10f3f443e5502be31d76a164c60af6dd4..77067cb71ee069d726a09b5f0212d5262ffb8c3a 100644
--- a/src/metabase/db/setup.clj
+++ b/src/metabase/db/setup.clj
@@ -68,8 +68,7 @@
& args :- [s/Any]]
(with-open [conn (.getConnection data-source)]
(.setAutoCommit conn false)
- ;; Set up liquibase and let it do its thing
-
+ ;; Set up liquibase and let it do its thing
(log/info (trs "Setting up Liquibase..."))
(liquibase/with-liquibase [liquibase conn]
(try
diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj
index 0466de95b923b3d69cf34b8da535709aa67d1c7c..7d937c5f36ffa5bb80fa435e9f8cd1ddbc74d7aa 100644
--- a/src/metabase/driver.clj
+++ b/src/metabase/driver.clj
@@ -830,18 +830,21 @@
dispatch-on-initialized-driver
:hierarchy #'hierarchy)
+;;; FIXME -- should be named `create-table!`
(defmulti create-table
"Create a table named `table-name`. If the table already exists it will throw an error."
{:added "0.47.0", :arglists '([driver db-id table-name col->type])}
dispatch-on-initialized-driver
:hierarchy #'hierarchy)
+;;; FIXME -- should be named `drop-table!`
(defmulti drop-table
"Drop a table named `table-name`. If the table doesn't exist it will not be dropped."
{:added "0.47.0", :arglists '([driver db-id table-name])}
dispatch-on-initialized-driver
:hierarchy #'hierarchy)
+;;; FIXME -- should be named `insert-into!`
(defmulti insert-into
"Insert `values` into a table named `table-name`. `values` is a sequence of rows, where each row's order matches
`column-names`."
diff --git a/src/metabase/driver/h2.clj b/src/metabase/driver/h2.clj
index 0ccf6f49d130df0adf549bc5d73a078b451181da..095c40bc3bfc7083d552d01242612d7dcafadddb 100644
--- a/src/metabase/driver/h2.clj
+++ b/src/metabase/driver/h2.clj
@@ -478,19 +478,20 @@
[_]
#{"INFORMATION_SCHEMA"})
-(defmethod sql-jdbc.execute/connection-with-timezone :h2
- [driver database ^String _timezone-id]
+(defmethod sql-jdbc.execute/do-with-connection-with-options :h2
+ [driver db-or-id-or-spec {:keys [write?], :as options} f]
;; h2 doesn't support setting timezones, or changing the transaction level without admin perms, so we can skip those
;; steps that are in the default impl
- (let [conn (.getConnection (sql-jdbc.execute/datasource-with-diagnostic-info! driver database))]
- (try
- ;; in H2, setting readOnly to true doesn't prevent writes
- ;; see https://github.com/h2database/h2database/issues/1163
- (doto conn
- (.setReadOnly true))
- (catch Throwable e
- (.close conn)
- (throw e)))))
+ (sql-jdbc.execute/do-with-resolved-connection
+ driver
+ db-or-id-or-spec
+ (dissoc options :session-timezone)
+ (fn [^java.sql.Connection conn]
+ (when-not (sql-jdbc.execute/recursive-connection?)
+ ;; in H2, setting readOnly to true doesn't prevent writes
+ ;; see https://github.com/h2database/h2database/issues/1163
+ (.setReadOnly conn (not write?)))
+ (f conn))))
;; de-CLOB any CLOB values that come back
(defmethod sql-jdbc.execute/read-column-thunk :h2
diff --git a/src/metabase/driver/mysql.clj b/src/metabase/driver/mysql.clj
index b158d85e4b2e10a4b1e6016f6dbd92fd63189444..fb329e4ec31a508229948f5d95b402c3d5b4b2e4 100644
--- a/src/metabase/driver/mysql.clj
+++ b/src/metabase/driver/mysql.clj
@@ -102,18 +102,22 @@
(defn- warn-on-unsupported-versions [driver details]
(sql-jdbc.conn/with-connection-spec-for-testing-connection [jdbc-spec [driver details]]
- (jdbc/with-db-metadata [metadata jdbc-spec]
- (when (unsupported-version? metadata)
- (log/warn
- (u/format-color 'red
- (str
- "\n\n********************************************************************************\n"
- (trs "WARNING: Metabase only officially supports MySQL {0}/MariaDB {1} and above."
- min-supported-mysql-version
- min-supported-mariadb-version)
- "\n"
- (trs "All Metabase features may not work properly when using an unsupported version.")
- "\n********************************************************************************\n")))))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ jdbc-spec
+ nil
+ (fn [^java.sql.Connection conn]
+ (when (unsupported-version? (.getMetaData conn))
+ (log/warn
+ (u/format-color 'red
+ (str
+ "\n\n********************************************************************************\n"
+ (trs "WARNING: Metabase only officially supports MySQL {0}/MariaDB {1} and above."
+ min-supported-mysql-version
+ min-supported-mariadb-version)
+ "\n"
+ (trs "All Metabase features may not work properly when using an unsupported version.")
+ "\n********************************************************************************\n"))))))))
(defmethod driver/can-connect? :mysql
[driver details]
diff --git a/src/metabase/driver/mysql/actions.clj b/src/metabase/driver/mysql/actions.clj
index 6b1b1111f79f107fd539a201ab667a3ad8144ef0..ec08c89ba3910c8fdeb02c0a28f3262a95bdaaf3 100644
--- a/src/metabase/driver/mysql/actions.clj
+++ b/src/metabase/driver/mysql/actions.clj
@@ -5,6 +5,7 @@
[clojure.string :as str]
[metabase.driver.sql-jdbc.actions :as sql-jdbc.actions]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.driver.sql.query-processor :as sql.qp]
[metabase.util :as u]
[metabase.util.i18n :refer [tru]]
@@ -133,13 +134,20 @@
[_driver _conn thunk]
(thunk))
-(defn- get-primary-keys [db-spec table-components]
+(defn- primary-keys [driver jdbc-spec table-components]
(let [schema (when (next table-components) (first table-components))
- table (last table-components)]
- (jdbc/with-db-metadata [md db-spec]
- (->> (.getPrimaryKeys md nil schema table)
- jdbc/metadata-result
- (mapv :column_name)))))
+ table (last table-components)]
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ jdbc-spec
+ nil
+ (fn [^java.sql.Connection conn]
+ (let [metadata (.getMetaData conn)]
+ (with-open [rset (.getPrimaryKeys metadata nil schema table)]
+ (loop [acc []]
+ (if-not (.next rset)
+ acc
+ (recur (conj acc (.getString rset "COLUMN_NAME")))))))))))
;;; MySQL returns the generated ID (of which cannot be more than one)
;;; as insert_id. If this is not null, we determine the name of the
@@ -151,9 +159,9 @@
;;; warning and return nil.
(defmethod sql-jdbc.actions/select-created-row :mysql
[driver create-hsql conn {:keys [insert_id] :as results}]
- (let [db-spec {:connection conn}
+ (let [jdbc-spec {:connection conn}
table-components (-> create-hsql :insert-into :components)
- pks (get-primary-keys db-spec table-components)
+ pks (primary-keys driver jdbc-spec table-components)
where-clause (if insert_id
[:= (-> pks first keyword) insert_id]
(into [:and]
@@ -165,7 +173,7 @@
:from [(:insert-into create-hsql)]
:where where-clause))
select-sql-args (sql.qp/format-honeysql driver select-hsql)
- query-results (jdbc/query db-spec
+ query-results (jdbc/query jdbc-spec
select-sql-args
{:identifiers identity, :transaction? false})]
(if (next query-results)
diff --git a/src/metabase/driver/mysql/ddl.clj b/src/metabase/driver/mysql/ddl.clj
index be38b455cf3211ff17d0ebee09279b549cf008c1..0eeb7472cf81697d50eac2588d65b08e09150264 100644
--- a/src/metabase/driver/mysql/ddl.clj
+++ b/src/metabase/driver/mysql/ddl.clj
@@ -1,12 +1,12 @@
(ns metabase.driver.mysql.ddl
(:require
[clojure.core.async :as a]
- [clojure.java.jdbc :as jdbc]
[clojure.string :as str]
[honey.sql :as sql]
[java-time :as t]
[metabase.driver.ddl.interface :as ddl.i]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.driver.sql.ddl :as sql.ddl]
[metabase.public-settings :as public-settings]
[metabase.query-processor :as qp]
@@ -17,21 +17,25 @@
(set! *warn-on-reflection* true)
-(defn- exec-async [conn-chan db-spec sql+params]
+(defn- exec-async [driver conn-chan db-spec sql+params]
(a/thread
- (jdbc/with-db-connection [conn db-spec]
- (try
- (let [pid (:pid (first (sql.ddl/jdbc-query conn ["select connection_id() pid"])))]
- (a/put! conn-chan pid)
- (sql.ddl/jdbc-query conn sql+params))
- (catch SQLNonTransientConnectionException _e
- ;; Our connection may be killed due to timeout, `kill` will throw an appropriate exception
- nil)
- (catch Exception e
- (log/warn e)
- e)))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ db-spec
+ {:write? true}
+ (fn [^java.sql.Connection conn]
+ (try
+ (let [pid (:pid (first (sql.ddl/jdbc-query conn ["select connection_id() pid"])))]
+ (a/put! conn-chan pid)
+ (sql.ddl/jdbc-query conn sql+params))
+ (catch SQLNonTransientConnectionException _e
+ ;; Our connection may be killed due to timeout, [[kill!]] will throw an appropriate exception
+ nil)
+ (catch Exception e
+ (log/warn e)
+ e))))))
-(defn- kill [conn pid]
+(defn- kill! [conn pid]
(let [results (sql.ddl/jdbc-query conn ["show processlist"])
result? (some (fn [r]
(and (= (:id r) pid)
@@ -46,43 +50,54 @@
"Spins up another channel to execute the statement.
If `timeout-ms` passes, send a kill statement to stop execution and throw exception
Otherwise return results returned by channel."
- [conn db-spec timeout-ms sql+params]
- (let [conn-chan (a/chan)
- exec-chan (exec-async conn-chan db-spec sql+params)
+ [driver conn db-spec timeout-ms sql+params]
+ (let [conn-chan (a/promise-chan)
+ exec-chan (exec-async driver conn-chan db-spec sql+params)
pid (a/<!! conn-chan)
+ _ (a/close! conn-chan)
timeout-chan (a/timeout timeout-ms)
- [v port] (a/alts!! [timeout-chan exec-chan])]
+ [v port] (a/alts!! [timeout-chan exec-chan])]
+ (a/close! exec-chan)
(cond
- (= port timeout-chan) (kill conn pid)
+ (= port timeout-chan) (kill! conn pid)
+ (= port exec-chan) (if (instance? Exception v)
+ (throw v)
+ v))))
- (= port exec-chan) (if (instance? Exception v)
- (throw v)
- v))))
-
-(defmethod ddl.i/refresh! :mysql [_driver database definition dataset-query]
+(defmethod ddl.i/refresh! :mysql
+ [driver database definition dataset-query]
(let [{:keys [query params]} (qp/compile dataset-query)
db-spec (sql-jdbc.conn/db->pooled-connection-spec database)]
- (jdbc/with-db-connection [conn db-spec]
- (sql.ddl/execute! conn [(sql.ddl/drop-table-sql database (:table-name definition))])
- ;; It is possible that this fails and rollback would not restore the table.
- ;; That is ok, the persisted-info will be marked inactive and the next refresh will try again.
- (execute-with-timeout! conn
- db-spec
- (.toMillis (t/minutes 10))
- (into [(sql.ddl/create-table-sql database definition query)] params))
- {:state :success})))
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ database
+ {:write? true}
+ (fn [conn]
+ (sql.ddl/execute! conn [(sql.ddl/drop-table-sql database (:table-name definition))])
+ ;; It is possible that this fails and rollback would not restore the table.
+ ;; That is ok, the persisted-info will be marked inactive and the next refresh will try again.
+ (execute-with-timeout! driver
+ conn
+ db-spec
+ (.toMillis (t/minutes 10))
+ (into [(sql.ddl/create-table-sql database definition query)] params))
+ {:state :success}))))
(defmethod ddl.i/unpersist! :mysql
- [_driver database persisted-info]
- (jdbc/with-db-connection [conn (sql-jdbc.conn/db->pooled-connection-spec database)]
- (try
- (sql.ddl/execute! conn [(sql.ddl/drop-table-sql database (:table_name persisted-info))])
- (catch Exception e
- (log/warn e)
- (throw e)))))
+ [driver database persisted-info]
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ database
+ {:write? true}
+ (fn [^java.sql.Connection conn]
+ (try
+ (sql.ddl/execute! conn [(sql.ddl/drop-table-sql database (:table_name persisted-info))])
+ (catch Exception e
+ (log/warn e)
+ (throw e))))))
(defmethod ddl.i/check-can-persist :mysql
- [database]
+ [{driver :engine, :as database}]
(let [schema-name (ddl.i/schema-name database (public-settings/site-uuid))
table-name (format "persistence_check_%s" (rand-int 10000))
db-spec (sql-jdbc.conn/db->pooled-connection-spec database)
@@ -98,7 +113,8 @@
(sql.ddl/execute! conn [(sql.ddl/drop-schema-sql database)]))]
[:persist.check/create-table
(fn create-table [conn]
- (execute-with-timeout! conn
+ (execute-with-timeout! driver
+ conn
db-spec
(.toMillis (t/minutes 10))
[(sql.ddl/create-table-sql
@@ -134,28 +150,32 @@
{:dialect :mysql})))]]]
;; Unlike postgres, mysql ddl clauses will not rollback in a transaction.
;; So we keep track of undo-steps to manually rollback previous, completed steps.
- (jdbc/with-db-connection [conn db-spec]
- (loop [[[step stepfn undofn] & remaining] steps
- undo-steps []]
- (let [result (try (stepfn conn)
- (log/info (trs "Step {0} was successful for db {1}"
- step (:name database)))
- ::valid
- (catch Exception e
- (log/warn (trs "Error in `{0}` while checking for model persistence permissions." step))
- (log/warn e)
- (try
- (doseq [[undo-step undofn] (reverse undo-steps)]
- (log/warn (trs "Undoing step `{0}` for db {1}" undo-step (:name database)))
- (undofn conn))
- (catch Exception _e
- (log/warn (trs "Unable to rollback database check for model persistence"))))
- step))]
- (cond (and (= result ::valid) remaining)
- (recur remaining (conj undo-steps [step undofn]))
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ db-spec
+ {:write? true}
+ (fn [conn]
+ (loop [[[step stepfn undofn] & remaining] steps
+ undo-steps []]
+ (let [result (try (stepfn conn)
+ (log/info (trs "Step {0} was successful for db {1}"
+ step (:name database)))
+ ::valid
+ (catch Exception e
+ (log/warn (trs "Error in `{0}` while checking for model persistence permissions." step))
+ (log/warn e)
+ (try
+ (doseq [[undo-step undofn] (reverse undo-steps)]
+ (log/warn (trs "Undoing step `{0}` for db {1}" undo-step (:name database)))
+ (undofn conn))
+ (catch Exception _e
+ (log/warn (trs "Unable to rollback database check for model persistence"))))
+ step))]
+ (cond (and (= result ::valid) remaining)
+ (recur remaining (conj undo-steps [step undofn]))
- (= result ::valid)
- [true :persist.check/valid]
+ (= result ::valid)
+ [true :persist.check/valid]
- :else
- [false step]))))))
+ :else
+ [false step])))))))
diff --git a/src/metabase/driver/postgres.clj b/src/metabase/driver/postgres.clj
index 0b5c1db1cf92e65d22081fffacf863ff6e1f1301..35c03b22523bf6ae4c7f292bdb866d8c8b62a57e 100644
--- a/src/metabase/driver/postgres.clj
+++ b/src/metabase/driver/postgres.clj
@@ -620,7 +620,7 @@
(default-base-types column)))
(defmethod sql-jdbc.sync/column->semantic-type :postgres
- [_ database-type _]
+ [_driver database-type _column-name]
;; this is really, really simple right now. if its postgres :json type then it's :type/SerializedJSON semantic-type
(case database-type
"json" :type/SerializedJSON
@@ -807,15 +807,19 @@
(defmethod driver/insert-into :postgres
[driver db-id table-name column-names values]
- (jdbc/with-db-connection [conn (sql-jdbc.conn/db->pooled-connection-spec db-id)]
- (let [copy-manager (CopyManager. (.unwrap (jdbc/get-connection conn) PgConnection))
- [sql & _] (sql/format {::copy (keyword table-name)
- :columns (map keyword column-names)
- ::from-stdin "''"}
- :quoted true
- :dialect (sql.qp/quote-style driver))
- tsvs (->> values
- (map row->tsv)
- (str/join "\n")
- (StringReader.))]
- (.copyIn copy-manager ^String sql tsvs))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ db-id
+ {:write? true}
+ (fn [^java.sql.Connection conn]
+ (let [copy-manager (CopyManager. (.unwrap conn PgConnection))
+ [sql & _] (sql/format {::copy (keyword table-name)
+ :columns (map keyword column-names)
+ ::from-stdin "''"}
+ :quoted true
+ :dialect (sql.qp/quote-style driver))
+ tsvs (->> values
+ (map row->tsv)
+ (str/join "\n")
+ (StringReader.))]
+ (.copyIn copy-manager ^String sql tsvs)))))
diff --git a/src/metabase/driver/postgres/ddl.clj b/src/metabase/driver/postgres/ddl.clj
index 596a55440b9755de49be5200abd11c2ca7840d58..8f0d10169cecd3a5b59ba1b38c87fd887cee142b 100644
--- a/src/metabase/driver/postgres/ddl.clj
+++ b/src/metabase/driver/postgres/ddl.clj
@@ -4,7 +4,7 @@
[honey.sql :as sql]
[java-time :as t]
[metabase.driver.ddl.interface :as ddl.i]
- [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.driver.sql.ddl :as sql.ddl]
[metabase.public-settings :as public-settings]
[metabase.query-processor :as qp]
@@ -36,26 +36,35 @@
;; Can't use a prepared parameter with these statements
(sql.ddl/execute! tx [(format "SET LOCAL statement_timeout TO '%s'" (str new-timeout))])))
-(defmethod ddl.i/refresh! :postgres [_driver database definition dataset-query]
+(defmethod ddl.i/refresh! :postgres
+ [driver database definition dataset-query]
(let [{:keys [query params]} (qp/compile dataset-query)]
- (jdbc/with-db-connection [conn (sql-jdbc.conn/db->pooled-connection-spec database)]
- (jdbc/with-db-transaction [tx conn]
- (set-statement-timeout! tx)
- (sql.ddl/execute! tx [(sql.ddl/drop-table-sql database (:table-name definition))])
- (sql.ddl/execute! tx (into [(sql.ddl/create-table-sql database definition query)] params)))
- {:state :success})))
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ database
+ {:write? true}
+ (fn [^java.sql.Connection conn]
+ (jdbc/with-db-transaction [tx {:connection conn}]
+ (set-statement-timeout! tx)
+ (sql.ddl/execute! tx [(sql.ddl/drop-table-sql database (:table-name definition))])
+ (sql.ddl/execute! tx (into [(sql.ddl/create-table-sql database definition query)] params)))
+ {:state :success}))))
(defmethod ddl.i/unpersist! :postgres
- [_driver database persisted-info]
- (jdbc/with-db-connection [conn (sql-jdbc.conn/db->pooled-connection-spec database)]
- (try
- (sql.ddl/execute! conn [(sql.ddl/drop-table-sql database (:table_name persisted-info))])
- (catch Exception e
- (log/warn e)
- (throw e)))))
+ [driver database persisted-info]
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ database
+ {:write? true}
+ (fn [conn]
+ (try
+ (sql.ddl/execute! conn [(sql.ddl/drop-table-sql database (:table_name persisted-info))])
+ (catch Exception e
+ (log/warn e)
+ (throw e))))))
(defmethod ddl.i/check-can-persist :postgres
- [database]
+ [{driver :engine, :as database}]
(let [schema-name (ddl.i/schema-name database (public-settings/site-uuid))
table-name (format "persistence_check_%s" (rand-int 10000))
steps [[:persist.check/create-schema
@@ -93,23 +102,27 @@
(ddl.i/populate-kv-table-honey-sql-form
schema-name)
{:dialect :ansi})))]]]
- (jdbc/with-db-connection [conn (sql-jdbc.conn/db->pooled-connection-spec database)]
- (jdbc/with-db-transaction
- [tx conn]
- (set-statement-timeout! tx)
- (loop [[[step stepfn] & remaining] steps]
- (let [result (try (stepfn tx)
- (log/info (trs "Step {0} was successful for db {1}"
- step (:name database)))
- ::valid
- (catch Exception e
- (log/warn (trs "Error in `{0}` while checking for model persistence permissions." step))
- (log/warn e)
- step))]
- (cond (and (= result ::valid) remaining)
- (recur remaining)
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ database
+ {:write? true}
+ (fn [^java.sql.Connection conn]
+ (jdbc/with-db-transaction
+ [tx {:connection conn}]
+ (set-statement-timeout! tx)
+ (loop [[[step stepfn] & remaining] steps]
+ (let [result (try (stepfn tx)
+ (log/info (trs "Step {0} was successful for db {1}"
+ step (:name database)))
+ ::valid
+ (catch Exception e
+ (log/warn (trs "Error in `{0}` while checking for model persistence permissions." step))
+ (log/warn e)
+ step))]
+ (cond (and (= result ::valid) remaining)
+ (recur remaining)
- (= result ::valid)
- [true :persist.check/valid]
+ (= result ::valid)
+ [true :persist.check/valid]
- :else [false step])))))))
+ :else [false step]))))))))
diff --git a/src/metabase/driver/sql/ddl.clj b/src/metabase/driver/sql/ddl.clj
index 66b6ac090e9b26c666bdc41f50703f8f7c34336f..61604d25b4b474e600d87506437689b6308d7ec2 100644
--- a/src/metabase/driver/sql/ddl.clj
+++ b/src/metabase/driver/sql/ddl.clj
@@ -13,15 +13,23 @@
(str "-- Metabase\n"
sql-str))
+(defn- jdbc-spec [connection-or-spec]
+ (cond
+ (instance? java.sql.Connection connection-or-spec) {:connection connection-or-spec}
+ (map? connection-or-spec) connection-or-spec
+ :else (throw (ex-info "Invalid JDBC connection spec" {:spec connection-or-spec}))))
+
+;;; TODO -- move the JDBC stuff to something like [[metabase.driver.sql-jdbc.ddl]]. JDBC-specific stuff does not belong
+;;; IN [[metabase.driver.sql]] !!
(defn execute!
"Executes sql and params with a standard remark prepended to the statement."
- [conn [sql & params]]
- (jdbc/execute! conn (into [(add-remark sql)] params)))
+ [connection-or-spec [sql & params]]
+ (jdbc/execute! (jdbc-spec connection-or-spec) (into [(add-remark sql)] params)))
(defn jdbc-query
"Queries sql and params with a standard remark prepended to the statement."
- [conn [sql & params]]
- (jdbc/query conn (into [(add-remark sql)] params)))
+ [connection-or-spec [sql & params]]
+ (jdbc/query (jdbc-spec connection-or-spec) (into [(add-remark sql)] params)))
(defn create-schema-sql
"SQL string to create a schema suitable"
diff --git a/src/metabase/driver/sql_jdbc.clj b/src/metabase/driver/sql_jdbc.clj
index c5c51ec987775ce2dc4e68533d4aa4984d887fd2..7eb27c8d9afc9b7529bbac2ad0cda9f7d3f05300 100644
--- a/src/metabase/driver/sql_jdbc.clj
+++ b/src/metabase/driver/sql_jdbc.clj
@@ -145,7 +145,11 @@
(defmethod driver/syncable-schemas :sql-jdbc
[driver database]
- (with-open [conn ^java.sql.Connection (jdbc/get-connection (sql-jdbc.conn/db->pooled-connection-spec database))]
- (let [[inclusion-patterns
- exclusion-patterns] (driver.s/db-details->schema-filter-patterns database)]
- (into #{} (sql-jdbc.sync.interface/filtered-syncable-schemas driver conn (.getMetaData conn) inclusion-patterns exclusion-patterns)))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ database
+ nil
+ (fn [^java.sql.Connection conn]
+ (let [[inclusion-patterns
+ exclusion-patterns] (driver.s/db-details->schema-filter-patterns database)]
+ (into #{} (sql-jdbc.sync.interface/filtered-syncable-schemas driver conn (.getMetaData conn) inclusion-patterns exclusion-patterns))))))
diff --git a/src/metabase/driver/sql_jdbc/actions.clj b/src/metabase/driver/sql_jdbc/actions.clj
index f8b7de6c2e3a36f7e694f6958e9355dd40e70282..db5b0e8977055248015b269cc3c5c71f87d16cd0 100644
--- a/src/metabase/driver/sql_jdbc/actions.clj
+++ b/src/metabase/driver/sql_jdbc/actions.clj
@@ -8,7 +8,6 @@
[metabase.actions :as actions]
[metabase.db.util :as mdb.u]
[metabase.driver :as driver]
- [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
[metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.driver.sql.query-processor :as sql.qp]
[metabase.driver.util :as driver.u]
@@ -117,25 +116,27 @@
[database-id f]
(if *connection*
(f *connection*)
- (let [jdbc-spec (sql-jdbc.conn/db->pooled-connection-spec database-id)]
- (with-open [conn (jdbc/get-connection jdbc-spec)]
- ;; execute inside of a transaction.
- (.setAutoCommit conn false)
- (log/tracef "BEGIN transaction on conn %s@0x%s" (.getCanonicalName (class conn)) (System/identityHashCode conn))
- ;; use the strictest transaction isolation level possible to avoid dirty, non-repeatable, and phantom reads
- (sql-jdbc.execute/set-best-transaction-level! (driver.u/database->driver database-id) conn)
- (try
- (let [result (binding [*connection* conn]
- (f conn))]
- (log/debug "f completed successfully; committing transaction.")
- (log/tracef "COMMIT transaction on conn %s@0x%s" (.getCanonicalName (class conn)) (System/identityHashCode conn))
- (.commit conn)
- result)
- (catch Throwable e
- (log/debugf "f threw Exception; rolling back transaction. Error: %s" (ex-message e))
- (log/tracef "ROLLBACK transaction on conn %s@0x%s" (.getCanonicalName (class conn)) (System/identityHashCode conn))
- (.rollback conn)
- (throw e)))))))
+ (let [driver (driver.u/database->driver database-id)]
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ database-id
+ {:write? true}
+ (fn [^Connection conn]
+ ;; execute inside of a transaction.
+ (.setAutoCommit conn false)
+ (log/tracef "BEGIN transaction on conn %s@0x%s" (.getCanonicalName (class conn)) (System/identityHashCode conn))
+ (try
+ (let [result (binding [*connection* conn]
+ (f conn))]
+ (log/debug "f completed successfully; committing transaction.")
+ (log/tracef "COMMIT transaction on conn %s@0x%s" (.getCanonicalName (class conn)) (System/identityHashCode conn))
+ (.commit conn)
+ result)
+ (catch Throwable e
+ (log/debugf "f threw Exception; rolling back transaction. Error: %s" (ex-message e))
+ (log/tracef "ROLLBACK transaction on conn %s@0x%s" (.getCanonicalName (class conn)) (System/identityHashCode conn))
+ (.rollback conn)
+ (throw e))))))))
(defmacro with-jdbc-transaction
"Execute `f` with a JDBC Connection for the Database with `database-id`. Uses [[*connection*]] if already bound,
@@ -144,7 +145,6 @@
[[connection-binding database-id] & body]
`(do-with-jdbc-transaction ~database-id (fn [~(vary-meta connection-binding assoc :tag 'Connection)] ~@body)))
-
(defmulti prepare-query*
"Multimethod for preparing a honeysql query `hsql-query` for a given action type `action`.
`action` is a keyword like `:row/create` or `:bulk/create`; `hsql-query` is a generic
diff --git a/src/metabase/driver/sql_jdbc/execute.clj b/src/metabase/driver/sql_jdbc/execute.clj
index 1148ad83425b5f59a9bf7a60ece0f0c558abcbda..a580a12ec7c93a42755d4cb24de4e1aac809d7cd 100644
--- a/src/metabase/driver/sql_jdbc/execute.clj
+++ b/src/metabase/driver/sql_jdbc/execute.clj
@@ -6,6 +6,7 @@
`metabase.driver.sql-jdbc.execute.legacy-impl`. "
(:require
[clojure.core.async :as a]
+ [clojure.java.jdbc :as jdbc]
[clojure.string :as str]
[java-time :as t]
[metabase.db.query :as mdb.query]
@@ -15,6 +16,10 @@
:as sql-jdbc.execute.diagnostic]
[metabase.driver.sql-jdbc.execute.old-impl :as sql-jdbc.execute.old]
[metabase.driver.sql-jdbc.sync.interface :as sql-jdbc.sync.interface]
+ [metabase.lib.schema.expression.temporal
+ :as lib.schema.expression.temporal]
+ [metabase.lib.schema.literal.jvm :as lib.schema.literal.jvm]
+ [metabase.models.database :refer [Database]]
[metabase.models.setting :refer [defsetting]]
[metabase.query-processor.context :as qp.context]
[metabase.query-processor.error-type :as qp.error-type]
@@ -26,7 +31,9 @@
[metabase.util :as u]
[metabase.util.i18n :refer [trs tru]]
[metabase.util.log :as log]
- [potemkin :as p])
+ [metabase.util.malli :as mu]
+ [potemkin :as p]
+ [toucan2.core :as t2])
(:import
(java.sql Connection JDBCType PreparedStatement ResultSet ResultSetMetaData Statement Types)
(java.time Instant LocalDate LocalDateTime LocalTime OffsetDateTime OffsetTime ZonedDateTime)
@@ -38,27 +45,72 @@
;;; | SQL JDBC Reducible QP Interface |
;;; +----------------------------------------------------------------------------------------------------------------+
-(defmulti connection-with-timezone
- "Fetch a Connection for a `database` with session time zone set to `timezone-id` (if supported by the driver.) The
- default implementation:
+(def ConnectionOptions
+ "Malli schema for the options passed to [[do-with-connection-with-options]]."
+ [:maybe
+ [:map
+ ;; a string like 'US/Pacific' or something like that.
+ [:session-timezone {:optional true} [:maybe [:ref ::lib.schema.expression.temporal/timezone-id]]]
+ ;; whether this Connection should NOT be read-only, e.g. for DDL stuff or inserting data or whatever.
+ [:write? {:optional true} [:maybe :boolean]]]])
- 1. Calls util fn `datasource` to get a c3p0 connection pool DataSource
- 2. Calls `.getConnection()` the normal way
- 3. Executes [[set-timezone-sql]] if implemented by the driver.
+(defmulti do-with-connection-with-options
+ "Fetch a [[java.sql.Connection]] from a `driver`/`db-or-id-or-spec`, and invoke
- `timezone-id` will be `nil` if a `report-timezone` Setting is not currently set; don't change the session time zone
- if this is the case.
+ (f connection)
- For drivers that support session timezones, the default implementation and [[set-timezone-sql]] should be considered
- deprecated in favor of implementing `connection-with-timezone` directly. This way you can set the session timezone
- in the most efficient manner, e.g. only setting it if needed (if there's an easy way for you to check this), or by
- setting it as a parameter of the connection itself (the default connection pools are automatically flushed when
- `report-timezone-id` changes).
+ If `db-or-id-or-spec` is a Database or Database ID, the default implementation fetches a pooled connection spec for
+ that Database using [[datasource]].
- Custom implementations should set transaction isolation to the least-locking level supported by the driver, and make
- connections read-only (*after* setting timezone, if needed)."
- {:added "0.35.0"
- :arglists '(^java.sql.Connection [driver database ^String timezone-id])}
+ If `db-or-id-or-spec` is a `clojure.java.jdbc` spec, it fetches a Connection
+ using [[clojure.java..jdbc/get-connection]]. Note that this will not be a pooled connection unless your spec is for
+ a pooled DataSource.
+
+ `options` matches the [[ConnectionOptions]] schema above.
+
+ * If `:session-timezone` is passed, it should be used to set the Session timezone for the Connection. If not passed,
+ leave as-is
+
+ * If `:write?` is NOT passed or otherwise falsey, make the connection read-only if possible; if it is truthy, make
+ the connection read-write. Note that this current does not run things inside a transaction automatically; you'll
+ have to do that yourself if you want it
+
+ The normal 'happy path' is more or less
+
+ (with-open [conn (.getConnection (datasource driver db-or-id-or-spec))]
+ (set-best-transaction-level! driver conn)
+ (set-time-zone-if-supported! driver conn session-timezone)
+ (.setReadOnly conn true)
+ (.setAutoCommit conn true) ; so the query(s) are not ran inside a transaction
+ (.setHoldability conn ResultSet/CLOSE_CURSORS_AT_COMMIT)
+ (f conn))
+
+ This default implementation is abstracted out into two functions, [[do-with-resolved-connection]]
+ and [[set-default-connection-options!]], that you can use as needed in custom implementations. See various driver
+ implementations for examples. You should only set connection options on top-level calls
+ to [[do-with-connection-with-options]]; check whether this is a [[recursive-connection?]] before setting options.
+
+ There are two usual ways to set the session timezone if your driver supports them:
+
+ 1. Specifying the session timezone based on the value of [[metabase.driver/report-timezone]] as a JDBC connection
+ parameter in the JDBC connection spec returned by [[metabase.driver.sql-jdbc.connection/connection-details->spec]].
+ If the spec returned by this method changes, connection pools associated with it will be flushed automatically.
+ This is the preferred way to set session timezones; if you set them this way, you DO NOT need to implement this
+ method unless you need to do something special with regards to setting the transaction level.
+
+ 2. Setting the session timezone manually on the [[java.sql.Connection]] returned by [[datasource]] based on the
+ value of `session-timezone`.
+
+ 2a. The default implementation will do this for you by executing SQL if you implement
+ [[set-timezone-sql]].
+
+ 2b. You can implement this method, [[do-with-connection-with-options]], yourself and set the timezone however you
+ wish. Only set it if `session-timezone` is not `nil`!
+
+ Custom implementations should set transaction isolation to the least-locking level supported by the driver, and make
+ connections read-only (*after* setting timezone, if needed)."
+ {:added "0.47.0"
+ :arglists '([driver db-or-id-or-spec options f])}
driver/dispatch-on-initialized-driver
:hierarchy #'driver/hierarchy)
@@ -71,6 +123,10 @@
[(driver/dispatch-on-initialized-driver driver) (class object)])
:hierarchy #'driver/hierarchy)
+;; TODO -- maybe like [[do-with-connection-with-options]] we should replace [[prepared-statment]] and [[statement]]
+;; with `do-with-prepared-statement` and `do-with-statement` methods -- that way you can't accidentally forget to wrap
+;; things in a `try-catch` and call `.close`
+
(defmulti ^PreparedStatement prepared-statement
"Create a PreparedStatement with `sql` query, and set any `params`. You shouldn't need to override the default
implementation for this method; if you do, take care to set options to maximize result set read performance (e.g.
@@ -135,23 +191,23 @@
;;; +----------------------------------------------------------------------------------------------------------------+
(defn datasource
- "Fetch the connection pool `DataSource` associated with `database`."
+ "Fetch the connection pool `DataSource` associated with `db-or-id-or-spec`."
{:added "0.35.0"}
- ^DataSource [database]
- (:datasource (sql-jdbc.conn/db->pooled-connection-spec database)))
+ ^DataSource [db-or-id-or-spec]
+ (:datasource (sql-jdbc.conn/db->pooled-connection-spec db-or-id-or-spec)))
(defn datasource-with-diagnostic-info!
"Fetch the connection pool `DataSource` associated with `database`, while also recording diagnostic info for the
pool. To be used in conjunction with `sql-jdbc.execute.diagnostic/capturing-diagnostic-info`."
{:added "0.40.0"}
- ^DataSource [driver database]
- (let [ds (datasource database)]
- (sql-jdbc.execute.diagnostic/record-diagnostic-info-for-pool driver (u/the-id database) ds)
+ ^DataSource [driver db-or-id]
+ (let [ds (datasource db-or-id)]
+ (sql-jdbc.execute.diagnostic/record-diagnostic-info-for-pool! driver (u/the-id db-or-id) ds)
ds))
(defn set-time-zone-if-supported!
"Execute `set-timezone-sql`, if implemented by driver, to set the session time zone. This way of setting the time zone
- should be considered deprecated in favor of implementing `connection-with-time-zone` directly."
+ should be considered deprecated in favor of implementing `connection-with-timezone` directly."
{:deprecated "0.35.0"}
[driver ^Connection conn ^String timezone-id]
(when timezone-id
@@ -191,33 +247,132 @@
(seq more)
(recur more)))))
-(defmethod connection-with-timezone :sql-jdbc
- [driver database ^String timezone-id]
- (let [conn (.getConnection (datasource-with-diagnostic-info! driver database))]
- (try
- (set-best-transaction-level! driver conn)
- (set-time-zone-if-supported! driver conn timezone-id)
+(mu/defn do-with-resolved-connection-data-source :- (lib.schema.literal.jvm/instance-of DataSource)
+ "Part of the default implementation for [[do-with-connection-with-options]]: get an appropriate `java.sql.DataSource`
+ for `db-or-id-or-spec`. Not for use with a JDBC spec wrapping a `java.sql.Connection` (a spec with the key
+ `:connection`), since we do not have control over its lifecycle and would thus not be able to use [[with-open]] with
+ Connections provided by this DataSource."
+ {:added "0.47.0", :arglists '(^javax.sql.DataSource [driver db-or-id-or-spec options])}
+ [driver :- :keyword
+ db-or-id-or-spec :- [:and
+ [:or :int :map]
+ [:fn
+ ;; can't wrap a java.sql.Connection here because we're not
+ ;; responsible for its lifecycle and that means you can't use
+ ;; `with-open` on the Connection you'd get from the DataSource
+ {:error/message "Cannot be a JDBC spec wrapping a java.sql.Connection"}
+ (complement :connection)]]
+ {:keys [^String session-timezone], :as _options} :- ConnectionOptions]
+ (if-not (u/id db-or-id-or-spec)
+ ;; not a Database or Database ID... this is a raw `clojure.java.jdbc` spec, use that
+ ;; directly.
+ (reify DataSource
+ (getConnection [_this]
+ #_{:clj-kondo/ignore [:discouraged-var]}
+ (jdbc/get-connection db-or-id-or-spec)))
+ ;; otherwise this is either a Database or Database ID.
+ (if-let [old-method-impl (get-method
+ #_{:clj-kondo/ignore [:deprecated-var]} sql-jdbc.execute.old/connection-with-timezone
+ driver)]
+ ;; use the deprecated impl for `connection-with-timezone` if one exists.
+ (do
+ (log/warn (trs "{0} is deprecated in Metabase 0.47.0. Implement {1} instead."
+ #_{:clj-kondo/ignore [:deprecated-var]}
+ `connection-with-timezone
+ `do-with-connection-with-options))
+ ;; for compatibility, make sure we pass it an actual Database instance.
+ (let [database (if (integer? db-or-id-or-spec)
+ (t2/select-one Database db-or-id-or-spec)
+ db-or-id-or-spec)]
+ (reify DataSource
+ (getConnection [_this]
+ (old-method-impl driver database session-timezone)))))
+ (datasource-with-diagnostic-info! driver db-or-id-or-spec))))
+
+(def ^:private ^:dynamic ^{:added "0.47.0"} *connection-recursion-depth*
+ "In recursive calls to [[do-with-connection-with-options]] we don't want to set options AGAIN, because this might
+ break things. For example in a top-level `:write?` call, we might disable auto-commit and run things in a
+ transaction; a read-only call inside of this transaction block should not go in and change the connection to be
+ auto-commit. So only set options at the top-level call, and use this to keep track of whether we're at the top level
+ or not.
+
+ This gets incremented inside [[do-with-resolved-connection]], so the top level call with have a depth of `0`, a
+ nested call will get `1`, and so forth. This is done this way and inside [[do-with-resolved-connection]]
+ and [[set-default-connection-options!]] so drivers that implement "
+ -1)
+
+(defn recursive-connection?
+ "Whether or not we are in a recursive call to [[do-with-connection-with-options]]. If we are, you shouldn't set
+ Connection options AGAIN, as that may override previous options that we don't want to override."
+ []
+ {:added "0.47.0"}
+ (pos? *connection-recursion-depth*))
+
+(mu/defn do-with-resolved-connection
+ "Execute
+
+ (f ^java.sql.Connection conn)
+
+ with a resolved JDBC connection. Part of the default implementation for [[do-with-connection-with-options]].
+ Generally does not set any `options`, but may set session-timezone if `driver` implements the
+ deprecated [[sql-jdbc.execute.old/connection-with-timezone]] method."
+ {:added "0.47.0"}
+ [driver :- :keyword
+ db-or-id-or-spec :- [:or :int :map]
+ options :- ConnectionOptions
+ f :- fn?]
+ (binding [*connection-recursion-depth* (inc *connection-recursion-depth*)]
+ (if-let [conn (:connection db-or-id-or-spec)]
+ (f conn)
+ (with-open [conn (.getConnection (do-with-resolved-connection-data-source driver db-or-id-or-spec options))]
+ (f conn)))))
+
+(mu/defn set-default-connection-options!
+ "Part of the default implementation of [[do-with-connection-with-options]]: set options for a newly fetched
+ Connection."
+ {:added "0.47.0"}
+ [driver :- :keyword
+ ^Connection conn :- (lib.schema.literal.jvm/instance-of Connection)
+ {:keys [^String session-timezone write?], :as options} :- ConnectionOptions]
+ (when-not (recursive-connection?)
+ (log/tracef "Setting default connection options with options %s" (pr-str options))
+ (set-best-transaction-level! driver conn)
+ (set-time-zone-if-supported! driver conn session-timezone)
+ (let [read-only? (not write?)]
(try
;; Setting the connection to read-only does not prevent writes on some databases, and is meant
;; to be a hint to the driver to enable database optimizations
;; See https://docs.oracle.com/javase/8/docs/api/java/sql/Connection.html#setReadOnly-boolean-
- (.setReadOnly conn true)
+ (log/trace (pr-str (list '.setReadOnly 'conn read-only?)))
+ (.setReadOnly conn read-only?)
(catch Throwable e
- (log/debug e (trs "Error setting connection to read-only"))))
+ (log/debugf e "Error setting connection readOnly to %s" (pr-str read-only?)))))
+ ;; if this is (supposedly) a read-only connection, enable auto-commit so this IS NOT ran inside of a transaction.
+ ;;
+ ;; TODO -- for `write?` connections, we should probably disable autoCommit and then manually call `.commit` at after
+ ;; `f`... we need to check and make sure that won't mess anything up, since some existing code is already doing it
+ ;; manually.
+ (when-not write?
(try
- ;; set autocommit to false so that pg honors fetchSize. Otherwise it commits the transaction and needs the
- ;; entire realized result set
- (.setAutoCommit conn false)
+ (log/trace (pr-str '(.setAutoCommit conn true)))
+ (.setAutoCommit conn true)
(catch Throwable e
- (log/debug e (trs "Error setting connection to autoCommit false"))))
- (try
- (.setHoldability conn ResultSet/CLOSE_CURSORS_AT_COMMIT)
- (catch Throwable e
- (log/debug e (trs "Error setting default holdability for connection"))))
- conn
+ (log/debug e "Error enabling connection autoCommit"))))
+ (try
+ (log/trace (pr-str '(.setHoldability conn ResultSet/CLOSE_CURSORS_AT_COMMIT)))
+ (.setHoldability conn ResultSet/CLOSE_CURSORS_AT_COMMIT)
(catch Throwable e
- (.close conn)
- (throw e)))))
+ (log/debug e (trs "Error setting default holdability for connection"))))))
+
+(defmethod do-with-connection-with-options :sql-jdbc
+ [driver db-or-id-or-spec options f]
+ (do-with-resolved-connection
+ driver
+ db-or-id-or-spec
+ options
+ (fn [^Connection conn]
+ (set-default-connection-options! driver conn options)
+ (f conn))))
;; TODO - would a more general method to convert a parameter to the desired class (and maybe JDBC type) be more
;; useful? Then we can actually do things like log what transformations are taking place
@@ -497,27 +652,54 @@
(execute-reducible-query driver sql params max-rows context respond)))
([driver sql params max-rows context respond]
- (with-open [conn (connection-with-timezone driver (qp.store/database) (qp.timezone/report-timezone-id-if-supported))
- stmt (statement-or-prepared-statement driver conn sql params (qp.context/canceled-chan context))
- ^ResultSet rs (try
- (execute-statement-or-prepared-statement! driver stmt max-rows params sql)
- (catch Throwable e
- (throw (ex-info (tru "Error executing query: {0}" (ex-message e))
- {:driver driver
- :sql (str/split-lines (mdb.query/format-sql sql driver))
- :params params
- :type qp.error-type/invalid-query}
- e))))]
- (let [rsmeta (.getMetaData rs)
- results-metadata {:cols (column-metadata driver rsmeta)}]
- (respond results-metadata (reducible-rows driver rs rsmeta (qp.context/canceled-chan context)))))))
+ (do-with-connection-with-options
+ driver
+ (qp.store/database)
+ {:session-timezone (qp.timezone/report-timezone-id-if-supported driver (qp.store/database))}
+ (fn [^Connection conn]
+ (with-open [stmt (statement-or-prepared-statement driver conn sql params (qp.context/canceled-chan context))
+ ^ResultSet rs (try
+ (execute-statement-or-prepared-statement! driver stmt max-rows params sql)
+ (catch Throwable e
+ (throw (ex-info (tru "Error executing query: {0}" (ex-message e))
+ {:driver driver
+ :sql (str/split-lines (mdb.query/format-sql sql driver))
+ :params params
+ :type qp.error-type/invalid-query}
+ e))))]
+ (let [rsmeta (.getMetaData rs)
+ results-metadata {:cols (column-metadata driver rsmeta)}]
+ (respond results-metadata (reducible-rows driver rs rsmeta (qp.context/canceled-chan context)))))))))
+
+
+;;; +----------------------------------------------------------------------------------------------------------------+
+;;; | Actions Stuff |
+;;; +----------------------------------------------------------------------------------------------------------------+
+
+(defmethod driver/execute-write-query! :sql-jdbc
+ [driver {{sql :query, :keys [params]} :native}]
+ {:pre [(string? sql)]}
+ (try
+ (do-with-connection-with-options
+ driver
+ {:session-timezone (qp.timezone/report-timezone-id-if-supported driver (qp.store/database))}
+ (fn [^Connection conn]
+ (with-open [stmt (statement-or-prepared-statement driver conn sql params nil)]
+ {:rows-affected (if (instance? PreparedStatement stmt)
+ (.executeUpdate ^PreparedStatement stmt)
+ (.executeUpdate stmt sql))})))
+ (catch Throwable e
+ (throw (ex-info (tru "Error executing write query: {0}" (ex-message e))
+ {:sql sql, :params params, :type qp.error-type/invalid-query}
+ e)))))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Convenience Imports from Old Impl |
;;; +----------------------------------------------------------------------------------------------------------------+
+#_{:clj-kondo/ignore [:deprecated-var]}
(p/import-vars
[sql-jdbc.execute.old
- ;; interface (set-parameter is imported as well at the top of the namespace)
+ connection-with-timezone
set-timezone-sql])
diff --git a/src/metabase/driver/sql_jdbc/execute/diagnostic.clj b/src/metabase/driver/sql_jdbc/execute/diagnostic.clj
index d1f5618a31a5887ac48b4ad8b25538078dbc6db7..1cb8649e4885a2798c9ed29fd21f813d4fc74f17 100644
--- a/src/metabase/driver/sql_jdbc/execute/diagnostic.clj
+++ b/src/metabase/driver/sql_jdbc/execute/diagnostic.clj
@@ -24,14 +24,14 @@
```
(sql-jdbc.execute.diagnostic/capturing-diagnostic-info [diag-info-fn]
;; various body forms
- ;; fetch the diagnostic info, which should be available if execute code called `record-diagnostic-info-for-pool`
+ ;; fetch the diagnostic info, which should be available if execute code called `record-diagnostic-info-for-pool!`
(diag-info-fn))
```"
{:style/indent 1}
[[diagnostic-info-fn-binding] & body]
`(do-with-diagnostic-info (fn [~diagnostic-info-fn-binding] ~@body)))
-(defn record-diagnostic-info-for-pool
+(defn record-diagnostic-info-for-pool!
"Captures diagnostic info related to the given `driver`, `database-id`, and `datasource` (which are all related).
The current information that is captured (in a map whose keys are namespaced keywords in this ns) is:
diff --git a/src/metabase/driver/sql_jdbc/execute/old_impl.clj b/src/metabase/driver/sql_jdbc/execute/old_impl.clj
index 05c69d70dfa874837c2569ced2485f1203e16bf2..8f8d94afe71c9a05e6240ef55a09325bc757a085 100644
--- a/src/metabase/driver/sql_jdbc/execute/old_impl.clj
+++ b/src/metabase/driver/sql_jdbc/execute/old_impl.clj
@@ -1,14 +1,20 @@
(ns metabase.driver.sql-jdbc.execute.old-impl
- "Old implementation of `metabase.driver.sql-jdbc.execute` for non-reducible results (i.e., for implementing
- `driver/execute-query` rather than `driver/execute-reducible-query`).
-
- All methods and functions in this namespace should be considered deprecated with the exception of `set-parameter`,
- which will be moved to `metabase.driver.sql-jdbc.execute` when this namespace is removed."
+ "Old implementations of [[metabase.driver.sql-jdbc.execute]] methods. All methods and functions in this namespace
+ should be considered deprecated and will be removed in future releases."
(:require
[metabase.driver :as driver]))
(set! *warn-on-reflection* true)
+(defmulti connection-with-timezone
+ "Deprecated in Metabase 47. Implement [[metabase.driver.sql-jdbc.execute/do-with-connection-with-options]] instead.
+ This method will be removed in or after Metabase 50."
+ {:added "0.35.0"
+ :deprecated "0.47.0"
+ :arglists '(^java.sql.Connection [driver database ^String timezone-id])}
+ driver/dispatch-on-initialized-driver
+ :hierarchy #'driver/hierarchy)
+
(defmulti set-timezone-sql
"Return a format string containing a SQL statement to be used to set the timezone for the current transaction.
The `%s` will be replaced with a string literal for a timezone, e.g. `US/Pacific.` (Timezone ID will come already
@@ -17,8 +23,8 @@
\"SET @@session.time_zone = %s;\"
This method is only called for drivers using the default implementation
- of [[metabase.driver.sql-jdbc.execute/connection-with-timezone]]; it should be considered deprecated in favor of
- implementing `connection-with-timezone` directly."
+ of [[metabase.driver.sql-jdbc.execute/do-with-connection-with-options]]; it should be considered deprecated in
+ favor of implementing [[metabase.driver.sql-jdbc.execute/do-with-connection-with-options]] directly."
{:deprecated "0.35.0", :arglists '([driver])}
driver/dispatch-on-initialized-driver
:hierarchy #'driver/hierarchy)
diff --git a/src/metabase/driver/sql_jdbc/sync/dbms_version.clj b/src/metabase/driver/sql_jdbc/sync/dbms_version.clj
index 939f329da0a75f5d72dc26040b3b729666d5205c..9a3383bf4e823ed42fcede6aedc29c6149baa5db 100644
--- a/src/metabase/driver/sql_jdbc/sync/dbms_version.clj
+++ b/src/metabase/driver/sql_jdbc/sync/dbms_version.clj
@@ -1,14 +1,19 @@
(ns metabase.driver.sql-jdbc.sync.dbms-version
(:require
- [clojure.java.jdbc :as jdbc]))
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]))
(set! *warn-on-reflection* true)
(defn dbms-version
"Default implementation of `driver/dbms-version` for SQL JDBC drivers. Uses JDBC DatabaseMetaData."
- [_driver jdbc-spec]
- (jdbc/with-db-metadata [metadata jdbc-spec]
- {:flavor (.getDatabaseProductName metadata)
- :version (.getDatabaseProductVersion metadata)
- :semantic-version [(.getDatabaseMajorVersion metadata)
- (.getDatabaseMinorVersion metadata)]}))
+ [driver jdbc-spec]
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ jdbc-spec
+ nil
+ (fn [^java.sql.Connection conn]
+ (let [metadata (.getMetaData conn)]
+ {:flavor (.getDatabaseProductName metadata)
+ :version (.getDatabaseProductVersion metadata)
+ :semantic-version [(.getDatabaseMajorVersion metadata)
+ (.getDatabaseMinorVersion metadata)]}))))
diff --git a/src/metabase/driver/sql_jdbc/sync/describe_database.clj b/src/metabase/driver/sql_jdbc/sync/describe_database.clj
index 29e19d94d6009ce25c687544960448b82e09ebc5..5235497990440006609994629d2b6da6c9681770 100644
--- a/src/metabase/driver/sql_jdbc/sync/describe_database.clj
+++ b/src/metabase/driver/sql_jdbc/sync/describe_database.clj
@@ -1,10 +1,8 @@
(ns metabase.driver.sql-jdbc.sync.describe-database
"SQL JDBC impl for `describe-database`."
(:require
- [clojure.java.jdbc :as jdbc]
[clojure.string :as str]
[metabase.driver :as driver]
- [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
[metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.driver.sql-jdbc.sync.common :as sql-jdbc.sync.common]
[metabase.driver.sql-jdbc.sync.interface :as sql-jdbc.sync.interface]
@@ -101,17 +99,17 @@
"Fetch a JDBC Metadata ResultSet of tables in the DB, optionally limited to ones belonging to a given
schema. Returns a reducible sequence of results."
[driver ^DatabaseMetaData metadata ^String schema-or-nil ^String db-name-or-nil]
- (sql-jdbc.sync.common/reducible-results
- #(.getTables metadata db-name-or-nil (some->> schema-or-nil (driver/escape-entity-name-for-metadata driver)) "%"
- (into-array String ["TABLE" "PARTITIONED TABLE" "VIEW" "FOREIGN TABLE" "MATERIALIZED VIEW"
- "EXTERNAL TABLE"]))
- (fn [^ResultSet rs]
- (fn []
- {:name (.getString rs "TABLE_NAME")
- :schema (.getString rs "TABLE_SCHEM")
- :description (when-let [remarks (.getString rs "REMARKS")]
- (when-not (str/blank? remarks)
- remarks))}))))
+ (with-open [rset (.getTables metadata db-name-or-nil (some->> schema-or-nil (driver/escape-entity-name-for-metadata driver)) "%"
+ (into-array String ["TABLE" "PARTITIONED TABLE" "VIEW" "FOREIGN TABLE" "MATERIALIZED VIEW"
+ "EXTERNAL TABLE"]))]
+ (loop [acc []]
+ (if-not (.next rset)
+ acc
+ (recur (conj acc {:name (.getString rset "TABLE_NAME")
+ :schema (.getString rset "TABLE_SCHEM")
+ :description (when-let [remarks (.getString rset "REMARKS")]
+ (when-not (str/blank? remarks)
+ remarks))}))))))
(defn fast-active-tables
"Default, fast implementation of `active-tables` best suited for DBs with lots of system tables (like Oracle). Fetch
@@ -157,23 +155,25 @@
:else
nil))
-(defn describe-database
- "Default implementation of `driver/describe-database` for SQL JDBC drivers. Uses JDBC DatabaseMetaData."
- [driver db-or-id-or-spec]
- {:tables (with-open [conn (jdbc/get-connection (sql-jdbc.conn/db->pooled-connection-spec db-or-id-or-spec))]
- ;; try to set the Connection to `READ_UNCOMMITED` if possible, or whatever the next least-locking level
- ;; is. Not sure how much of a difference that makes since we're not running this inside a transaction,
- ;; but better safe than sorry
- (sql-jdbc.execute/set-best-transaction-level! driver conn)
- (let [schema-filter-prop (driver.u/find-schema-filters-prop driver)
- has-schema-filter-prop? (some? schema-filter-prop)
- default-active-tbl-fn #(into #{} (sql-jdbc.sync.interface/active-tables driver conn nil nil))]
- (if has-schema-filter-prop?
- (if-let [database (db-or-id-or-spec->database db-or-id-or-spec)]
- (let [prop-nm (:name schema-filter-prop)
- [inclusion-patterns exclusion-patterns] (driver.s/db-details->schema-filter-patterns
- prop-nm
- database)]
- (into #{} (sql-jdbc.sync.interface/active-tables driver conn inclusion-patterns exclusion-patterns)))
- (default-active-tbl-fn))
- (default-active-tbl-fn))))})
+(mu/defn describe-database
+ "Default implementation of [[metabase.driver/describe-database]] for SQL JDBC drivers. Uses JDBC DatabaseMetaData."
+ [driver :- :keyword
+ db-or-id-or-spec :- [:or :int :map]]
+ {:tables
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ db-or-id-or-spec
+ nil
+ (fn [^Connection conn]
+ (let [schema-filter-prop (driver.u/find-schema-filters-prop driver)
+ has-schema-filter-prop? (some? schema-filter-prop)
+ default-active-tbl-fn #(into #{} (sql-jdbc.sync.interface/active-tables driver conn nil nil))]
+ (if has-schema-filter-prop?
+ (if-let [database (db-or-id-or-spec->database db-or-id-or-spec)]
+ (let [prop-nm (:name schema-filter-prop)
+ [inclusion-patterns exclusion-patterns] (driver.s/db-details->schema-filter-patterns
+ prop-nm
+ database)]
+ (into #{} (sql-jdbc.sync.interface/active-tables driver conn inclusion-patterns exclusion-patterns)))
+ (default-active-tbl-fn))
+ (default-active-tbl-fn)))))})
diff --git a/src/metabase/driver/sql_jdbc/sync/describe_table.clj b/src/metabase/driver/sql_jdbc/sync/describe_table.clj
index 78d8d53e050fc39f67f5272fa4388293f9ca686f..2129afe4fc1d4836991adc26992ea1e1e0e8a50c 100644
--- a/src/metabase/driver/sql_jdbc/sync/describe_table.clj
+++ b/src/metabase/driver/sql_jdbc/sync/describe_table.clj
@@ -8,7 +8,7 @@
[medley.core :as m]
[metabase.db.metadata-queries :as metadata-queries]
[metabase.driver :as driver]
- [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.driver.sql-jdbc.sync.common :as sql-jdbc.sync.common]
[metabase.driver.sql-jdbc.sync.interface :as sql-jdbc.sync.interface]
[metabase.driver.sql.query-processor :as sql.qp]
@@ -24,7 +24,9 @@
(set! *warn-on-reflection* true)
-(defmethod sql-jdbc.sync.interface/column->semantic-type :sql-jdbc [_ _ _] nil)
+(defmethod sql-jdbc.sync.interface/column->semantic-type :sql-jdbc
+ [_driver _database-type _column-name]
+ nil)
(defn pattern-based-database-type->base-type
"Return a `database-type->base-type` function that matches types based on a sequence of pattern / base-type pairs.
@@ -225,9 +227,12 @@
[driver db-or-id-or-spec-or-conn table]
(if (instance? Connection db-or-id-or-spec-or-conn)
(describe-table* driver db-or-id-or-spec-or-conn table)
- (let [spec (sql-jdbc.conn/db->pooled-connection-spec db-or-id-or-spec-or-conn)]
- (with-open [conn (jdbc/get-connection spec)]
- (describe-table* driver conn table)))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ db-or-id-or-spec-or-conn
+ nil
+ (fn [^Connection conn]
+ (describe-table* driver conn table)))))
(defn- describe-table-fks*
[_driver ^Connection conn {^String schema :schema, ^String table-name :name} & [^String db-name-or-nil]]
@@ -246,9 +251,12 @@
[driver db-or-id-or-spec-or-conn table & [db-name-or-nil]]
(if (instance? Connection db-or-id-or-spec-or-conn)
(describe-table-fks* driver db-or-id-or-spec-or-conn table db-name-or-nil)
- (let [spec (sql-jdbc.conn/db->pooled-connection-spec db-or-id-or-spec-or-conn)]
- (with-open [conn (jdbc/get-connection spec)]
- (describe-table-fks* driver conn table db-name-or-nil)))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ db-or-id-or-spec-or-conn
+ nil
+ (fn [^Connection conn]
+ (describe-table-fks* driver conn table db-name-or-nil)))))
(def ^:dynamic *nested-field-column-max-row-length*
"Max string length for a row for nested field column before we just give up on parsing it.
@@ -397,35 +405,39 @@
(defn describe-nested-field-columns
"Default implementation of [[metabase.driver.sql-jdbc.sync.interface/describe-nested-field-columns]] for SQL JDBC
drivers. Goes and queries the table if there are JSON columns for the nested contents."
- [driver spec table]
- (with-open [conn (jdbc/get-connection spec)]
- (let [table-identifier-info [(:schema table) (:name table)]
- table-fields (describe-table-fields driver conn table nil)
- json-fields (filter #(isa? (:base-type %) :type/JSON) table-fields)]
- (if (nil? (seq json-fields))
- #{}
- (sql.qp/with-driver-honey-sql-version driver
- (let [existing-fields-by-name (m/index-by :name (t2/select Field :table_id (u/the-id table)))
- unfold-json-fields (remove (fn [field]
- (when-let [existing-field (existing-fields-by-name (:name field))]
- (false? (:json_unfolding existing-field))))
- json-fields)]
- (if (empty? unfold-json-fields)
- #{}
- (binding [hx/*honey-sql-version* (sql.qp/honey-sql-version driver)]
- (let [json-field-names (mapv #(apply hx/identifier :field (into table-identifier-info [(:name %)])) unfold-json-fields)
- table-identifier (apply hx/identifier :table table-identifier-info)
- sql-args (sql.qp/format-honeysql driver {:select (mapv sql.qp/maybe-wrap-unaliased-expr json-field-names)
- :from [(sql.qp/maybe-wrap-unaliased-expr table-identifier)]
- :limit metadata-queries/nested-field-sample-limit})
- query (jdbc/reducible-query spec sql-args {:identifiers identity})
- field-types (transduce describe-json-xform describe-json-rf query)
- fields (field-types->fields field-types)]
- (if (> (count fields) max-nested-field-columns)
- (do
- (log/warn
- (format
- "More nested field columns detected than maximum. Limiting the number of nested field columns to %d."
- max-nested-field-columns))
- (set (take max-nested-field-columns fields)))
- fields))))))))))
+ [driver jdbc-spec table]
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ jdbc-spec
+ nil
+ (fn [^Connection conn]
+ (let [table-identifier-info [(:schema table) (:name table)]
+ table-fields (describe-table-fields driver conn table nil)
+ json-fields (filter #(isa? (:base-type %) :type/JSON) table-fields)]
+ (if (nil? (seq json-fields))
+ #{}
+ (sql.qp/with-driver-honey-sql-version driver
+ (let [existing-fields-by-name (m/index-by :name (t2/select Field :table_id (u/the-id table)))
+ unfold-json-fields (remove (fn [field]
+ (when-let [existing-field (existing-fields-by-name (:name field))]
+ (false? (:json_unfolding existing-field))))
+ json-fields)]
+ (if (empty? unfold-json-fields)
+ #{}
+ (binding [hx/*honey-sql-version* (sql.qp/honey-sql-version driver)]
+ (let [json-field-names (mapv #(apply hx/identifier :field (into table-identifier-info [(:name %)])) unfold-json-fields)
+ table-identifier (apply hx/identifier :table table-identifier-info)
+ sql-args (sql.qp/format-honeysql driver {:select (mapv sql.qp/maybe-wrap-unaliased-expr json-field-names)
+ :from [(sql.qp/maybe-wrap-unaliased-expr table-identifier)]
+ :limit metadata-queries/nested-field-sample-limit})
+ query (jdbc/reducible-query jdbc-spec sql-args {:identifiers identity})
+ field-types (transduce describe-json-xform describe-json-rf query)
+ fields (field-types->fields field-types)]
+ (if (> (count fields) max-nested-field-columns)
+ (do
+ (log/warn
+ (format
+ "More nested field columns detected than maximum. Limiting the number of nested field columns to %d."
+ max-nested-field-columns))
+ (set (take max-nested-field-columns fields)))
+ fields)))))))))))
diff --git a/src/metabase/lib/schema/expression/temporal.cljc b/src/metabase/lib/schema/expression/temporal.cljc
index db3db6ae63187e3170dca8ecddad5ef5ae94ef42..f265da03608a7a2e5d4fd0c1010775e1772201c0 100644
--- a/src/metabase/lib/schema/expression/temporal.cljc
+++ b/src/metabase/lib/schema/expression/temporal.cljc
@@ -77,7 +77,6 @@
(mr/def ::timezone-id
[:and
-
::common/non-blank-string
(into [:enum
{:error/message "valid timezone ID"
diff --git a/src/metabase/lib/schema/literal/jvm.clj b/src/metabase/lib/schema/literal/jvm.clj
index 66bc10aad98c0c6c76d559947c1cc4d81bb9391a..7dd076d082e137aef1dc34c2c9c1ff506167dd1f 100644
--- a/src/metabase/lib/schema/literal/jvm.clj
+++ b/src/metabase/lib/schema/literal/jvm.clj
@@ -6,7 +6,9 @@
(set! *warn-on-reflection* true)
-(defn- instance-of [^Class klass]
+(defn instance-of
+ "Convenience for defining a Malli schema for an instance of a particular Class."
+ [^Class klass]
[:fn {:error/message (str "instance of " (.getName klass))}
#(instance? klass %)])
diff --git a/src/metabase/query_processor/middleware/add_implicit_joins.clj b/src/metabase/query_processor/middleware/add_implicit_joins.clj
index 45a41a783dc29d7237c96cd96c89b8d465abd8cc..4fe206b70f61a80fd4945a0bc2fe67c1c03c5735 100644
--- a/src/metabase/query_processor/middleware/add_implicit_joins.clj
+++ b/src/metabase/query_processor/middleware/add_implicit_joins.clj
@@ -119,7 +119,7 @@
(if-not (some #{:source-metadata} &parents)
(let [join-alias (or (fk-field-id->join-alias (:source-field opts))
(throw (ex-info (tru "Cannot find matching FK Table ID for FK Field {0}"
- (fk-field-id->join-alias (:source-field opts)))
+ (pr-str (:source-field opts)))
{:resolving &match
:candidates fk-field-id->join-alias})))]
[:field id-or-name (assoc opts :join-alias join-alias)])
diff --git a/test/metabase/db/fix_mysql_utf8_test.clj b/test/metabase/db/fix_mysql_utf8_test.clj
index 0a3d2376ba2680a09e549d782f1254fc54f2452e..78f1186432ad1db5330ccd162c94ba1d1faa126f 100644
--- a/test/metabase/db/fix_mysql_utf8_test.clj
+++ b/test/metabase/db/fix_mysql_utf8_test.clj
@@ -6,17 +6,24 @@
[metabase.db.data-source :as mdb.data-source]
[metabase.db.setup :as mdb.setup]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.models :refer [Database]]
[metabase.test :as mt]
[toucan.db :as db]
[toucan2.core :as t2]))
(defn- create-test-db! []
- (jdbc/with-db-connection [server-conn (sql-jdbc.conn/connection-details->spec :mysql
- (mt/dbdef->connection-details :mysql :server nil))]
- (doseq [statement ["DROP DATABASE IF EXISTS utf8_test;"
- "CREATE DATABASE utf8_test;"]]
- (jdbc/execute! server-conn statement))))
+ (let [spec (sql-jdbc.conn/connection-details->spec
+ :mysql
+ (mt/dbdef->connection-details :mysql :server nil))]
+ (sql-jdbc.execute/do-with-connection-with-options
+ :mysql
+ spec
+ {:write? true}
+ (fn [^java.sql.Connection server-conn]
+ (doseq [statement ["DROP DATABASE IF EXISTS utf8_test;"
+ "CREATE DATABASE utf8_test;"]]
+ (jdbc/execute! {:connection server-conn} statement))))))
(defn- test-data-source ^javax.sql.DataSource []
(mdb.data-source/broken-out-details->DataSource
diff --git a/test/metabase/db/liquibase_test.clj b/test/metabase/db/liquibase_test.clj
index 20a56d64608cad444c286857771c1752516ae550..f68f0274d08e1784a1f403f18182a5945651f6cc 100644
--- a/test/metabase/db/liquibase_test.clj
+++ b/test/metabase/db/liquibase_test.clj
@@ -1,22 +1,26 @@
(ns metabase.db.liquibase-test
(:require
- [clojure.java.jdbc :as jdbc]
[clojure.string :as str]
[clojure.test :refer :all]
[metabase.db.liquibase :as liquibase]
[metabase.db.test-util :as mdb.test-util]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
- [metabase.test :as mt]))
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
+ [metabase.test :as mt]
+ [next.jdbc :as next.jdbc]))
(deftest mysql-engine-charset-test
(mt/test-driver :mysql
(testing "Make sure MySQL CREATE DATABASE statements have ENGINE/CHARACTER SET appended to them (#10691)"
- (jdbc/with-db-connection [conn (sql-jdbc.conn/connection-details->spec
- :mysql
- (mt/dbdef->connection-details :mysql :server nil))]
- (doseq [statement ["DROP DATABASE IF EXISTS liquibase_test;"
- "CREATE DATABASE liquibase_test;"]]
- (jdbc/execute! conn statement)))
+ (sql-jdbc.execute/do-with-connection-with-options
+ :mysql
+ (sql-jdbc.conn/connection-details->spec :mysql
+ (mt/dbdef->connection-details :mysql :server nil))
+ {:write? true}
+ (fn [^java.sql.Connection conn]
+ (doseq [statement ["DROP DATABASE IF EXISTS liquibase_test;"
+ "CREATE DATABASE liquibase_test;"]]
+ (next.jdbc/execute! conn [statement]))))
(liquibase/with-liquibase [liquibase (->> (mt/dbdef->connection-details :mysql :db {:database-name "liquibase_test"})
(sql-jdbc.conn/connection-details->spec :mysql)
mdb.test-util/->ClojureJDBCSpecDataSource)]
diff --git a/test/metabase/db/test_util.clj b/test/metabase/db/test_util.clj
index ec6820c2a5d57106066af5f38cc32dee05818722..e0d5d4d4d2722b0fbf2a20b47fbfe8c380a61ffd 100644
--- a/test/metabase/db/test_util.clj
+++ b/test/metabase/db/test_util.clj
@@ -15,6 +15,7 @@
javax.sql.DataSource
(getConnection [_]
+ #_{:clj-kondo/ignore [:discouraged-var]}
(jdbc/get-connection jdbc-spec))
(getConnection [_ _user _password]
diff --git a/test/metabase/driver/mysql_test.clj b/test/metabase/driver/mysql_test.clj
index 83ce2e2817711c04b730b5f05f27fb7d85a47e8c..228d6c813a1e2d5f079b0940074dcbc600a6f229 100644
--- a/test/metabase/driver/mysql_test.clj
+++ b/test/metabase/driver/mysql_test.clj
@@ -620,9 +620,9 @@
(is (thrown-with-msg?
Exception
#"Killed mysql process id [\d,]+ due to timeout."
- (#'mysql.ddl/execute-with-timeout! db-spec db-spec 10 ["select sleep(5)"]))))
+ (#'mysql.ddl/execute-with-timeout! :mysql db-spec db-spec 10 ["select sleep(5)"]))))
(testing "When the query takes less time than the timeout, it is successful."
- (is (some? (#'mysql.ddl/execute-with-timeout! db-spec db-spec 5000 ["select sleep(0.1) as val"]))))))))
+ (is (some? (#'mysql.ddl/execute-with-timeout! :mysql db-spec db-spec 5000 ["select sleep(0.1) as val"]))))))))
(deftest syncable-schemas-test
(mt/test-driver :mysql
diff --git a/test/metabase/driver/postgres_test.clj b/test/metabase/driver/postgres_test.clj
index 99d12ec65ea926baee56f06918faa2e9ffabd8f8..322030e915d7a2bf9e9ef9c158c608c43778947d 100644
--- a/test/metabase/driver/postgres_test.clj
+++ b/test/metabase/driver/postgres_test.clj
@@ -35,10 +35,9 @@
#_{:clj-kondo/ignore [:discouraged-namespace]}
[metabase.util.honeysql-extensions :as hx]
[metabase.util.log :as log]
+ [next.jdbc :as next.jdbc]
[toucan2.core :as t2]
- [toucan2.tools.with-temp :as t2.with-temp])
- (:import
- (java.sql DatabaseMetaData)))
+ [toucan2.tools.with-temp :as t2.with-temp]))
(set! *warn-on-reflection* true)
@@ -323,8 +322,12 @@
spec (sql-jdbc.conn/connection-details->spec :postgres details)]
;; create the postgres DB
(drop-if-exists-and-create-db! db-name)
- (let [major-v ((jdbc/with-db-metadata [metadata spec]
- #(.getDatabaseMajorVersion ^DatabaseMetaData metadata)))]
+ (let [major-v (sql-jdbc.execute/do-with-connection-with-options
+ :postgres
+ spec
+ nil
+ (fn [^java.sql.Connection conn]
+ (.. conn getMetaData getDatabaseMajorVersion)))]
(if (>= major-v 10)
;; create the DB object
(t2.with-temp/with-temp [Database database {:engine :postgres, :details (assoc details :dbname db-name)}]
@@ -404,15 +407,14 @@
json-part (json/generate-string {:bob :dobbs})
insert (str "CREATE TABLE json_alias_test (json_part JSON NOT NULL);"
(format "INSERT INTO json_alias_test (json_part) VALUES ('%s');" json-part))]
- (jdbc/with-db-connection [_conn (sql-jdbc.conn/connection-details->spec :postgres details)]
- (jdbc/execute! spec [insert]))
- (mt/with-temp* [Database [database {:engine :postgres, :details details}]
- Table [table {:db_id (u/the-id database) :name "json_alias_test"}]
- Field [field {:table_id (u/the-id table)
- :nfc_path [:bob
- "injection' OR 1=1--' AND released = 1"
- (keyword "injection' OR 1=1--' AND released = 1")],
- :name "json_alias_test"}]]
+ (jdbc/execute! spec [insert])
+ (t2.with-temp/with-temp [Database database {:engine :postgres, :details details}
+ Table table {:db_id (u/the-id database) :name "json_alias_test"}
+ Field field {:table_id (u/the-id table)
+ :nfc_path [:bob
+ "injection' OR 1=1--' AND released = 1"
+ (keyword "injection' OR 1=1--' AND released = 1")],
+ :name "json_alias_test"}]
(let [field-bucketed [:field (u/the-id field)
{:temporal-unit :month,
:metabase.query-processor.util.add-alias-info/source-table (u/the-id table),
@@ -489,9 +491,8 @@
(let [details (mt/dbdef->connection-details :postgres :db {:database-name "describe-json-test"
:json-unfolding true})
spec (sql-jdbc.conn/connection-details->spec :postgres details)]
- (jdbc/with-db-connection [_conn (sql-jdbc.conn/connection-details->spec :postgres details)]
- (jdbc/execute! spec [describe-json-table-sql]))
- (mt/with-temp* [Database [database {:engine :postgres, :details details}]]
+ (jdbc/execute! spec [describe-json-table-sql])
+ (t2.with-temp/with-temp [Database database {:engine :postgres, :details details}]
(mt/with-db database
(is (= [:type/JSON :type/SerializedJSON]
(-> (sql-jdbc.sync/describe-table :postgres database {:name "describe_json_table"})
@@ -553,10 +554,9 @@
(let [details (mt/dbdef->connection-details :postgres :db {:database-name "describe-json-with-schema-test"
:json-unfolding true})
spec (sql-jdbc.conn/connection-details->spec :postgres details)]
- (jdbc/with-db-connection [_conn (sql-jdbc.conn/connection-details->spec :postgres details)]
- (jdbc/execute! spec [(str "CREATE SCHEMA bobdobbs;"
- "CREATE TABLE bobdobbs.describe_json_table (trivial_json JSONB NOT NULL);"
- "INSERT INTO bobdobbs.describe_json_table (trivial_json) VALUES ('{\"a\": 1}');")]))
+ (jdbc/execute! spec [(str "CREATE SCHEMA bobdobbs;"
+ "CREATE TABLE bobdobbs.describe_json_table (trivial_json JSONB NOT NULL);"
+ "INSERT INTO bobdobbs.describe_json_table (trivial_json) VALUES ('{\"a\": 1}');")])
(t2.with-temp/with-temp [Database database {:engine :postgres, :details details}]
(mt/with-db database
(sync-tables/sync-tables-and-database! database)
@@ -579,10 +579,9 @@
(let [details (mt/dbdef->connection-details :postgres :db {:database-name "describe-json-funky-names-test"
:json-unfolding true})
spec (sql-jdbc.conn/connection-details->spec :postgres details)]
- (jdbc/with-db-connection [_conn (sql-jdbc.conn/connection-details->spec :postgres details)]
- (jdbc/execute! spec [(str "CREATE SCHEMA \"AAAH_#\";"
- "CREATE TABLE \"AAAH_#\".\"dESCribe_json_table_%\" (trivial_json JSONB NOT NULL);"
- "INSERT INTO \"AAAH_#\".\"dESCribe_json_table_%\" (trivial_json) VALUES ('{\"a\": 1}');")]))
+ (jdbc/execute! spec [(str "CREATE SCHEMA \"AAAH_#\";"
+ "CREATE TABLE \"AAAH_#\".\"dESCribe_json_table_%\" (trivial_json JSONB NOT NULL);"
+ "INSERT INTO \"AAAH_#\".\"dESCribe_json_table_%\" (trivial_json) VALUES ('{\"a\": 1}');")])
(t2.with-temp/with-temp [Database database {:engine :postgres, :details details}]
(mt/with-db database
(sync-tables/sync-tables-and-database! database)
@@ -609,8 +608,7 @@
big-json (json/generate-string big-map)
sql (str "CREATE TABLE big_json_table (big_json JSON NOT NULL);"
(format "INSERT INTO big_json_table (big_json) VALUES ('%s');" big-json))]
- (jdbc/with-db-connection [_conn (sql-jdbc.conn/connection-details->spec :postgres details)]
- (jdbc/execute! spec [sql]))
+ (jdbc/execute! spec [sql])
(t2.with-temp/with-temp [Database database {:engine :postgres, :details details}]
(mt/with-db database
(sync-tables/sync-tables-and-database! database)
@@ -716,12 +714,16 @@
(defn- do-with-money-test-db [thunk]
(drop-if-exists-and-create-db! "money_columns_test")
(let [details (mt/dbdef->connection-details :postgres :db {:database-name "money_columns_test"})]
- (jdbc/with-db-connection [conn (sql-jdbc.conn/connection-details->spec :postgres details)]
- (doseq [sql+args [["CREATE table bird_prices (bird TEXT, price money);"]
- ["INSERT INTO bird_prices (bird, price) VALUES (?, ?::numeric::money), (?, ?::numeric::money);"
- "Lucky Pigeon" 6.0
- "Katie Parakeet" 23.99]]]
- (jdbc/execute! conn sql+args)))
+ (sql-jdbc.execute/do-with-connection-with-options
+ :postgres
+ (sql-jdbc.conn/connection-details->spec :postgres details)
+ {:write? true}
+ (fn [^java.sql.Connection conn]
+ (doseq [sql+args [["CREATE table bird_prices (bird TEXT, price money);"]
+ ["INSERT INTO bird_prices (bird, price) VALUES (?, ?::numeric::money), (?, ?::numeric::money);"
+ "Lucky Pigeon" 6.0
+ "Katie Parakeet" 23.99]]]
+ (next.jdbc/execute! conn sql+args))))
(t2.with-temp/with-temp [Database db {:engine :postgres, :details (assoc details :dbname "money_columns_test")}]
(sync/sync-database! db)
(mt/with-db db
@@ -731,12 +733,16 @@
(mt/test-driver :postgres
(testing "We should support the Postgres MONEY type"
(testing "It should be possible to return money column results (#3754)"
- (with-open [conn (sql-jdbc.execute/connection-with-timezone :postgres (mt/db) nil)
- stmt (sql-jdbc.execute/prepared-statement :postgres conn "SELECT 1000::money AS \"money\";" nil)
- rs (sql-jdbc.execute/execute-prepared-statement! :postgres stmt)]
- (let [row-thunk (sql-jdbc.execute/row-thunk :postgres rs (.getMetaData rs))]
- (is (= [1000.00M]
- (row-thunk))))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ :postgres
+ (mt/db)
+ nil
+ (fn [conn]
+ (with-open [stmt (sql-jdbc.execute/prepared-statement :postgres conn "SELECT 1000::money AS \"money\";" nil)
+ rs (sql-jdbc.execute/execute-prepared-statement! :postgres stmt)]
+ (let [row-thunk (sql-jdbc.execute/row-thunk :postgres rs (.getMetaData rs))]
+ (is (= [1000.00M]
+ (row-thunk))))))))
(do-with-money-test-db
(fn []
@@ -792,8 +798,8 @@
properly."
[]
(drop-if-exists-and-create-db! "enums_test")
- (jdbc/with-db-connection [conn (sql-jdbc.conn/connection-details->spec :postgres (enums-test-db-details))]
- (jdbc/execute! conn [enums-db-sql])))
+ (let [spec (sql-jdbc.conn/connection-details->spec :postgres (enums-test-db-details))]
+ (jdbc/execute! spec [enums-db-sql])))
(defn- do-with-enums-db {:style/indent 0} [f]
(create-enums-db!)
@@ -1014,12 +1020,15 @@
"DROP USER IF EXISTS no_select_test_user;"
"CREATE USER no_select_test_user WITH PASSWORD '123456';"
"GRANT SELECT ON TABLE \"no-select-test\".PUBLIC.table_with_perms TO no_select_test_user;"]]
- (jdbc/execute! spec [statement])))
+ (is (= [0]
+ (jdbc/execute! spec [statement])))))
(let [test-user-details (assoc (mt/dbdef->connection-details :postgres :db {:database-name "no-select-test"})
:user "no_select_test_user"
:password "123456")]
(t2.with-temp/with-temp [Database database {:engine :postgres, :details test-user-details}]
- (sync/sync-database! database)
+ ;; make sure that sync still succeeds even tho some tables are not SELECTable.
+ (binding [sync-util/*log-exceptions-and-continue?* false]
+ (is (some? (sync/sync-database! database))))
(is (= #{"table_with_perms"}
(t2/select-fn-set :name Table :db_id (:id database)))))))))
diff --git a/test/metabase/driver/sql_jdbc/connection_test.clj b/test/metabase/driver/sql_jdbc/connection_test.clj
index ab37b963750c1131867e68c8f40b3f973a6d1252..e38661f699826241f340422d3b7ac0120276440b 100644
--- a/test/metabase/driver/sql_jdbc/connection_test.clj
+++ b/test/metabase/driver/sql_jdbc/connection_test.clj
@@ -7,6 +7,7 @@
[metabase.db.spec :as mdb.spec]
[metabase.driver :as driver]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.driver.sql-jdbc.test-util :as sql-jdbc.tu]
[metabase.driver.util :as driver.u]
[metabase.models :refer [Database Secret]]
@@ -15,6 +16,7 @@
[metabase.test.data :as data]
[metabase.test.fixtures :as fixtures]
[metabase.util :as u]
+ [next.jdbc :as next.jdbc]
[toucan2.core :as t2]
[toucan2.tools.with-temp :as t2.with-temp]))
@@ -49,26 +51,30 @@
(with-redefs [sql-jdbc.conn/destroy-pool! (fn [id destroyed-spec]
(original-destroy id destroyed-spec)
(reset! destroyed? true))]
- (jdbc/with-db-connection [_conn spec]
- (jdbc/execute! spec ["CREATE TABLE birds (name varchar)"])
- (jdbc/execute! spec ["INSERT INTO birds values ('rasta'),('lucky')"])
- (t2.with-temp/with-temp [Database database {:engine :h2, :details connection-details}]
- (testing "database id is not in our connection map initially"
- ;; deref'ing a var to get the atom. looks weird
- (is (not (contains? @@#'sql-jdbc.conn/database-id->connection-pool
- (u/id database)))))
- (testing "when getting a pooled connection it is now in our connection map"
- (let [stored-spec (sql-jdbc.conn/db->pooled-connection-spec database)
- birds (jdbc/query stored-spec ["SELECT * FROM birds"])]
- (is (seq birds))
- (is (contains? @@#'sql-jdbc.conn/database-id->connection-pool
- (u/id database)))))
- (testing "and is no longer in our connection map after cleanup"
- (#'sql-jdbc.conn/set-pool! (u/id database) nil nil)
- (is (not (contains? @@#'sql-jdbc.conn/database-id->connection-pool
- (u/id database)))))
- (testing "the pool has been destroyed"
- (is @destroyed?)))))))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ :h2
+ spec
+ {:write? true}
+ (fn [conn]
+ (next.jdbc/execute! conn ["CREATE TABLE birds (name varchar)"])
+ (next.jdbc/execute! conn ["INSERT INTO birds values ('rasta'),('lucky')"])
+ (t2.with-temp/with-temp [Database database {:engine :h2, :details connection-details}]
+ (testing "database id is not in our connection map initially"
+ ;; deref'ing a var to get the atom. looks weird
+ (is (not (contains? @@#'sql-jdbc.conn/database-id->connection-pool
+ (u/id database)))))
+ (testing "when getting a pooled connection it is now in our connection map"
+ (let [stored-spec (sql-jdbc.conn/db->pooled-connection-spec database)
+ birds (jdbc/query stored-spec ["SELECT * FROM birds"])]
+ (is (seq birds))
+ (is (contains? @@#'sql-jdbc.conn/database-id->connection-pool
+ (u/id database)))))
+ (testing "and is no longer in our connection map after cleanup"
+ (#'sql-jdbc.conn/set-pool! (u/id database) nil nil)
+ (is (not (contains? @@#'sql-jdbc.conn/database-id->connection-pool
+ (u/id database)))))
+ (testing "the pool has been destroyed"
+ (is @destroyed?))))))))))
(deftest c3p0-datasource-name-test
(mt/test-drivers (sql-jdbc.tu/sql-jdbc-drivers)
diff --git a/test/metabase/driver/sql_jdbc/execute_test.clj b/test/metabase/driver/sql_jdbc/execute_test.clj
new file mode 100644
index 0000000000000000000000000000000000000000..6ae9b6e83389a39af49d53ef38a6aeef274e576d
--- /dev/null
+++ b/test/metabase/driver/sql_jdbc/execute_test.clj
@@ -0,0 +1,15 @@
+(ns metabase.driver.sql-jdbc.execute-test
+ (:require
+ [clojure.test :refer :all]
+ [malli.core :as mc]
+ [malli.error :as me]
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]))
+
+(deftest ^:parallel ConnectionOptions-test
+ (are [options error] (= error
+ (me/humanize (mc/explain sql-jdbc.execute/ConnectionOptions options)))
+ nil nil
+ {} nil
+ {:session-timezone nil} nil
+ {:session-timezone "US/Pacific"} nil
+ {:session-timezone "X"} {:session-timezone ["invalid timezone ID: \"X\""]}))
diff --git a/test/metabase/driver/sql_jdbc/sync/describe_database_test.clj b/test/metabase/driver/sql_jdbc/sync/describe_database_test.clj
index ed50020c8d5da6b3aba79444f5ddc45a309ca336..2bf1e2cbbe2dd51518d0d884a7be0c88ffdb6555 100644
--- a/test/metabase/driver/sql_jdbc/sync/describe_database_test.clj
+++ b/test/metabase/driver/sql_jdbc/sync/describe_database_test.clj
@@ -3,7 +3,7 @@
[clojure.java.jdbc :as jdbc]
[clojure.test :refer :all]
[metabase.driver :as driver]
- [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.driver.sql-jdbc.sync.describe-database
:as sql-jdbc.describe-database]
[metabase.driver.sql-jdbc.sync.interface :as sql-jdbc.sync.interface]
@@ -25,7 +25,7 @@
(use-fixtures :once (fixtures/initialize :plugins))
-(deftest simple-select-probe-query-test
+(deftest ^:parallel simple-select-probe-query-test
(testing "honey-sql-version produces the same query"
(are [version] (= ["SELECT TRUE AS \"_\" FROM \"schema\".\"wow\" WHERE 1 <> 1 LIMIT 0"]
(binding [hx/*honey-sql-version* version]
@@ -55,22 +55,28 @@
(descendants driver/hierarchy :sql-jdbc))))
(deftest fast-active-tables-test
- (let [spec (sql-jdbc.conn/db->pooled-connection-spec (mt/db))]
- (with-open [conn (jdbc/get-connection spec)]
- ;; We have to mock this to make it work with all DBs
- (with-redefs [sql-jdbc.describe-database/all-schemas (constantly #{"PUBLIC"})]
- (is (= ["CATEGORIES" "CHECKINS" "USERS" "VENUES"]
- (->> (into [] (sql-jdbc.describe-database/fast-active-tables (or driver/*driver* :h2) conn nil nil))
- (map :name)
- sort)))))))
+ (is (= ["CATEGORIES" "CHECKINS" "USERS" "VENUES"]
+ (sql-jdbc.execute/do-with-connection-with-options
+ (or driver/*driver* :h2)
+ (mt/db)
+ nil
+ (fn [^java.sql.Connection conn]
+ ;; We have to mock this to make it work with all DBs
+ (with-redefs [sql-jdbc.describe-database/all-schemas (constantly #{"PUBLIC"})]
+ (->> (into [] (sql-jdbc.describe-database/fast-active-tables (or driver/*driver* :h2) conn nil nil))
+ (map :name)
+ sort)))))))
(deftest post-filtered-active-tables-test
- (let [spec (sql-jdbc.conn/db->pooled-connection-spec (mt/db))]
- (with-open [conn (jdbc/get-connection spec)]
- (is (= ["CATEGORIES" "CHECKINS" "USERS" "VENUES"]
- (->> (into [] (sql-jdbc.describe-database/post-filtered-active-tables :h2 conn nil nil))
- (map :name)
- sort))))))
+ (is (= ["CATEGORIES" "CHECKINS" "USERS" "VENUES"]
+ (sql-jdbc.execute/do-with-connection-with-options
+ :h2
+ (mt/db)
+ nil
+ (fn [^java.sql.Connection conn]
+ (->> (into [] (sql-jdbc.describe-database/post-filtered-active-tables :h2 conn nil nil))
+ (map :name)
+ sort))))))
(deftest describe-database-test
(is (= {:tables #{{:name "USERS", :schema "PUBLIC", :description nil}
@@ -94,10 +100,14 @@
;; taking advantage of the fact that `sql-jdbc.describe-database/describe-database` can accept JBDC connections
;; instead of databases; by doing this we can keep the connection open and check whether resultsets are still
;; open before they would normally get closed
- (jdbc/with-db-connection [conn (sql-jdbc.conn/db->pooled-connection-spec db)]
- (sql-jdbc.describe-database/describe-database driver conn)
- (reduce + (for [^ResultSet rs @resultsets]
- (if (.isClosed rs) 0 1)))))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ db
+ nil
+ (fn [conn]
+ (sql-jdbc.describe-database/describe-database driver {:connection conn})
+ (reduce + (for [^ResultSet rs @resultsets]
+ (if (.isClosed rs) 0 1))))))))
(defn- count-active-tables-in-db
[db-id]
diff --git a/test/metabase/driver/sql_jdbc/sync/describe_table_test.clj b/test/metabase/driver/sql_jdbc/sync/describe_table_test.clj
index 8de3e2e1586869472ae9df47d20f2d73ca2c9ad0..1d4a92e2461d3cd6531c6a1e61c009bf222ce341 100644
--- a/test/metabase/driver/sql_jdbc/sync/describe_table_test.clj
+++ b/test/metabase/driver/sql_jdbc/sync/describe_table_test.clj
@@ -4,7 +4,7 @@
[clojure.test :refer :all]
[metabase.driver :as driver]
[metabase.driver.mysql-test :as mysql-test]
- [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.driver.sql-jdbc.sync.describe-table
:as sql-jdbc.describe-table]
[metabase.driver.sql-jdbc.sync.interface :as sql-jdbc.sync.interface]
@@ -154,15 +154,18 @@
(mt/test-drivers (mt/normal-drivers-with-feature :nested-field-columns)
(when-not (mysql-test/is-mariadb? (u/id (mt/db)))
(mt/dataset json
- (let [spec (sql-jdbc.conn/connection-details->spec driver/*driver* (:details (mt/db)))
- table (t2/select-one Table :id (mt/id :json))]
- (with-open [conn (jdbc/get-connection spec)]
- (let [fields (sql-jdbc.describe-table/describe-table-fields driver/*driver* conn table nil)
- json-field (first (filter #(= (:name %) "json_bit") fields))
- text-field (first (filter #(= (:name %) "bloop") fields))]
- (is (= :details-only
- (:visibility-type json-field)))
- (is (nil? (:visibility-type text-field)))))))))))
+ (let [table (t2/select-one Table :id (mt/id :json))]
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver/*driver*
+ (mt/db)
+ nil
+ (fn [^java.sql.Connection conn]
+ (let [fields (sql-jdbc.describe-table/describe-table-fields driver/*driver* conn table nil)
+ json-field (first (filter #(= (:name %) "json_bit") fields))
+ text-field (first (filter #(= (:name %) "bloop") fields))]
+ (is (= :details-only
+ (:visibility-type json-field)))
+ (is (nil? (:visibility-type text-field))))))))))))
(deftest describe-nested-field-columns-test
(testing "flattened-row"
diff --git a/test/metabase/models/field_values_test.clj b/test/metabase/models/field_values_test.clj
index bd911daed40a0a536441c0f04d04951fdaf815b4..f04107ba6c3c3b4b49223f71bfdfcf7403ef1cbb 100644
--- a/test/metabase/models/field_values_test.clj
+++ b/test/metabase/models/field_values_test.clj
@@ -7,6 +7,7 @@
[clojure.test :refer :all]
[java-time :as t]
[metabase.db.metadata-queries :as metadata-queries]
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.models.database :refer [Database]]
[metabase.models.dimension :refer [Dimension]]
[metabase.models.field :refer [Field]]
@@ -16,10 +17,11 @@
[metabase.sync :as sync]
[metabase.test :as mt]
[metabase.util :as u]
+ [next.jdbc :as next.jdbc]
[toucan2.core :as t2]
[toucan2.tools.with-temp :as t2.with-temp]))
-(deftest field-should-have-field-values?-test
+(deftest ^:parallel field-should-have-field-values?-test
(doseq [[group input->expected] {"Text and Category Fields"
{{:has_field_values :list
:visibility_type :normal
@@ -122,7 +124,7 @@
(-> (t2/select-one FieldValues :id field-values-id)
(select-keys [:values :human_readable_values])))
-(defn- sync-and-find-values [db field-values-id]
+(defn- sync-and-find-values! [db field-values-id]
(sync/sync-database! db)
(find-values field-values-id))
@@ -166,51 +168,55 @@
(deftest update-human-readable-values-test
(testing "Test \"fixing\" of human readable values when field values change"
;; Create a temp warehouse database that can have it's field values change
- (jdbc/with-db-connection [conn {:classname "org.h2.Driver", :subprotocol "h2", :subname "mem:temp"}]
- (jdbc/execute! conn ["drop table foo if exists"])
- (jdbc/execute! conn ["create table foo (id integer primary key, category_id integer not null, desc text)"])
- (jdbc/insert-multi! conn :foo [{:id 1 :category_id 1 :desc "foo"}
- {:id 2 :category_id 2 :desc "bar"}
- {:id 3 :category_id 3 :desc "baz"}])
- ;; Create a new in the Database table for this newly created temp database
- (t2.with-temp/with-temp [Database db {:engine :h2
- :name "foo"
- :is_full_sync true
- :details "{\"db\": \"mem:temp\"}"}]
- ;; Sync the database so we have the new table and it's fields
- (sync/sync-database! db)
- (let [table-id (t2/select-one-fn :id Table :db_id (u/the-id db) :name "FOO")
- field-id (t2/select-one-fn :id Field :table_id table-id :name "CATEGORY_ID")
- field-values-id (t2/select-one-fn :id FieldValues :field_id field-id)]
- ;; Add in human readable values for remapping
- (is (t2/update! FieldValues field-values-id {:human_readable_values ["a" "b" "c"]}))
- (let [expected-original-values {:values [1 2 3]
- :human_readable_values ["a" "b" "c"]}
- expected-updated-values {:values [-2 -1 0 1 2 3]
- :human_readable_values ["-2" "-1" "0" "a" "b" "c"]}]
- (is (= expected-original-values
- (find-values field-values-id)))
-
- (testing "There should be no changes to human_readable_values when resync'd"
- (is (= expected-original-values
- (sync-and-find-values db field-values-id))))
-
- (testing "Add new rows that will have new field values"
- (jdbc/insert-multi! conn :foo [{:id 4 :category_id -2 :desc "foo"}
- {:id 5 :category_id -1 :desc "bar"}
- {:id 6 :category_id 0 :desc "baz"}])
- (testing "Sync to pickup the new field values and rebuild the human_readable_values"
- (is (= expected-updated-values
- (sync-and-find-values db field-values-id)))))
-
- (testing "Resyncing this (with the new field values) should result in the same human_readable_values"
- (is (= expected-updated-values
- (sync-and-find-values db field-values-id))))
-
- (testing "Test that field values can be removed and the corresponding human_readable_values are removed as well"
- (jdbc/delete! conn :foo ["id in (?,?,?)" 1 2 3])
- (is (= {:values [-2 -1 0] :human_readable_values ["-2" "-1" "0"]}
- (sync-and-find-values db field-values-id))))))))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ :h2
+ {:classname "org.h2.Driver", :subprotocol "h2", :subname "mem:temp"}
+ {:write? true}
+ (fn [^java.sql.Connection conn]
+ (next.jdbc/execute! conn ["drop table foo if exists"])
+ (next.jdbc/execute! conn ["create table foo (id integer primary key, category_id integer not null, desc text)"])
+ (jdbc/insert-multi! {:connection conn} :foo [{:id 1 :category_id 1 :desc "foo"}
+ {:id 2 :category_id 2 :desc "bar"}
+ {:id 3 :category_id 3 :desc "baz"}])
+ ;; Create a new in the Database table for this newly created temp database
+ (t2.with-temp/with-temp [Database db {:engine :h2
+ :name "foo"
+ :is_full_sync true
+ :details "{\"db\": \"mem:temp\"}"}]
+ ;; Sync the database so we have the new table and it's fields
+ (sync/sync-database! db)
+ (let [table-id (t2/select-one-fn :id Table :db_id (u/the-id db) :name "FOO")
+ field-id (t2/select-one-fn :id Field :table_id table-id :name "CATEGORY_ID")
+ field-values-id (t2/select-one-fn :id FieldValues :field_id field-id)]
+ ;; Add in human readable values for remapping
+ (is (t2/update! FieldValues field-values-id {:human_readable_values ["a" "b" "c"]}))
+ (let [expected-original-values {:values [1 2 3]
+ :human_readable_values ["a" "b" "c"]}
+ expected-updated-values {:values [-2 -1 0 1 2 3]
+ :human_readable_values ["-2" "-1" "0" "a" "b" "c"]}]
+ (is (= expected-original-values
+ (find-values field-values-id)))
+
+ (testing "There should be no changes to human_readable_values when resync'd"
+ (is (= expected-original-values
+ (sync-and-find-values! db field-values-id))))
+
+ (testing "Add new rows that will have new field values"
+ (jdbc/insert-multi! {:connection conn} :foo [{:id 4 :category_id -2 :desc "foo"}
+ {:id 5 :category_id -1 :desc "bar"}
+ {:id 6 :category_id 0 :desc "baz"}])
+ (testing "Sync to pickup the new field values and rebuild the human_readable_values"
+ (is (= expected-updated-values
+ (sync-and-find-values! db field-values-id)))))
+
+ (testing "Resyncing this (with the new field values) should result in the same human_readable_values"
+ (is (= expected-updated-values
+ (sync-and-find-values! db field-values-id))))
+
+ (testing "Test that field values can be removed and the corresponding human_readable_values are removed as well"
+ (jdbc/delete! {:connection conn} :foo ["id in (?,?,?)" 1 2 3])
+ (is (= {:values [-2 -1 0] :human_readable_values ["-2" "-1" "0"]}
+ (sync-and-find-values! db field-values-id)))))))))))
(deftest validate-human-readable-values-test
(testing "Should validate FieldValues :human_readable_values when"
diff --git a/test/metabase/test/data/one_off_dbs.clj b/test/metabase/test/data/one_off_dbs.clj
index 5d04f36959e243ce92126a00c58dad25ef9414d1..cf8106fe71aae60314ce2a37b43e06c93351071a 100644
--- a/test/metabase/test/data/one_off_dbs.clj
+++ b/test/metabase/test/data/one_off_dbs.clj
@@ -5,6 +5,7 @@
[clojure.java.jdbc :as jdbc]
[clojure.string :as str]
[metabase.db.spec :as mdb.spec]
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.models.database :refer [Database]]
[metabase.sync :as sync]
[metabase.test.data :as data]
@@ -19,13 +20,17 @@
(defn do-with-blank-db
"Impl for `with-blank-db` macro; prefer that to using this directly."
- [f]
+ [thunk]
(let [details {:db (str "mem:" (tu.random/random-name) ";DB_CLOSE_DELAY=10")}]
(t2.with-temp/with-temp [Database db {:engine :h2, :details details}]
(data/with-db db
- (jdbc/with-db-connection [conn (mdb.spec/spec :h2 details)]
- (binding [*conn* conn]
- (f)))))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ :h2
+ (mdb.spec/spec :h2 details)
+ {:write? true}
+ (fn [^java.sql.Connection conn]
+ (binding [*conn* {:connection conn}]
+ (thunk))))))))
(defmacro with-blank-db
"An empty canvas upon which you may paint your dreams.
diff --git a/test/metabase/test/data/sql_jdbc/load_data.clj b/test/metabase/test/data/sql_jdbc/load_data.clj
index a3e87f0d10097dc1bf466b721be1d993af1892e9..afdc2b620436f5064fca5f7fb074e9d157c80931 100644
--- a/test/metabase/test/data/sql_jdbc/load_data.clj
+++ b/test/metabase/test/data/sql_jdbc/load_data.clj
@@ -107,11 +107,11 @@
(defn- make-insert!
"Used by `make-load-data-fn`; creates the actual `insert!` function that gets passed to the `insert-middleware-fns`
described above."
- [driver conn {:keys [database-name], :as _dbdef} {:keys [table-name], :as _tabledef}]
+ [driver spec {:keys [database-name], :as _dbdef} {:keys [table-name], :as _tabledef}]
(let [components (for [component (sql.tx/qualified-name-components driver database-name table-name)]
(ddl.i/format-name driver (u/qualified-name component)))
table-identifier (sql.qp/->honeysql driver (apply hx/identifier :table components))]
- (partial do-insert! driver conn table-identifier)))
+ (partial do-insert! driver spec table-identifier)))
(defn make-load-data-fn
"Create an implementation of `load-data!`. This creates a function to actually insert a row or rows, wraps it with any
@@ -119,12 +119,16 @@
[& insert-middleware-fns]
(let [insert-middleware (apply comp insert-middleware-fns)]
(fn [driver dbdef tabledef]
- (jdbc/with-db-connection [conn (spec/dbdef->spec driver :db dbdef)]
- (.setAutoCommit (jdbc/get-connection conn) false)
- (let [insert! (insert-middleware (make-insert! driver conn dbdef tabledef))
- rows (load-data-get-rows driver dbdef tabledef)]
- (log/tracef "Inserting rows like: %s" (first rows))
- (insert! rows))))))
+ (sql-jdbc.execute/do-with-connection-with-options
+ driver
+ (spec/dbdef->spec driver :db dbdef)
+ {:write? true}
+ (fn [^java.sql.Connection conn]
+ (.setAutoCommit conn false)
+ (let [insert! (insert-middleware (make-insert! driver {:connection conn} dbdef tabledef))
+ rows (load-data-get-rows driver dbdef tabledef)]
+ (log/tracef "Inserting rows like: %s" (first rows))
+ (insert! rows)))))))
;;; ------------------------------------------ Predefinied load-data! impls ------------------------------------------
diff --git a/test/metabase/upload_test.clj b/test/metabase/upload_test.clj
index c45df8b6411c4df5fdf1ddaf7d5f478282dd857c..2678ab76f639e0b84e67f066d35822a6d99f2583 100644
--- a/test/metabase/upload_test.clj
+++ b/test/metabase/upload_test.clj
@@ -16,17 +16,17 @@
[metabase.util :as u]
[toucan2.core :as t2])
(:import
- [java.io File]))
+ (java.io File)))
(set! *warn-on-reflection* true)
-(def bool-type :metabase.upload/boolean)
-(def int-type :metabase.upload/int)
-(def float-type :metabase.upload/float)
-(def vchar-type :metabase.upload/varchar_255)
-(def date-type :metabase.upload/date)
-(def datetime-type :metabase.upload/datetime)
-(def text-type :metabase.upload/text)
+(def ^:private bool-type :metabase.upload/boolean)
+(def ^:private int-type :metabase.upload/int)
+(def ^:private float-type :metabase.upload/float)
+(def ^:private vchar-type :metabase.upload/varchar_255)
+(def ^:private date-type :metabase.upload/date)
+(def ^:private datetime-type :metabase.upload/datetime)
+(def ^:private text-type :metabase.upload/text)
(defn- do-with-mysql-local-infile-activated
"Helper for [[with-mysql-local-infile-activated]]"
@@ -48,7 +48,7 @@
(jdbc/query conn-spec
"set global local_infile = 0"))))))
-(defmacro with-mysql-local-infile-activated
+(defmacro ^:private with-mysql-local-infile-activated
"Turn on local_infile for MySQL"
[& body]
`(do-with-mysql-local-infile-activated (fn [] ~@body)))
@@ -129,7 +129,7 @@
(is (= expected-value
(parser string-value))))))))
-(deftest type-coalescing-test
+(deftest ^:parallel type-coalescing-test
(doseq [[type-a type-b expected] [[bool-type bool-type bool-type]
[bool-type int-type int-type]
[bool-type date-type vchar-type]
@@ -169,7 +169,7 @@
(.write w contents))
csv-file)))
-(deftest detect-schema-test
+(deftest ^:parallel detect-schema-test
(testing "Well-formed CSV file"
(is (= {"name" vchar-type
"age" int-type
@@ -236,7 +236,7 @@
;; comma, but blank column
"Sebulba, 112,"]))))))
-(deftest detect-schema-dates-test
+(deftest ^:parallel detect-schema-dates-test
(testing "Dates"
(is (= {"date" date-type
"not_date" vchar-type