migrations icon indicating copy to clipboard operation
migrations copied to clipboard

Slight behaviour change after #1131

Open albe opened this issue 3 years ago • 3 comments

Since the change of #1131 an odd behaviour change has occured to us (when switching from 3.1.0 to 3.1.1 without any further changes): Transaction commit failed because the transaction has been marked for rollback only.

Driver: PDO PGSQL

See https://github.com/doctrine/migrations/pull/1131#issuecomment-803570168 and https://github.com/neos/neos-development-collection/pull/3311#issuecomment-803560353

Summary:

  • when a migration that fails is executed before (auto-migrate) another code executes statements and tries to persist, the commit fails with above error message
  • this only happens with 3.1.1 and only for PDO PGSQL and any PHP version, though the bug that #1131 fixes is only related to PDO MySQL+PHP8
  • hence as of 3.1.1 a failing migration leaves a (rollback) transaction open after invoking Migrator::migrate() (hence DbalMigrator::executeMigrations()), likely because an according commit() is now missing that would have been caught and rolled back

albe avatar Apr 03 '21 09:04 albe

/cc @ostrolucky @greg0ire

Indeed a very edge-case (running auto-migrations is bad and we only did this for automated behaviour testing), and maybe you want to decide to keep it at this. But the change in behaviour is documented here now :)

albe avatar Apr 03 '21 09:04 albe

It's not clear to me how to reproduce this

ostrolucky avatar Apr 03 '21 10:04 ostrolucky

Technically roughly this (I don't know how that would/should look sanely in vanilla - just going into what we are doing beneath our abstraction layer(s)): Run SchemaTool::createSchema() to setup your database with latest schema (but without marking migrations as having executed in the MetadataStorage - yes, this is wrong). Then in your (pseudo)code:

// build $planCalculator and $migrator instances with dependencies
...
try {
    // This will try to execute migrations that fail, because the schema is already up to date and e.g. `ALTER TABLE X CHANGE no_longer_existing_column_in_latest_schema ...` fails
    $migrator->migrate($planCalculator->getPlanUntilVersion('latest'), new MigratorConfiguration());
} catch (DBALException $exception) {
    $dontTruncate = true;
} catch (\PDOException $exception) {
}
// truncate all tables to empty them for next test run, but that shouldn't have an effect to the error case
...
$someEntityRepository->add($myNewTestEntity);
$entityManager->flush();

The important thing is that the migration attempt and the entityManager flush happen within one execution, i.e. there is a left-over transaction still open.

albe avatar Apr 03 '21 10:04 albe