Skip to content
Snippets Groups Projects
Unverified Commit 53f1c87c authored by Cam Saul's avatar Cam Saul
Browse files

No need to run JDBC queries in futures since async-wait already does that

parent 296e8561
Branches
Tags
No related merge requests found
......@@ -163,7 +163,8 @@
`(do-with-ensured-connection ~db (fn [~conn-binding] ~@body)))
(defn- cancelable-run-query
"Runs `sql` in such a way that it can be interrupted via a `future-cancel`"
"Runs JDBC query, canceling it if an InterruptedException is caught (e.g. if there query is canceled before
finishing)."
[db sql params opts]
(with-ensured-connection [conn db]
;; This is normally done for us by java.jdbc as a result of our `jdbc/query` call
......@@ -172,20 +173,8 @@
;; (Not all drivers support this so ignore Exceptions if they don't)
(u/ignore-exceptions
(.closeOnCompletion stmt))
;; Need to run the query in another thread so that this thread can cancel it if need be
(try
;; This thread is interruptable because it's awaiting the other thread (the one actually running the query).
;; Interrupting this thread means that the client has disconnected (or we're shutting down) and so we can give
;; up on the query running in the future
(let [query-future (future
(try
(jdbc/query conn (into [stmt] params) opts)
(catch Throwable e
e)))
result @query-future]
(if (instance? Throwable result)
(throw result)
result))
(jdbc/query conn (into [stmt] params) opts)
(catch InterruptedException e
(log/warn (tru "Client closed connection, canceling query"))
;; This is what does the real work of canceling the query. We aren't checking the result of
......@@ -193,7 +182,7 @@
(.cancel stmt)
(throw e))
(catch Exception e
(.cancel stmt)
(u/ignore-exceptions (.cancel stmt))
e)))))
(defn- run-query
......
......@@ -4,7 +4,9 @@
[medley.core :as m]
[metabase.query-processor :as qp]
[metabase.query-processor.interface :as qp.i]
[metabase.test.data :as data]))
[metabase.test.data :as data]
[metabase.util.schema :as su]
[schema.core :as s]))
(defn- bad-query []
{:database (data/id)
......@@ -27,12 +29,13 @@
"LIMIT 1048576")
:params nil})
(def ^:private ^{:arglists '([stacktrace])} valid-stacktrace? (every-pred seq (partial every? (every-pred string? seq))))
(def ^:private ^{:arglists '([stacktrace])} valid-stacktrace?
(complement (partial s/check [su/NonBlankString])))
;; running a bad query via `process-query` should return stacktrace, query, preprocessed query, and native query
(expect
{:status :failed
:class java.util.concurrent.ExecutionException
:class Exception
:error true
:stacktrace true
;; `:database` is removed by the catch-exceptions middleware for historical reasons
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment