refinery icon indicating copy to clipboard operation
refinery copied to clipboard

Add ability to run migrations outside of a transaction

Open mschmo opened this issue 2 years ago • 5 comments

This would be useful (and needed) for certain commands that cannot be ran within a transaction, such as CREATE INDEX CONCURRENTLY in postgres.

This would ideally be something that could configured per migration file, similar to how goose has the ability to mark migration files with a -- +goose NO TRANSACTION comment or how Flyway provides a executeInTransaction setting in script config files.

mschmo avatar May 15 '23 23:05 mschmo

Hi, and thanks for your interest! I wasn't aware of that, interesting. Currently Transaction is one of the core traits. We can rename it to Execution and either have a boolean transaction flag for the execute message. Are you interested in tackling that? thanks

jxs avatar May 20 '23 17:05 jxs

Hey @jxs - I'd definitely be interested in that! Thank you for the starting points about the Transaction trait. That makes sense for the point where we can execute the migration either within or outside of a transaction and I'll start working on implementing that.

As far as how a user would configure that setting on a migration SQL file, do you have any preferences to the design/approach? I saw in the README that refinery's design is based on Flyway, so should we use the same approach via script config files? Such that each migration .sql file could have an optional .sql.conf file with an executeInTransaction setting (default to true).

mschmo avatar May 23 '23 05:05 mschmo

I'd go for goose's way of doing it, and having special formatted comments like: -- +refinery NO TRANSACTION.

We can do it parsing the migration file. Ideally we could use Migration for that, thing is we should probably first split Migration into AppliedMigration and UnappliedMigration that we can there compare with the PartialEq. We can then when creating an UnappliedMigration that parses the input &str line by line.

As this will most likely decrease performance, I'd also add a flag on the UnappliedMigration constructor to disable this, that we can then leverage via Runner What do you think?

jxs avatar May 23 '23 14:05 jxs