diff --git a/src/metabase/db.clj b/src/metabase/db.clj
index e8cd3d0ca9e31f77c9fc50b44544318f638789f9..cb837d12d4e28cb0bda9ecc746e4cbdea5d39d84 100644
--- a/src/metabase/db.clj
+++ b/src/metabase/db.clj
@@ -24,7 +24,10 @@
liquibase.database.jvm.JdbcConnection
liquibase.resource.ClassLoaderResourceAccessor))
-;; ## DB FILE, JDBC/KORMA DEFINITONS
+
+;;; +------------------------------------------------------------------------------------------------------------------------+
+;;; | DB FILE & CONNECTION DETAILS |
+;;; +------------------------------------------------------------------------------------------------------------------------+
(def db-file
"Path to our H2 DB file from env var or app config."
@@ -88,49 +91,148 @@
(defn jdbc-details
"Takes our own MB details map and formats them properly for connection details for JDBC."
- [db-details]
- {:pre [(map? db-details)]}
- ;; TODO: it's probably a good idea to put some more validation here and be really strict about what's in `db-details`
- (case (:type db-details)
- :h2 (dbspec/h2 db-details)
- :mysql (dbspec/mysql (assoc db-details :db (:dbname db-details)))
- :postgres (dbspec/postgres (assoc db-details :db (:dbname db-details)))))
+ ([]
+ (jdbc-details @db-connection-details))
+ ([db-details]
+ {:pre [(map? db-details)]}
+ ;; TODO: it's probably a good idea to put some more validation here and be really strict about what's in `db-details`
+ (case (:type db-details)
+ :h2 (dbspec/h2 db-details)
+ :mysql (dbspec/mysql (assoc db-details :db (:dbname db-details)))
+ :postgres (dbspec/postgres (assoc db-details :db (:dbname db-details))))))
-;; ## MIGRATE
+;;; +------------------------------------------------------------------------------------------------------------------------+
+;;; | MIGRATE |
+;;; +------------------------------------------------------------------------------------------------------------------------+
-(def ^:private ^:const ^String changelog-file
- "migrations/liquibase.json")
+(def ^:private ^:const ^String changelog-file "migrations/liquibase.json")
+
+(defn- migrations-sql
+ "Return a string of SQL containing the DDL statements needed to perform unran LIQUIBASE migrations."
+ ^String [^Liquibase liquibase]
+ (let [writer (StringWriter.)]
+ (.update liquibase "" writer)
+ (.toString writer)))
+
+(defn- migrations-lines
+ "Return a sequnce of DDL statements that should be used to perform migrations for LIQUIBASE.
+
+ MySQL gets snippy if we try to run the entire DB migration as one single string; it seems to only like it if we run one statement at a time;
+ Liquibase puts each DDL statement on its own line automatically so just split by lines and filter out blank / comment lines. Even though this
+ is not neccesary for H2 or Postgres go ahead and do it anyway because it keeps the code simple and doesn't make a significant performance difference."
+ [^Liquibase liquibase]
+ (for [line (s/split-lines (migrations-sql liquibase))
+ :when (not (or (s/blank? line)
+ (re-find #"^--" line)))]
+ line))
+
+(defn- has-unran-migrations?
+ "Does LIQUIBASE have migration change sets that haven't been ran yet?
+
+ It's a good idea to Check to make sure there's actually something to do before running `(migrate :up)` because `migrations-sql` will
+ always contain SQL to create and release migration locks, which is both slightly dangerous and a waste of time when we won't be using them."
+ ^Boolean [^Liquibase liquibase]
+ (boolean (seq (.listUnrunChangeSets liquibase nil))))
+
+(defn- has-migration-lock?
+ "Is a migration lock in place for LIQUIBASE?"
+ ^Boolean [^Liquibase liquibase]
+ (boolean (seq (.listLocks liquibase))))
+
+(defn- wait-for-migration-lock-to-be-cleared
+ "Check and make sure the database isn't locked. If it is, sleep for 2 seconds and then retry several times.
+ There's a chance the lock will end up clearing up so we can run migrations normally."
+ [^Liquibase liquibase]
+ (u/auto-retry 5
+ (when (has-migration-lock? liquibase)
+ (Thread/sleep 2000)
+ (throw (Exception. "Database has migration lock; cannot run migrations. You can force-release these locks by running `java -jar metabase.jar migrate release-locks`.")))))
+
+(defn- migrate-up-if-needed
+ "Run any unran LIQUIBASE migrations, if needed.
+
+ This creates SQL for the migrations to be performed, then executes each DDL statement.
+ Running `.update` directly doesn't seem to work as we'd expect; it ends up commiting the changes made and they can't be rolled back at
+ the end of the transaction block. Converting the migration to SQL string and running that via `jdbc/execute!` seems to do the trick."
+ [conn, ^Liquibase liquibase]
+ (when (has-unran-migrations? liquibase)
+ (wait-for-migration-lock-to-be-cleared liquibase)
+ (doseq [line (migrations-lines liquibase)]
+ (jdbc/execute! conn [line]))))
+
+(defn- force-migrate-up-if-needed
+ "Force migrating up. This does two things differently from `migrate-up-if-needed`:
+
+ 1. This doesn't check to make sure the DB locks are cleared
+ 2. Any DDL statements that fail are ignored
+
+ It can be used to fix situations where the database got into a weird state, as was common before the fixes made in #3295.
+
+ Each DDL statement is ran inside a nested transaction; that way if the nested transaction fails we can roll it back without rolling back the entirety of changes
+ that were made. (If a single statement in a transaction fails you can't do anything futher until you clear the error state by doing something like calling `.rollback`.)"
+ [conn, ^Liquibase liquibase]
+ (when (has-unran-migrations? liquibase)
+ (doseq [line (migrations-lines liquibase)]
+ (log/info line)
+ (jdbc/with-db-transaction [nested-transaction-connection conn]
+ (try (jdbc/execute! nested-transaction-connection [line])
+ (log/info (u/format-color 'green "[SUCCESS]"))
+ (catch Throwable e
+ (.rollback (jdbc/get-connection nested-transaction-connection))
+ (log/error (u/format-color 'red "[ERROR] %s" (.getMessage e)))))))))
+
+(defn- conn->liquibase
+ "Get a `Liquibase` object from JDBC CONN."
+ (^Liquibase []
+ (conn->liquibase (jdbc-details)))
+ (^Liquibase [conn]
+ (let [^JdbcConnection liquibase-conn (JdbcConnection. (jdbc/get-connection conn))
+ ^Database database (.findCorrectDatabaseImplementation (DatabaseFactory/getInstance) liquibase-conn)]
+ (Liquibase. changelog-file (ClassLoaderResourceAccessor.) database))))
(defn migrate
- "Migrate the database (this can also be ran via command line like `lein run migrate down-one`):
+ "Migrate the database (this can also be ran via command line like `java -jar metabase.jar migrate up` or `lein run migrate up`):
* `:up` - Migrate up
+ * `:force` - Force migrate up, ignoring locks and any DDL statements that fail.
* `:down` - Rollback *all* migrations
* `:down-one` - Rollback a single migration
* `:print` - Just print the SQL for running the migrations, don't actually run them.
* `:release-locks` - Manually release migration locks left by an earlier failed migration.
- (This shouldn't be necessary now that we run migrations inside a transaction,
- but is available just in case)."
+ (This shouldn't be necessary now that we run migrations inside a transaction, but is available just in case)."
+ ([]
+ (migrate :up))
([direction]
(migrate @db-connection-details direction))
([db-details direction]
(jdbc/with-db-transaction [conn (jdbc-details db-details)]
- (let [^Database database (-> (DatabaseFactory/getInstance)
- (.findCorrectDatabaseImplementation (JdbcConnection. (jdbc/get-connection conn))))
- ^Liquibase liquibase (Liquibase. changelog-file (ClassLoaderResourceAccessor.) database)]
- (case direction
- :up (.update liquibase "")
- :down (.rollback liquibase 10000 "")
- :down-one (.rollback liquibase 1 "")
- :print (let [writer (StringWriter.)]
- (.update liquibase "" writer)
- (.toString writer))
- :release-locks (.forceReleaseLocks liquibase))))))
+ ;; Tell transaction to automatically `.rollback` instead of `.commit` when the transaction finishes
+ (jdbc/db-set-rollback-only! conn)
+ ;; Disable auto-commit. This should already be off but set it just to be safe
+ (.setAutoCommit (jdbc/get-connection conn) false)
+ ;; Set up liquibase and let it do its thing
+ (try
+ (let [liquibase (conn->liquibase conn)]
+ (case direction
+ :up (migrate-up-if-needed conn liquibase)
+ :force (force-migrate-up-if-needed conn liquibase)
+ :down (.rollback liquibase 10000 "")
+ :down-one (.rollback liquibase 1 "")
+ :print (println (migrations-sql liquibase))
+ :release-locks (.forceReleaseLocks liquibase)))
+ ;; Migrations were successful; disable rollback-only so `.commit` will be called instead of `.rollback`
+ (jdbc/db-unset-rollback-only! conn)
+ :done
+ ;; If for any reason any part of the migrations fail then rollback all changes
+ (catch Throwable e
+ ;; This should already be happening as a result of `db-set-rollback-only!` but running it twice won't hurt so better safe than sorry
+ (.rollback (jdbc/get-connection conn))
+ (throw e))))))
;;; +------------------------------------------------------------------------------------------------------------------------+
-;;; | DB CONNECTION / TRANSACTION STUFF |
+;;; | CONNECTION POOLS & TRANSACTION STUFF |
;;; +------------------------------------------------------------------------------------------------------------------------+
(defn connection-pool
@@ -193,7 +295,7 @@
;;; +------------------------------------------------------------------------------------------------------------------------+
-;;; | DB SETUP & MIGRATIONS |
+;;; | DB SETUP |
;;; +------------------------------------------------------------------------------------------------------------------------+
diff --git a/src/metabase/driver/generic_sql/query_processor.clj b/src/metabase/driver/generic_sql/query_processor.clj
index bff34c1ee46c09acc3030fde05d1354c5c033191..4e84e8fc58bf16aa4b5bbab4ab73c85cb2724a05 100644
--- a/src/metabase/driver/generic_sql/query_processor.clj
+++ b/src/metabase/driver/generic_sql/query_processor.clj
@@ -227,14 +227,14 @@
[sql]
(when sql
(-> sql
- (s/replace #"\sFROM" "\nFROM")
+ (s/replace #"\sFROM" "\nFROM")
(s/replace #"\sLEFT JOIN" "\nLEFT JOIN")
- (s/replace #"\sWHERE" "\nWHERE")
- (s/replace #"\sGROUP BY" "\nGROUP BY")
- (s/replace #"\sORDER BY" "\nORDER BY")
- (s/replace #"\sLIMIT" "\nLIMIT")
- (s/replace #"\sAND\s" "\n AND ")
- (s/replace #"\sOR\s" "\n OR "))))
+ (s/replace #"\sWHERE" "\nWHERE")
+ (s/replace #"\sGROUP BY" "\nGROUP BY")
+ (s/replace #"\sORDER BY" "\nORDER BY")
+ (s/replace #"\sLIMIT" "\nLIMIT")
+ (s/replace #"\sAND\s" "\n AND ")
+ (s/replace #"\sOR\s" "\n OR "))))
;; TODO - make this a protocol method ?
@@ -304,13 +304,15 @@
(throw (Exception. (exception->nice-error-message e))))))
(defn- do-with-auto-commit-disabled
- "Disable auto-commit for this transaction, that way shady queries are unable to modify the database; execute F in a try-finally block.
- In the `finally`, rollback any changes made during this transaction just to be extra-double-sure JDBC doesn't try to commit them automatically for us."
+ "Disable auto-commit for this transaction, and make the transaction `rollback-only`, which means when the transaction finishes `.rollback` will be called instead of `.commit`.
+ Furthermore, execute F in a try-finally block; in the `finally`, manually call `.rollback` just to be extra-double-sure JDBC any changes made by the transaction aren't committed."
{:style/indent 1}
- [{^java.sql.Connection connection :connection}, f]
- (.setAutoCommit connection false)
+ [conn f]
+ (jdbc/db-set-rollback-only! conn)
+ (.setAutoCommit (jdbc/get-connection conn) false)
+ ;; TODO - it would be nice if we could also `.setReadOnly` on the transaction as well, but that breaks setting the timezone. Is there some way we can have our cake and eat it too?
(try (f)
- (finally (.rollback connection))))
+ (finally (.rollback (jdbc/get-connection conn)))))
(defn- do-in-transaction [connection f]
(jdbc/with-db-transaction [transaction-connection connection]