Make transactions optional

This introduces a new configuration key that allows the user to generate
a method override that disables transaction.

docs/en/explanation/implicit-commits.rst

@@ -0,0 +1,84 @@
+Implicit commits
+================
+
+Since PHP8, if you are using some platforms with some drivers such as
+MySQL with PDO, you may get an error that you did not get before when
+using this library: ``There is no active transaction``. It comes from
+the fact that some platforms like MySQL or Oracle do not support DDL
+statements (``CREATE TABLE``, ``ALTER TABLE``, etc.) in transactions.
+
+The issue existed before PHP 8 but is now made visible by e.g. PDO,
+which now produces the above error message when this library attempts to
+commit a transaction that has already been commited before.
+
+Consider the following migration.
+
+.. code-block:: php
+
+    public function up(Schema $schema): void
+    {
+        $users = [
+            ['name' => 'mike', 'id' => 1],
+            ['name' => 'jwage', 'id' => 2],
+            ['name' => 'ocramius', 'id' => 3],
+        ];
+
+        foreach ($users as $user) {
+            $this->addSql('UPDATE user SET happy = true WHERE name = :name AND id = :id', $user);
+        }
+
+        $this->addSql('CREATE TABLE example_table (id INT AUTO_INCREMENT NOT NULL, title VARCHAR(255) DEFAULT NULL, PRIMARY KEY(id))');
+    }
+
+When you run that migration, what actually happens with some platforms
+is you get the updates inside an implicitly commited transaction, then
+the ``CREATE TABLE`` happens outside that transaction, and then there is
+an attempt to commit an non-existent transaction.
+
+In that sort of situation, if you still wish to get the DML statements
+inside a transaction, we recommend you split the migration in 2
+migrations, as follows.
+
+.. code-block:: php
+
+    final class Version20210401193057 extends AbstractMigration
+    {
+        public function up(Schema $schema): void
+        {
+            $users = [
+                ['name' => 'mike', 'id' => 1],
+                ['name' => 'jwage', 'id' => 2],
+                ['name' => 'ocramius', 'id' => 3],
+            ];
+
+            foreach ($users as $user) {
+                $this->addSql('UPDATE user SET happy = true WHERE name = :name AND id = :id', $user);
+            }
+        }
+    }
+
+    final class Version20210401193058 extends AbstractMigration
+    {
+        public function up(Schema $schema): void
+        {
+            $this->addSql('CREATE TABLE example_table (id INT AUTO_INCREMENT NOT NULL, title VARCHAR(255) DEFAULT NULL, PRIMARY KEY(id))');
+        }
+
+        public function isTransactional(): bool
+        {
+            return false;
+        }
+    }
+
+Please refer to the manual of your database platform to know if you need
+to do this or not.
+
+At the moment, this library checks if there is an active transaction
+before commiting it, which means you should not encouter the error
+described above. It will not be the case in the next major version
+though, and you should prepare for that.
+
+To help you deal with this issue, the library features a configuration
+key called ``transactional``. Setting it to ``false`` will cause new
+migrations to be generated with the override method above, making new
+migrations non-transactional by default.

docs/en/reference/configuration.rst

@@ -38,6 +38,7 @@ Now, in the root of your project place a file named ``migrations.php``, ``migrat
             ],
 
             'all_or_nothing' => true,
+            'transactional' => true,
             'check_database_platform' => true,
             'organize_migrations' => 'none',
             'connection' => null,
@@ -58,6 +59,7 @@ Now, in the root of your project place a file named ``migrations.php``, ``migrat
            'MyProject\Component\Migrations': ./Component/MyProject/Migrations
 
         all_or_nothing: true
+        transactional: true
         check_database_platform: true
         organize_migrations: none
 
@@ -90,6 +92,7 @@ Now, in the root of your project place a file named ``migrations.php``, ``migrat
             </migrations-paths>
 
             <all-or-nothing>true</all-or-nothing>
+            <transactional>true</transactional>
 
             <check-database-platform>true</check-database-platform>
             <organize_migrations>none</organize_migrations>
@@ -112,6 +115,7 @@ Now, in the root of your project place a file named ``migrations.php``, ``migrat
             },
 
             "all_or_nothing": true,
+            "transactional": true,
             "check_database_platform": true,
             "organize_migrations": "none",
 
@@ -136,6 +140,9 @@ Here are details about what each configuration option does:
 +----------------------------+------------+------------------------------+----------------------------------------------------------------------------------+
 | all_or_nothing             | no         | false                        | Whether or not to wrap multiple migrations in a single transaction.              |
 +----------------------------+------------+------------------------------+----------------------------------------------------------------------------------+
+| transactional              | no         | true                         | Whether or not to wrap migrations in a single transaction.                       |
+|                            |            |                              |                                                                                  |
++----------------------------+------------+------------------------------+----------------------------------------------------------------------------------+
 | migrations                 | no         | []                           | Manually specify the array of migration versions instead of finding migrations.  |
 +----------------------------+------------+------------------------------+----------------------------------------------------------------------------------+
 | check_database_platform    | no         | true                         | Whether to add a database platform check at the beginning of the generated code. |
@@ -226,6 +233,13 @@ All or Nothing Transaction
 When using the ``all_or_nothing`` option, multiple migrations ran at the same time will be wrapped in a single
 transaction. If one migration fails, all migrations will be rolled back
 
+Using or not using transactions
+-------------------------------
+
+By default, migrations are transactional, meaning code in a migration
+is wrapped in a transaction.
+Setting ``transactional`` to ``false`` will disable that.
+
 From the Command Line
 ~~~~~~~~~~~~~~~~~~~~~
 

docs/en/reference/migration-classes.rst

@@ -75,7 +75,8 @@ Override this method if you want to disable transactions in a migration. It defa
     statement, and before running it. Make sure to read the manual of
     your database platform to know what is actually happening.
     ``isTransactional()`` does not guarantee that statements are wrapped
-    in a single transaction.
+    in a single transaction. To learn more about this, read the
+    :ref:`dedicated explanation <../explanation/implicit-commits>`.
 
 getDescription
 ~~~~~~~~~~~~~~

lib/Doctrine/Migrations/Configuration/Configuration.php

@@ -41,6 +41,9 @@ final class Configuration
     /** @var bool */
     private $allOrNothing = false;
 
+    /** @var bool */
+    private $transactional = true;
+
     /** @var string|null */
     private $connectionName;
 
@@ -193,6 +196,17 @@ public function isAllOrNothing(): bool
         return $this->allOrNothing;
     }
 
+    public function setTransactional(bool $transactional): void
+    {
+        $this->assertNotFrozen();
+        $this->transactional = $transactional;
+    }
+
+    public function isTransactional(): bool
+    {
+        return $this->transactional;
+    }
+
     public function setCheckDatabasePlatform(bool $checkDbPlatform): void
     {
         $this->checkDbPlatform = $checkDbPlatform;

lib/Doctrine/Migrations/Configuration/Migration/ConfigurationArray.php

@@ -61,6 +61,9 @@ public function getConfiguration(): Configuration
             'all_or_nothing' => static function ($value, Configuration $configuration): void {
                 $configuration->setAllOrNothing(is_bool($value) ? $value : BooleanStringFormatter::toBoolean($value, false));
             },
+            'transactional' => static function ($value, Configuration $configuration): void {
+                $configuration->setAllOrNothing(is_bool($value) ? $value : BooleanStringFormatter::toBoolean($value, true));
+            },
             'check_database_platform' =>  static function ($value, Configuration $configuration): void {
                 $configuration->setCheckDatabasePlatform(is_bool($value) ? $value : BooleanStringFormatter::toBoolean($value, false));
             },

lib/Doctrine/Migrations/Configuration/Migration/XmlFile.php

@@ -47,6 +47,13 @@ public function getConfiguration(): Configuration
             );
         }
 
+        if (isset($config['transactional'])) {
+            $config['transactional'] = BooleanStringFormatter::toBoolean(
+                $config['transactional'],
+                true
+            );
+        }
+
         if (isset($config['migrations_paths'])) {
             $config['migrations_paths'] = $this->getDirectoriesRelativeToFile(
                 $config['migrations_paths'],

lib/Doctrine/Migrations/Generator/Generator.php

@@ -58,7 +58,7 @@ public function down(Schema $schema): void
     {
         // this down() migration is auto-generated, please modify it to your needs
 <down>
-    }
+    }<override>
 }
 
 TEMPLATE;
@@ -98,6 +98,15 @@ public function generateMigration(
             '<className>' => $className,
             '<up>' => $up !== null ? '        ' . implode("\n        ", explode("\n", $up)) : null,
             '<down>' => $down !== null ? '        ' . implode("\n        ", explode("\n", $down)) : null,
+            '<override>' => $this->configuration->isTransactional() ? '' : <<<'METHOD'
+
+
+    public function isTransactional(): bool
+    {
+        return false;
+    }
+METHOD
+        ,
         ];
 
         $code = strtr($this->getTemplate(), $replacements);

tests/Doctrine/Migrations/Tests/Configuration/_files/config.json

@@ -10,5 +10,6 @@
         "executed_at_column_name" : "doctrine_migration_executed_at_column_test",
         "execution_time_column_name" : "doctrine_migration_execution_time_column_test"
     },
-    "all_or_nothing"            : true
+    "all_or_nothing"            : true,
+    "transactional"             : true
 }

tests/Doctrine/Migrations/Tests/Configuration/_files/config.php

@@ -15,5 +15,6 @@
     'migrations'      => ['Foo', 'Bar'],
 
     'all_or_nothing'            => true,
+    'transactional'             => true,
     'check_database_platform'   => false,
 ];

tests/Doctrine/Migrations/Tests/Configuration/_files/config.yml

@@ -10,5 +10,6 @@ migrations_paths:
   DoctrineMigrationsTest: .
 migrations : ["Foo", "Bar"]
 all_or_nothing: true
+transactional: true
 check_database_platform: true
 

tests/Doctrine/Migrations/Tests/Generator/GeneratorTest.php

@@ -4,10 +4,14 @@
 
 namespace Doctrine\Migrations\Tests\Generator;
 
+use Doctrine\DBAL\Connection;
 use Doctrine\Migrations\Configuration\Configuration;
 use Doctrine\Migrations\Generator\Generator;
 use InvalidArgumentException;
 use PHPUnit\Framework\TestCase;
+use Psr\Log\LoggerInterface;
+use Test\VersionNonTransactional1234;
+use Test\VersionNonTransactional1235;
 
 use function class_exists;
 use function file_get_contents;
@@ -95,6 +99,49 @@ public function testCustomTemplateThrowsInvalidArgumentExceptionWhenTemplateEmpt
         unlink($customTemplate);
     }
 
+    public function testItCanGenerateNonTransactionalMigrations(): void
+    {
+        $this->configuration->setTransactional(false);
+        $path = $this->migrationGenerator->generateMigration(
+            'Test\\VersionNonTransactional1234',
+            '// up',
+            '// down'
+        );
+
+        self::assertFileExists($path);
+
+        include $path;
+
+        self::assertTrue(class_exists('Test\VersionNonTransactional1234'));
+        self::assertFalse((new VersionNonTransactional1234(
+            $this->createStub(Connection::class),
+            $this->createStub(LoggerInterface::class)
+        ))->isTransactional());
+
+        unlink($path);
+    }
+
+    public function testItCanGenerateTransactionalMigrationsByDefault(): void
+    {
+        $path = $this->migrationGenerator->generateMigration(
+            'Test\\VersionNonTransactional1235',
+            '// up',
+            '// down'
+        );
+
+        self::assertFileExists($path);
+
+        include $path;
+
+        self::assertTrue(class_exists('Test\VersionNonTransactional1235'));
+        self::assertTrue((new VersionNonTransactional1235(
+            $this->createStub(Connection::class),
+            $this->createStub(LoggerInterface::class)
+        ))->isTransactional());
+
+        unlink($path);
+    }
+
     protected function setUp(): void
     {
         $this->configuration = new Configuration();