weavejester / ragtime

Database-independent migration library
Eclipse Public License 1.0
610 stars 85 forks source link

migrations aren't transactional #107

Open yogthos opened 8 years ago

yogthos commented 8 years ago

When multiple migrations are placed in a single file using the --;; separator, then you can end up with partially applied migrations. For example, if you have the following migration:

CREATE TABLE foo (id int);
--;;
CREATE TABLE bar id int);

The foo table will be created, then bar table creation statement will cause an exception. However, the first statement will not be rolled back and foo table remains after the migration fails.

weavejester commented 8 years ago

Isn't the solution to this to use a transaction? (For databases that support transactions with CREATE and DROP)

If the migration "up" partially applies, the "down" may also fail because the database isn't in a valid state. The only way I can see of solving this is to either make migrations more fine-grained, or use database transactions.

yogthos commented 8 years ago

The way I handle it in migratus is to break the migration up into a sequence of commands and run those in a transaction as seen here. This way either all statements are applied or the entire transaction is rolled back. The migrations would not be marked as applied in the latter case, and the down migration would not be run.

So, using database transactions is the solution, but I don't think it makes sense to expose this to the users.

danielcompton commented 8 years ago

The trouble as @weavejester has alluded to is that not all DB's support transactional DDL's. e.g. http://stackoverflow.com/questions/4711447/oracle-ddl-and-transaction-rollback http://stackoverflow.com/questions/28197013/transactional-ddl-workflow-for-mysql http://stackoverflow.com/questions/1043598/is-it-possible-to-run-multiple-ddl-statements-inside-a-transaction-within-sql-s

AFAICT Postgres is the only major DB that supports this well. ragtime implicitly running migrations in a transaction probably wouldn't give the guarantees that people expect, except under Postgres?

weavejester commented 8 years ago

Some databases have limitations on what can inside a transaction. For instance, the CREATE TABLE command in MySQL forces the transaction to commit, so it can no longer be rolled back. Postgres has good support for transactional DDL, but not all databases support it comprehensively.

If an exception occurs within a database migration, and that puts the database in an invalid state, then I don't think there's a good solution that works in all cases. A transaction rollback would be ideal, but doesn't work with all databases, and running the "down" script of the migration may fail, as it assumes the the "up" script was applied in full.

weavejester commented 8 years ago

And everything @danielcompton said as well :)

yogthos commented 8 years ago

That makes sense, but wouldn't it be better to attempt to do the transaction as opposed to not. If the database doesn't support transactional DDL you're just back to the default case, so no harm done, or am I missing something? :)

yogthos commented 8 years ago

Also, it looks like really it's mainly a problem with older versions of Oracle and MySQL, while every other DB supports transactional DDLs.

weavejester commented 8 years ago

I seem to recall there was some reason why putting migrations in a transaction caused errors on some databases, but I can't remember exactly what it was. Maybe there isn't any harm to putting everything inside a transaction, but I think we'd have to try it with several databases to be certain.

yogthos commented 8 years ago

That sounds reasonable, FWIW I haven't had any reports of problems in Migratus regarding this.

hypirion commented 6 years ago

Also, for reference: Postgres has only partial support for transactional DDLs, e.g. ALTER TYPE cannot add values inside a transaction.

erez-rabih commented 5 years ago

I would also like to mention that postgresql doesn't support creating index concurrenctly within a transaction:

org.postgresql.util.PSQLException: ERROR: CREATE INDEX CONCURRENTLY cannot run inside a transaction block

Is there a way to run a specific migration outside a transaction?