yogthos / migratus

MIGRATE ALL THE THINGS!
642 stars 93 forks source link

First migrate gets stuck on table-exists? #179

Closed apa512 closed 4 years ago

apa512 commented 4 years ago

I have a confusing issue. It looks like SQLException isn't being caught in table-exists?.

(migratus.core/migrate {:store :database
                        :migration-dir "migrations/"
                        :migration-table-name "migrations"
                        :db {:dbtype "postgresql"
                             :dbname "mydb"
                             :user "myuser"}})

=>

SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
Execution error (PSQLException) at org.postgresql.core.v3.QueryExecutorImpl/receiveErrorResponse (QueryExecutorImpl.java:2578).
ERROR: relation "migrations" does not exist
  Position: 15

Stacktrace:

:cause "ERROR: relation \"migrations\" does not exist\n  Position: 15"
 :via
 [{:type org.postgresql.util.PSQLException
   :message "The database returned ROLLBACK, so the transaction cannot be committed. Transaction failure cause is <<ERROR: relation \"migrations\" does not exist\n  Position: 15>>"
   :at [org.postgresql.core.v3.QueryExecutorImpl processResults "QueryExecutorImpl.java" 2209]}
  {:type org.postgresql.util.PSQLException
   :message "ERROR: relation \"migrations\" does not exist\n  Position: 15"
   :at [org.postgresql.core.v3.QueryExecutorImpl receiveErrorResponse "QueryExecutorImpl.java" 2578]}]
 :trace
 [[org.postgresql.core.v3.QueryExecutorImpl receiveErrorResponse "QueryExecutorImpl.java" 2578]
  [org.postgresql.core.v3.QueryExecutorImpl processResults "QueryExecutorImpl.java" 2313]
  [org.postgresql.core.v3.QueryExecutorImpl execute "QueryExecutorImpl.java" 331]
  [org.postgresql.jdbc.PgStatement executeInternal "PgStatement.java" 448]
  [org.postgresql.jdbc.PgStatement execute "PgStatement.java" 369]
  [org.postgresql.jdbc.PgPreparedStatement executeWithFlags "PgPreparedStatement.java" 159]
  [org.postgresql.jdbc.PgPreparedStatement executeQuery "PgPreparedStatement.java" 109]
  [clojure.java.jdbc$execute_query_with_params invokeStatic "jdbc.clj" 1062]
  [clojure.java.jdbc$execute_query_with_params invoke "jdbc.clj" 1056]
  [clojure.java.jdbc$db_query_with_resultset_STAR_ invokeStatic "jdbc.clj" 1079]
  [clojure.java.jdbc$db_query_with_resultset_STAR_ invoke "jdbc.clj" 1065]
  [clojure.java.jdbc$query invokeStatic "jdbc.clj" 1155]
  [clojure.java.jdbc$query invoke "jdbc.clj" 1117]
  [clojure.java.jdbc$query invokeStatic "jdbc.clj" 1133]
  [clojure.java.jdbc$query invoke "jdbc.clj" 1117]
  [migratus.database$table_exists_QMARK_$fn__4180 invoke "database.clj" 157]
  [clojure.java.jdbc$db_transaction_STAR_ invokeStatic "jdbc.clj" 789]
  [clojure.java.jdbc$db_transaction_STAR_ invoke "jdbc.clj" 759]
  [clojure.java.jdbc$db_transaction_STAR_ invokeStatic "jdbc.clj" 772]
  [clojure.java.jdbc$db_transaction_STAR_ invoke "jdbc.clj" 759]
  [migratus.database$table_exists_QMARK_ invokeStatic "database.clj" 154]
  [migratus.database$table_exists_QMARK_ invoke "database.clj" 145]
  [migratus.database$init_schema_BANG_ invokeStatic "database.clj" 217]
  [migratus.database$init_schema_BANG_ invoke "database.clj" 209]
  [migratus.database.Database connect "database.clj" 270]
  [migratus.core$run invokeStatic "core.clj" 25]
  [migratus.core$run invoke "core.clj" 22]
  [migratus.core$migrate invokeStatic "core.clj" 83]
  [migratus.core$migrate invoke "core.clj" 78]

And in the CLJ REPL:

(migratus.database/table-exists? {:datasource (:db system)} "migrations")

ERROR: relation "migrations" does not exist
  Position: 15

But

(try (migratus.database/table-exists? {:datasource (:db system)} "migrations") (catch java.sql.SQLException _ :hello))
:hello

I really have no idea what to make of this. The only thing I can think of is that this is my first time using Migratus with a deps.edn project.

yogthos commented 4 years ago

Looks like the table hasn't been created before migrate was run, did you run init first to initialize the schema?

apa512 commented 4 years ago

I did not run init. Am I supposed to manually create the migrations table?

yogthos commented 4 years ago

Running the init function will create the migration table.

apa512 commented 4 years ago

@yogthos Are you sure sure? Maybe I'm misreading the code, but it looks to me like init is only supposed to run an optional init.sql while migrate is supposed to create the migrations table if it's missing.

I've done some more digging and what happens for me is that the SQL exception in migratus.database/table-exists? when the migrations table doesn't exist "escapes" to migratus.databse/init-schema! which causes migratus.database/create-migration-table! not to be called.

If I .rollback the transaction before returning false in table-exists? everything works like I expected, i.e. migrate creates my migrations table and runs all the migrations.

yogthos commented 4 years ago

Ah, you're right the table is created by the [init-schema!]((defn init-schema! [db table-name modify-sql-fn]) function, and that's called on connect here, and that in turn is clled in run that's used by migrate.

I'm not able to reproduce the issue on my end, so not sure what's causing it in your case, but If doing a rollback before returning false addresses it then would make sense to add it in. If you'd like to do a PR with your version I can push out

apa512 commented 4 years ago

Something happened in org.postgresql/postgresql {:mvn/version "42.2.11"}. Before this, everything works fine.

apa512 commented 4 years ago

I guess https://github.com/pgjdbc/pgjdbc/pull/1729 is the relevant change.

yogthos commented 4 years ago

Ah, that would do it. So, just have to add db-set-rollback-only! in the transaction then, and that should do the trick?

(sql/with-db-transaction
    [t-con db]
    (try
      (sql/db-set-rollback-only! t-con)
      (sql/query t-con [(str "SELECT 1 FROM " table-name)])
      true
      (catch SQLException _
        false)))
apa512 commented 4 years ago

Yes, this fixes it for me.

yogthos commented 4 years ago

Great, just pushed out 1.2.8 with the fix. Thanks for digging in, and sorry I misled you there at the start. :)

apa512 commented 4 years ago

:+1: Thanks for the quick release