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

Fix SQLite not doing rollback when altering columns fails #4336

Merged

Conversation

nickrum
Copy link
Contributor

@nickrum nickrum commented Mar 3, 2021

The statements produced by the statements producer are currently not executed inside a transaction, which means that the changes are not rolled back if an error occurs. This will be fixed by #4189, but I won't find the time to finish that PR before the 0.95.0 release. Once it's done, this change can be reverted.

@kibertoad
Copy link
Collaborator

@nickrum What's your estimate for #4189? We can potentially delay 0.95.0 if it's somewhere in sight.

@nickrum
Copy link
Contributor Author

nickrum commented Mar 3, 2021

@kibertoad Finishing #4189 is my next top priority, but I will be busy with personal things for at least 1-2 weeks. Implementing the SQL part, modifying the runner to execute queries on a transaction and writing tests will then take at least a few days depending on how things go. So, I don't think it would be worth it to delay the release.

@kibertoad kibertoad merged commit ed0e8a5 into knex:master Mar 3, 2021
@nickrum nickrum deleted the sqlite-alter-columns-rollback-hotfix branch March 3, 2021 18:59
nickrum added a commit to nickrum/knex that referenced this pull request Dec 15, 2021
…ex#4336)"

Reverts knex#4336. Now that knex#4189 has been merged, we can savely revert this PR.
nickrum added a commit to nickrum/knex that referenced this pull request Dec 15, 2021
…ex#4336)"

Reverts knex#4336.
Now that knex#4189 has been merged, we can safely revert this PR.
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 this pull request may close these issues.

None yet

2 participants