-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Use DELETE FROM instead of TRUNCATE for MySQL #656
Use DELETE FROM instead of TRUNCATE for MySQL #656
Conversation
The first PR mentioned above accidentally rearranged imports. This PR only changes the one line necessary. I'd be happy to see this merged soon as this is a critical bug. It nearly did damage to one of my databases. |
Pull Request Test Coverage Report for Build 1560897359
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per #584 (comment), the transaction needs to be serializable
This PR fixes a very dangerous bug: If the migration process gets killed at the wrong moment, it leaves the version table empty (the truncate was successful, but the insert was interrupted). Next thing that happens is that the tool tries to reapply all the migrations from the beginning, which can be disastrous. I can mark it as serializable. But why do you think it changes anything? According to https://dev.mysql.com/doc/refman/5.7/en/innodb-transaction-isolation-levels.html |
Please do. For safety, we should use the most strict isolation level. There's nearly no downside and setting the migration version doesn't need to be performant. e.g. we're happy to pay for any overhead between |
@antigremlin Can you rebase your branch on base branch and update the PR? 🙏🏿 |
My apologies. Was going to do that and didn't notice the PR is already merged. |
Resolves issue #584
In MySQL, TRUNCATE statements cannot be rolled back, at least for 5.7:
This is another attempt at fixing, as the previous PR #585 has stalled. The credit goes to @martinarrieta