add full-rollback option for production migrations

[Mikulas]

Usecase: running migrations during deploy should not alter the database at all. It is however handy to only rollback the last migration during development, so the original behaviour is retained.

Side effect: migrations are committed at once, not per single migration.
Also affects original continue mode, not just full rollback mode.
Original behaviour could have caused (very edge case) transient problems during deploy if one of the migrations introduced an back incompatible error fixed in another migration queued for execution and an application query was run in that time.

[hrach]

I see it practically useless since alter of database commits the transaction automatically (edit: and immidiately). But I may be missing some point, since I don't get it all.

[Mikulas]

AFAIK nope, pg has great transactions

http://www.postgresql.org/docs/7.4/static/sql-rollback.html
https://wiki.postgresql.org/wiki/Transactional_DDL_in_PostgreSQL:_A_Competitive_Analysis
http://stackoverflow.com/questions/4692690/is-it-possible-to-roll-back-create-table-and-alter-table-statements-in-major-sql

[JanTvrdik]

Why not make rollbacking all migrations a default behavior?

[Mikulas]

It is however handy to only rollback the last migration during development, so the original behaviour is retained.

Also because rolling schema changes is not supported in mysql. I feel safer introducing it as a opt-in feature.

[hrach]

Yes, of course Postgre is about sth another. Personally, I am ok with switch, but I see two important facts:

• migrations & deployment are two different things, assumption, that migrations in productions are connected with deployment is very likely, but they are still two different things.
• we emphasize the difference between mysql & postgresql.

[hrach]

This is probably wrong. It migrations contains more sqls and only the last one is wrong (throws exception), all previous queries are commited, aren't they?

[Mikulas]

I've retained original transactions that wrap single file. This should only affect the new outer one.

[Mikulas]

You are actually right; this won't work with mysql (because it has terrible support for nested transactions). I will have to rewrite this for other than postgres driver which I initially had in mind when implementing this.

[Mikulas]

migrations & deployment are two different things, assumption, that migrations in productions are connected with deployment is very likely, but they are still two different things.

Not sure I get your point. I'm just saying this full rollback would mostly be useful for deployment, but there is no implied connection.

we emphasize the difference between mysql & postgresql.

Fair enough. I'm having troubles correcting tests for mysql anyway, so what if I enabled this mode for postgres only? Throwing during args parsing if driver is mysql and full-rollback is on.

[hrach]

Not sure I get your point. I'm just saying this full rollback would mostly be useful for deployment, but there is no implied connection.

Sorry, I was under impression that '--production' switch in symfony commands will force this option.

[Mikulas]

Ok I believe I'm done. Tests should be passing now; hhvm errored on packagist timeout or something.

[hrach]

I rerun the hhvm buil, we will see :)

[Mikulas]

Packagist:

[Mikulas]

So the tests work now, this pull request seems to introduce no regression. Neither did c184cf1 but it broke hhvm build https://travis-ci.org/nextras/migrations/jobs/71658822

[Mikulas]

Turns out 084163a is not required and the same problem can be fixed by #20

[Mikulas]

I took the liberty on rebasing this on #21 so the tests are passing. (Original ref is 5f61fdc)

[hrach]

@Mikulas feel free to rebase again :)
@JanTvrdik, please review it and merge! I'm ok with this. :)

[hrach]

@Mikulas one other thing: probably you could try to enhace our doc to mention it :))

[hrach]

Btw, http://www.postgresql.org/docs/9.5/static/transaction-iso.html

Important: Some PostgreSQL data types and functions have special rules regarding transactional behavior. In particular, changes made to a sequence (and therefore the counter of a column declared using serial) are immediately visible to all other transactions and are not rolled back if the transaction that made the changes aborts. See Section 9.16 and Section 8.1.4.

[Mikulas]

If I remember the doc correctly, that should only mean that incrementing the sequence is immediately published so no two transactions try to insert two same values. Sequence provides unique values that are not necessarily–contrary to the name–in sequence.

Edit: yeah found it:

http://www.postgresql.org/docs/9.5/static/functions-sequence.html
Important: To avoid blocking concurrent transactions that obtain numbers from the same sequence, a nextval operation is never rolled back; that is, once a value has been fetched it is considered used, even if the transaction that did the nextval later aborts. This means that aborted transactions might leave unused "holes" in the sequence of assigned values.

You are correct that setval might break transactions. It's documented behaviour though so no developer should be surprised if it's not rolled back.

[hrach]

Well, this is vailid also for setval, so the rollback will never be 100% reliable for all possible migrations.

[Mikulas]

Yep I agree, but it's documented, so whoever uses the function should know it'll break. Using setval in migrations seems like bad practice anyway.

Anyway, I still have to write failing tests for mysql and then fix the code. Current implementation relies on nesting transactions which is apparently something mysql can't handle.