Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sqlite: Migrations are always wrapped in implicit transactions #346

Closed
saj opened this issue Feb 21, 2020 · 2 comments · Fixed by #350
Closed

sqlite: Migrations are always wrapped in implicit transactions #346

saj opened this issue Feb 21, 2020 · 2 comments · Fixed by #350

Comments

@saj
Copy link
Contributor

saj commented Feb 21, 2020

Contrary to the documentation and maintainer notes in the bug tracker -- see here and here, for example -- the migrate database driver for SQLite does, in fact, wrap migration statements in an implicit transaction.

These implicit transactions are usually not a problem in SQLite. Unlike other SQL implementations, DDL statements in SQLite can execute just fine within a transaction. (Actually, in SQLite, a DDL statement that is executed outside a transaction will automatically start a new transaction. All DDL statements execute within a transaction.) However, these implicit transactions do cause problems in other areas.

Users may be confused to find that their own BEGIN statements are met with errors. A transaction may not be started within a transaction. This is not a significant problem in and of itself. The documentation could simply be amended to direct SQLite users to avoid their own BEGIN and COMMIT statements.

The big, and perhaps insurmountable, problem is that certain necessary SQLite PRAGMA statements cannot be executed within a transaction. Due to limitations in SQLite's ALTER TABLE implementation, users often need to selectively toggle the enforcement of foreign key constraints when pivoting between tables. In wrapping all migrations with an implicit transaction, the migrate tool has snapped this essential ability away from the user.

One potential workaround would be for users to COMMIT the implicit transaction then BEGIN an explicit transaction within a migration. I do not know whether this is safe. To cite one source, the authors of go-database-sql.org have gone out of their way to caution users against mixing Tx methods with explicit BEGIN/COMMIT statements.

I am unsure whether current behaviour was intentional. Unfortunately, I don't think it is possible to change this behaviour without breaking backwards compatibility. Any users that are currently employing the workaround described in the previous paragraph would be broken by the removal of the implicit transactions.

Something for v5? :-)


Migrate Version
v4.7.1

Loaded Source Drivers
file

Loaded Database Drivers
sqlite3

Go Version
go version go1.13.8 darwin/amd64

@dhui
Copy link
Member

dhui commented Feb 25, 2020

Thanks for the detailed report @saj!

To maintain backwards compatibility, we could add a config flag to the SQLite DB driver to disable the wrapping of migrations in a transaction. Maybe call the config flag NoTxWrap (and x-no-tx-wrap when specified via URI). Then in a later backwards incompatible version of migrate, we could replace this flag with the inverse (named TxWrap and x-tx-wrap accordingly).

We should also SQLite DB driver README accordingly

@saj
Copy link
Contributor Author

saj commented Feb 28, 2020

That'll work quite nicely. Patch in #350.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants