Wrapping migrations in transaction prevents use of certain DDL queries

[adamrdavid]

First off thanks for this package! ❤️

Context

Related discussions:

• Atomic migrations? #1059
• Execute migration in transaction #1324

Examples in other ORM migrators:

• https://hexdocs.pm/ecto_sql/Ecto.Migration.html#index/3-migration-file
• https://github.com/ankane/strong_migrations/blob/master/README.md#good-8

Problem

Related to the "atomic migrations" change for postgres #1379

Some postgres DDL statements cannot be run from within a transaction and attempting to do so will result in a postgres error like:
STATEMENT cannot run inside a transaction block

Examples of queries not allowed in a transaction:

• CREATE/DROP INDEX CONCURRENTLY (it is recommended to create indexes on large tables concurrently to avoid blocking writes: https://www.postgresql.org/docs/current/sql-createindex.html#SQL-CREATEINDEX-CONCURRENTLY)
• certain ALTER TABLE sub commands
• ALTER TYPE...ADD VALUE
• CREATE DATABASE

Possible solutions

• use an env var to disable wrapping transaction (example PR: Add env var option to prevent wrapping migrations in a transaction #2046)
• add an arg to up, down, fresh, refresh commands that allow skipping transaction (possibly a combination of this and the env var)
• move transaction to be per migration, use trait implementation to disable transaction on specific migration
  • this doesn't solve the issue of having all migrations run within a transaction, but it could be a separate feature to have each individual migration run in its own transaction (that could be optionally disabled). "Atomic migrations" is a bit ambiguous between "each migration is atomic" vs "each run of all migrations is atomic"

[rustonaut]

To add some relevant context:

• a small amount of commands (e.g. most things with a CONCURRENTLY modifier as they use transactions internally) and a few commands which operate on the whole database system (e.g. CREATE DATABASE) can not be run in a transaction
• most DDL commands work in transactions, but with limitations

E.g. you can add a value to an pg enum using ALTER TYPE...ADD VALUE in a transaction, but the value will not be available until the transaction commits, so doing a data migration or setting it as new default value in the same transaction will fail.

Ignoring ALTER TABLE commands used with a CONCURRENTLY modifier: In the past there also had been various limitations about how you can combine certain alter commands in the same transaction through at least some of them have been removed so I'm not sure how much this still matter.

---

All in all I would in general recommend one-transaction per migration for postgress instead of one transaction over all migrations, with a way to opt out of transaction.

The simplest short term fix would be to remove the pg transaction and let users do their transactions themself.

I think it probably would be best to e.g. add something like a MigrationTrait.should_run_in_migration(&self, backend: DatabaseBackend) -> bool with good defaults. And depending on that use one or no transaction per-migration.

Alternative to returning a bool there could be some more clever mechanisms. E.g. a enum like { MergedTransaction, OwnTransaction, NoTransaction } (where consecutive MergedTransaction transactions will be run in one transaction). But that might be more debugging and error/usage complexity then it's worth. But tbh. MergedTransaction seems like a unnecessary/bad premature optimization. At least until you reach a point where most times you anyway have to instrument or implement the Migrator yourself due to other reasons (e.g. syncing multi step migrations to the database with multi step migrations of code updates).

[rustonaut]

Additionally I would argue it is necessary to take database specific steps to make sure only one migration process is run at the same time. (In addition to using transactions in pg.)

For example if you have load balancing you need that to make sure only one of this instances tries to do the migration. Transactions are not enough for that as with that you still would have multiple instances trying, which can lead to a multitude of issues if migrations are not idempotent (it can even lead to lost or corrupted data, e.g. when doing transformations on a JSON column which can be done twice but shouldn't).

In rust terms it's kinda like the difference between a fancy concurrent data-structure using a Mutex<DB> internally (transaction) and a fancy std::sync::Once.call_once (per-migration, resets when running down of the migration).

Importantly this is possible with all currently supported DBs AFIK.

sqlx's migrator isn't perfect in many ways, but for a relative simple out of the box solution it does this quite well. "Quite well", as in: I don't think you can have a perfect solution without a lot of complexity as it overlaps with code deployment logic but it's approach can bring you surprisingly far when bootstrapping a new application. And would be a good place to start from.

It also would be convenient to have a way to let other code access this primitive as I have as I had used things like that in the past to somewhat synchronize updates to other sub-systems. But that would be a separate discussion.

[kevinfaveri]

Hello guys, is there any way to run concurrent index creation for postgresql at the moment on seaorm?