migrations: DDL migrations produce errors on PHP 8.0 + MySQL in transactional mode

Just do it like pierres as a workaround until its fixed. Add following code into your migrations.

class Version**** extends AbstractMigration
{

    public function up(Schema $schema): void
    {
        // your migration
    }

    public function isTransactional(): bool
    {
        return false;
    }
}

“doctrine/doctrine-migrations-bundle”:“3.1.1”, “doctrine/migrations”: "3.2.1

This works for me,

downgrade doctrine/doctrine-migrations-bundle from 3.2.0 to 3.1.1 and doctrine/migrations from 3.3.0 to 3.2.1

on php8 & MySQL 8.0.20

So AFAIS up to PHP 7.4 such a migration will execute roughly as if it were isTransactional() == false (only roughly because everything up to the first DDL will be within a transaction), because the failing commit was silently ignored. In PHP8 however the manual commit will now throw a PDOException, which totally breaks the transaction and stops following migrations in the execution plan from running. This is a major PITA for all projects using doctrine migrations and trying to support PHP 8, as it can not easily be handled by them automatically (catching the exception there will still have later migrations not executed). This means all existing migrations need to be changed and own users need to be advised to change their migrations.

In this case, I’m with the OP and the right solution IMO would be to create a BUGFIX that catches this very specific PDOException and ignores it to create b/c for now. Then a “sane” solution to solve this through proper API can be discussed with all the time needed.

        if ($migration->isTransactional()) {
            //commit only if running in transactional mode
            try {
                $this->connection->commit();
            } catch (\PDOException $e) {
                // FIXME: Workaround for PHP 8 and PDO mysql b/c, see https://github.com/doctrine/migrations/issues/1104
                if (PHP_MAJOR_VERSION < 8 || $e->getMessage() !== 'There is no active transaction') {
                    throw $e;
                }
            }
        }

Is this an anti-pattern? Pretty sure. Would I want to do something like this regularly? Nope. Is this solving a pain? Yep. Dirty as heck, but it is what it is.

Keep in mind that a migration does not necessarily contain DDL statements. A migration that only consists of INSERT and UPDATE statements can very well be transactional.

Hi! The easiest solution is to provide needed version in composer.json

"doctrine/doctrine-migrations-bundle":"3.1.1", "doctrine/migrations": "3.2.1",

Transactions works correctly with php 8.0 + MySql 5.7

See linked issues and pull requests. I don’t know why do you guys comment on closed issue…

And I have opened https://github.com/doctrine/migrations/pull/1131 for fixing PHP 8.0 + MySQL issue.

Is this an anti-pattern? Pretty sure. Would I want to do something like this regularly? Nope. Is this solving a pain? Yep. Dirty as heck, but it is what it is.

It’s ugly, but I like it! 👍

Following the doc : https://www.doctrine-project.org/projects/doctrine-migrations/en/3.3/reference/configuration.html Disable globally transactional by setting it to false for migrations solve that case.

If you are using Symfony and doctrine/doctrine-migrations-bundle 3.2.x, follow the doc : https://symfony.com/bundles/DoctrineMigrationsBundle/current/index.html To disable transactional from config/packages/doctrine_migrations.yaml

Link to the 3.1 PR in case anyone is interested: https://github.com/doctrine/dbal/pull/4481

What’s your opinion on the first solution? Too risky? It does change the behavior existing migrations, and would technically be a BC-break for that reason I suppose, although one could argue that the behavior was not the expected one in the first place.

I would propose to have AbstractMigration::isTransactional() defaulting to $platform->supportsTransactionalDDL() (adding that method in DBAL if it does not exist yet).

For platforms supporting transactional DDL, this is the best default (if you make a mistake in your migration, it avoids having to clean a state of a half-executed migration). But default to always transactional indeed does not look good anymore on mysql due to the magic it does…

@digitalkaoz what fix? There is no fix, just don’t attempt to use DDL + transactions with MySQL or Oracle, they do not support it.

@ostrolucky I wonder if you actually read the comments before making your remark. Because if you did, you would understand that ‘linked issues and pull requests’ had nothing to do with the problem discussed. Oh and probably you would also understand why we comment on a closed issue.

any fixes for php 8? weird things but all_or_nothing doesn’t help still using isTransactional() return false

“doctrine/doctrine-migrations-bundle”:“3.1.1”, “doctrine/migrations”: "3.2.1

This works for me,

downgrade doctrine/doctrine-migrations-bundle from 3.2.0 to 3.1.1 and doctrine/migrations from 3.3.0 to 3.2.1

on php8 & MySQL 8.0.20

Same setup, can confirm, this works

I’d rather not upgrade to 3 yet, it’s not perfect when using multiple entityManagers (at least i haven’t found a good way to make it work yet). But your solution might work for others. Thanks either way.

I had also updated doctrine/migrations and doctrine/doctrine-migrations-bundle to their latest versions, but started to receive this error again. Turned out that it was caused by a Connection wrapper which isn’t handled correctly by Doctrine/Migrations/Tools/TransactionHelper.php. See https://github.com/getsentry/sentry-symfony/issues/488, https://github.com/doctrine/migrations/pull/1149 and https://github.com/doctrine/dbal/issues/4616.

@yyaremenko my bad this time 😃 I’ve meant doctrine/doctrine-migrations-bundle:^3.1.1. As of this moment, this should install doctrine/migrations:3.1.2.

1. Remove composer.lock <-- important

@yyaremenko blindly recommending to remove composer.lock could lead to issues with other packages that get upgraded. Why not composer require doctrine/migrations:^3.1.1?

I see. If we are going to do this I think we should also fix this paragraph of the docs. We should probably add a warning similar to this one (but in proper english 😉), and tell people to refer to the docs of their RDBMS to know what might actually be happening.

Yeah one thing after another. People were delusioned for a long long time that they get some kind of transaction protection during DDL statements in MySQL, this is not the right ticket to solve it in. By the way, Oracle also doesn’t support it and possibly other databases. For now we just need to make migrations work with MySQL again. That’s why I plan to submit a patch today or tomorrow according solution in https://github.com/doctrine/migrations/issues/1104#issuecomment-786311346 so we could already have fix which makes migrations on PHP 8 with most popular RDBMS work again.

until it’s fixed in DBAL

Given the answer you got here, I’m starting to have some doubts about this.

Regarding https://github.com/doctrine/migrations/issues/1104#issuecomment-786311346 , it has the merit of not being coupled at all to the DBAL, unlike instanceof MySQLPlatform, so there’s that. What’s very wrong with it is that it hides the fact that the migration is not wrapped in a transaction to our users and that despite the fact isTransactional() returns true. That has always been the case, and it’s only surfacing now that an exception is thrown, but that kind of shushing sure doesn’t feel right.

Maybe it should be part of the configuration? Meaning possible values for transactional_by_default could become:

• true
• tentatively or mostly or silence_failures
• false

That way people that use MySQL + PDO or Oracle or whatever doesn’t work can have migrations that works without deluding themselves. We could even print a message in the output indicating whether the migration ended up being transactional or not in this kind of case.

Another workaround is to switch to mysqli driver instead of pdo_mysql, as only pdo_mysql triggers this exception, as can be seen in https://github.com/doctrine/dbal/pull/4544

@morozov I am not sure the DBAL could solve the issue, but it would definitely help to have inTransaction() added to the API of Connection. For systems that don’t have an equivalent to PDO::inTransaction(), it could still be implemented by doing some bookkeeping regarding the number of calls to beginTransaction(), commit() and rollBack().

In v2, this package has 4 occurrences of commit(), all of which probably need to be complexified to account for this MySQL “feature”. Also, there are 4 occurrences of rollBack(), and surely, calling rollBack() if there is no active transaction would fail, e.g. in this (unlikely, I concede that) scenario:

try {
    $connection->beginTransaction();
    $connection->executeSomeSQLWithDDLStatements(); // works fine and auto-commits
    callUnrelatedToDatabaseThatThrows();
} catch (Throwable $e) {
    $connection->rollBack();
}

Alternate solution to adding inTransaction to the API would be to add an argument to commit and rollBack that would make the check to inTransaction implicit, or to add a new API like maybeCommit / maybeRollback.

All of my proposals would constitue BC-breaks in the DBAL though, so we would still have to complexify code here as a first step I’m afraid.

Also, none of them feel really good to me, maybe you will have better ideas?

Yes, instanceof \PDO is also working, just checked it. I think this approach is better.