sqlx
sqlx copied to clipboard
launchbadge
Option to run migrations without a transaction
As far as my understanding goes, all migration scripts are currently run within a SQL transaction. This creates issue in Postgres when trying to change a type (e.g. add a variant to an enum):
error: while executing migrations:
error returned from database:
ALTER TYPE ... ADD cannot run inside a transaction block
I'd say there are two ways forward:
- make it the user responsibility to wrap its migration query in a transaction, if it makes sense;
- add a way to "disable" transaction wrapping to the CLI.
What do you think?
When pinning the version of Postgres to 12/13 I fail to reproduce the issue. I closed it until I can add enough details for you to reproduce :+1:
Upon closer investigation, it only happens with Postgres 11 or earlier: https://stackoverflow.com/a/53150621/5861287
I could see something like sqlx migrate run --no-transaction (-n for short, maybe?)
Or perhaps disabling transactions per-file with -- SQLX_NO_TRANSACTION at the start of the file?
A per-file solution would be ideal - it might be undesirable to disable transaction guarantees for all migrations while it is actually needed only for one.
A comment could be a good solution, although moves the migration scripts away from being pure SQL.
An alternative would be to automatically add a BEGIN TRANSACTION and COMMIT to the up.sql template generated by sqlx migrate add which the user can choose to keep or remove.
A comment could be a good solution, although moves the migration scripts away from being pure SQL.
I don't think it's an issue since that's a valid SQL comment.
An alternative would be to automatically add a
BEGIN TRANSACTIONandCOMMITto the up.sql template generated by sqlx migrate add which the user can choose to keep or remove.
The problem is that would silently cause existing migrations in the wild to not run in a transaction, which would have unpredictable results. Someone might also erase those without realizing that they probably want it to be in a transaction.
I guess we could have --no-transaction be an option for sqlx migrate add that adds -- SQLX_NO_TRANSACTION to the generated file. We're already talking about having a sqlx-config.json or something like that which can configure defaults for things like this.
I don't think it's an issue since that's a valid SQL comment.
It is indeed a valid SQL comment, but it actually does stuff, so it's not really a comment :stuck_out_tongue: I think it's a viable strategy, just wanted to stress that out.
The problem is that would silently cause existing migrations in the wild to not run in a transaction, which would have unpredictable results. Someone might also erase those without realizing that they probably want it to be in a transaction. I guess we could have --no-transaction be an option for sqlx migrate add that adds -- SQLX_NO_TRANSACTION to the generated file. We're already talking about having a sqlx-config.json or something like that which can configure defaults for things like this.
Sounds good to me.
Do you need help implementing this?
I appreciate this issue has been open a while with no activity, but just chiming in with an alternative approach which I think has advantages over the options being discussed.
FlyWay's approach to managing these cases is to by default run scripts within a transaction, but to allow a per-script opt out. The opt out is configured via a per-script config file (named identically to the script, but with an alternative extension). This would be backward compatible, doesn't rely on meaningful comments, and open to extension should further per-script configuration requirements arise in the future.
I like the comment in the migration approach. It keeps things simpler, makes it obvious when looking at the migration that it's meant to not be run in a transaction, and makes the no-transaction flag part of the migration checksum.
There's already a precedent for meaningful comments, the shebang (#!) in shell scripts. It's really not that surprising of a concept.
I don't really like the idea of having this information in a separate file just because that's another file you have to look at to understand the actual semantics of the migration.
As another option, why not make it part of the filename? Like we already have .up and .down for reversible migrations, what about like .no_tx or something like that?
<version>_<snake_case_name>[.up | .down][.no_tx].sql
I think I would prefer the comment though, just to avoid piling too much semantics onto filenames, especially since IDEs tend to abbreviate longer filenames and so it might be easy to miss.
Just to add more cases where this is "known". Liquibase uses this to control various aspects in their "plain SQL changelog files". It does work quite well and i consider this a not so bad solution :)
e.g.
--preconditions onFail:HALT onError:HALT
--precondition-sql-check expectedResult:0 SELECT COUNT(*) FROM my_table
https://docs.liquibase.com/concepts/basic/sql-format.html
Upon closer investigation, it only happens with Postgres 11 or earlier: https://stackoverflow.com/a/53150621/5861287
I see this error using the official Docker image of PG 15.2
I also ran into this issue while creating a materialized view, so would be great to have a way to opt out of the transaction wrap:
error: while executing migrations: error returned from database: CREATE MATERIALIZED VIEW ... WITH DATA cannot run inside a transaction block
e: From a "user" point of view, I quite like the proposed -- SQLX_NO_TRANSACTION comment -- it's right there in the file, relatively self-explanatory, and easy to add more context as my own comment before/after if needed
I am new to SQLX/Postgres and Rust but experiencing the problem trying to create a materialized view and was thinking about maybe helping solve this problem... I see that the apply fn is starting the transaction in the postgres driver
Is this an issue only for Postgres?
If so, I wouldn't mind opening a PR to add the no_tx metadata to the Migration struct, driven from the filename as @abonander mentioned in his comment
In short, something like this:
-
Figure out how to update the
MigrationSourcearound here to pick up the.no_txand -
add it to the Migration struct sqlx-core/src/migrate/migration.rs:
#[derive(Debug, Clone)]
pub struct Migration {
...
pub no_tx: boolean
}
And finally updating the driver as mentioned above.
