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

mysql: if migration sets "SET autocommit=0", final update of version# doesn't happen #49

Open
binary1230 opened this issue Jul 14, 2023 · 5 comments

Comments

@binary1230
Copy link

binary1230 commented Jul 14, 2023

this was admittedly a little silly but:

I had a migration like this:

-- upgrade from version 4 to version 5
SET autocommit = 0;
BEGIN;

# (for this bug, the actual statement doesnt matter)
ALTER TABLE `some_table` ADD `whatever` ....... blah .....;

COMMIT;

That was the last migration in a series of migrations (say going from version 3 to version 5).

The migration above succeeded, and I expected that the "migration_version" table would look like this at the end:
status="complete", version=5

but instead I was seeing this:
status="partial up", version=5

What was happening is because "autocommit" was being turned off by the migration SQL, the byjg/migration library executed the UPDATE statement to set it to "complete", version=5, but never explicitly committed the transaction, so, it was never actually written to the DB.

So, the version number was left in the last state it was in previously which was "partial up", version=5

I realize it's a bit silly to use "autocommit=0" when I'm also doing explicit BEGIN/COMMIT to mark the start/end of a tranasction. I've removed that from my SQL scripts.

but, I feel like having the migration library explicitly commit after updating the table is a good idea anyway just in case your users do something silly, like what I did : ) perhaps adding $this->getDbDriver()->commitTransaction(); in the setVersion() function (and maybe in other places)

Love the library, thanks!

*Edited to fix one detail about the version#

@binary1230
Copy link
Author

forgot to mention ver# is

    "byjg/migration": "4.2.3",

probably not latest, I can try with that

@byjg
Copy link
Owner

byjg commented Jul 14, 2023

I believe introducing the auto-commit = 0 in the migration files causes secondary issues on the migration control table. The migration_version seems never be updated in the last step.

The ideal is the transaction control to be on the migration library side. What I can do is to create an optional parameter allowing the migration library controls or not the transaction.

byjg added a commit that referenced this issue Jul 15, 2023
@byjg
Copy link
Owner

byjg commented Jul 15, 2023

I looked into this carefully and I implemented the transaction control in the Migration class. This way I can control the process and status.

You need to use the version 4.9.x-dev for now, since I didn't close this version yet. The 4.9 version, has a breaking change in the registerDatabase method.

Avoiding Partial Migration

A partial migration is when the migration script is interrupted in the middle of the process due to an error or a manual interruption.

The migration table will be with the status partial up or partial down and it needs to be fixed manually before be able to migrate again.

To avoid this situation, you can specify the migration will be run in a transactional context.
If the migration script fails, the transaction will be rolled back and the migration table will be marked as complete and
the version will be the immediately previous version before the script that causes the error.

To enable this feature, you need to call the method withTransactionEnabled passing true as parameter:

<?php
$migration->withTransactionEnabled(true);

byjg added a commit that referenced this issue Jul 19, 2023
@byjg
Copy link
Owner

byjg commented Jul 19, 2023

Hello, after further tests with MySQL I am regret to say that MySQL doesn't support transactions for DDL commands (create table, alter table,etc)

So, both my implementation, and if you SET autocommit = 0 + Begin Transaction in the script will not work with MySQL, unfortunately.

Here some additional information:

Sorry for that.

@binary1230
Copy link
Author

binary1230 commented Jul 19, 2023 via email

@byjg byjg mentioned this issue Jan 5, 2024
Merged
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

No branches or pull requests

2 participants