Enhancement: Wrap each migration file in an atomic transaction

[mcandre]

Could migration scripts be wrapped in db.BeginTx() ... db.Rollback/db.Commit(), to make it easier to resolve any errors that occur during migrations?

This could help keep migrations in a clean state, even when a migration fails! Rather than having to clean up the innards of the golang-migrate state, the user can simply fix the migrations and run them again until they pass.

https://golang.org/pkg/database/sql/#Conn.BeginTx

[dhui]

This would be an interesting change to support in the driver. (See: #14)
However, not every RDBMS supports transactions for DDL/schema changes. e.g. MySQL I'm looking at you...
This is an issue since some migrations modify schema and others modify data.

For now, migrate errs on the side of flexibility.

[TVenuMadhav]

Golang Migrate Version 1 to Version 4

Are all migration files run in a transaction implicitly(from a black box perspective)?

if yes, then why do we have to explicitly mention (Begin/commit) in migration files with multiple sql statements?

Thanks in advance,

PS :- working with Postgres here.

[dhui]

Are all migration files run in a transaction implicitly(from a black box perspective)?

No. That's what this issue/feature request is about. See: https://github.com/golang-migrate/migrate/blob/master/MIGRATIONS.md#migration-content-format

if yes, then why do we have to explicitly mention (Begin/commit) in migration files with multiple sql statements?

It's a best practice. If your migration contains multiple statements, a failure in a single statement will rollback the whole migration which makes it easier to re-apply the whole migration after the issue has been resolved.

An issue w/ this feature request is that it's not backwards compatible. e.g. existing migrations using with BEGIN/COMMIT will break

[tv42]

I find it really hard to believe this is something the user is supposed to remember every single time they write a migration. I'm personally going to look for an alternate solution now, I really don't trust myself to remember that.

[dhui]

Other options:

1. Fork the DB driver and wrap your statement in a transaction in your fork
2. Implement this as an option for each database. As this issue is currently written, it implies this change would take affect for all migrations instead of being DB specific

See also: #374

[u007]

i think it could be roled out progressively one by one driver

[maddsua]

I see that this might not be possible for every DB that gomigrate supports, however I think most of the SQL DBs do. That said, if a DB supports transactions, having gomigrate use them automatically would be a great idea.

I nearly submitted a bug report regarding this behavior but then realized that it simply doesn't put transactions there for you.

So, why not just add a simple logic like this:

...
if driver.supports_tx() and migration.not_contains_tx() then
    migration = wrap_tx(migration)
...

This way if a user DOES NOT use transactions in their migrations for the supported DBs gomigrate can wrap it for them thus preventing potential foot-shootings.

[mcandre]

might as well fork