doctrine1 icon indicating copy to clipboard operation
doctrine1 copied to clipboard

Database Migration does set current version in DB

Open JohannesTyra opened this issue 2 years ago • 6 comments

If u run php symfony doctrine:migrate the tash will so the migration_version table to the current version.

That does not happen anymore. The value (version) does not change.

Its in the Doctrine/Migration.php: https://github.com/FriendsOfSymfony1/doctrine1/blob/master/lib/Doctrine/Migration.php#L350

                $this->_connection->commit();
                $this->setCurrentVersion($to);

Does not work anymore.

I changed it to:

              $this->setCurrentVersion($to);
              $this->_connection->commit();
                

That works!

JohannesTyra avatar Feb 21 '23 09:02 JohannesTyra

After $this->_connection->commit(); the method $this->setCurrentVersion($to); will be not be executed.

JohannesTyra avatar Feb 21 '23 09:02 JohannesTyra

Exec throws an error $this->_connection->commit();

>> doctrine  Migrating from version 113 to 114
>> doctrine  executing migration : 114, class: Version114


PDOException: There is no active transaction in vendor/friendsofsymfony1/doctrine1/lib/Doctrine/Transaction.php:417
Stack trace:
#0 vendor/friendsofsymfony1/doctrine1/lib/Doctrine/Transaction.php(417): PDO->commit()
#1 vendor/friendsofsymfony1/doctrine1/lib/Doctrine/Transaction.php(279): Doctrine_Transaction->_doCommit()
#2 vendor/friendsofsymfony1/doctrine1/lib/Doctrine/Connection.php(1450): Doctrine_Transaction->commit()
#3 vendor/friendsofsymfony1/doctrine1/lib/Doctrine/Migration.php(351): Doctrine_Connection->commit()

JohannesTyra avatar Feb 21 '23 09:02 JohannesTyra

That works:


                try {
                  $this->_connection->commit();
                } catch (Exception $e){
                  echo $e;
                }
                
                $this->setCurrentVersion($to);

JohannesTyra avatar Feb 21 '23 09:02 JohannesTyra

That works:

>                 try {
                  $this->_connection->commit();
                } catch (Exception $e){
                  echo $e;
                }
                
                $this->setCurrentVersion($to);

That would set the new version even if the migration failed I guess :thinking:

thePanz avatar Feb 21 '23 16:02 thePanz

@thePanz It was just a quick fix to get it work ;)

The question is: Why this happens: "PDOException: There is no active transaction in xxx". What change causes this?

JohannesTyra avatar Feb 22 '23 08:02 JohannesTyra

This error happens in PHP 8+. See Implicit Commits in PHP8.

The problem happens in Migration.php, method "migrate()" (rows 319 to 355). Specifically, row 319 starts a transaction, then does migrations by calling $this->_doMigrate($to) in row 328. However, since this calls ALTER TABLE in each migration, the transaction is closed, which means that by the time our code reaches row 350 to $this->_connection->commit(), we no longer have a transaction to commit and our code explodes, never setting the version in the database.

There are several solutions to this problem, depending on how one wants to approach it, but I believe it would be the cleanest to check if a transaction exists after migration and if not, to start a new one, essentially replicating what happens implicitly in PHP 5-7. For example:

diff --git a/lib/Doctrine/Migration.php b/lib/Doctrine/Migration.php
index 615c792ec..1d8b0c742 100644
--- a/lib/Doctrine/Migration.php
+++ b/lib/Doctrine/Migration.php
@@ -326,6 +326,9 @@ class Doctrine_Migration
             }
 
             $this->_doMigrate($to);
+            if (!$this->_connection->getDbh()->inTransaction()) {
+                $this->_connection->beginTransaction();
+            }
         } catch (Exception $e) {
             $this->addError($e);
         }

Note that this means we could perform one migration successfully, fail the second one and be stuck with an incorrect migration version. However, this is already the case, as PHP 5-7 already do this. The correct approach would be to alter the tables in a migration, then update the version, then start the next migration, which would require a refactoring of this method.

Side note, this bug may most obviously appear in migrations, but could also happen otherwise, for example if our code calls to create a new table in the database on the fly.

agaluf avatar Dec 14 '23 13:12 agaluf