Make it possible to addSql() that is executed as a statement

|      Q       |   A
|------------- | -----------
| Type         | improvement
| BC Break     | no
| Fixed issues | fixes #1325

 #### Summary

When the DBAL connection uses `mysqli` as the driver, prepared statements are sent to the MySQL server through a dedicated protocol mechanism. For example, `CREATE TRIGGER` statements are not possible in this case.

To use SQL statements like `CREATE TRIGGER`, the `DbalExecutor` may not (at least in the case of the `mysqli` driver) use `Connection::executeQuery()`, but has to call `Connection::executeStatement()`. See #1325 for more details.

This PR adds a new `executeAsStatement` parameter to `\Doctrine\Migrations\AbstractMigration::addSql()`, which defaults to `false` (current behaviour). By setting it to true, a migration can pass the information to the `DbalExecutor` that the statement must be executed with `executeStatement()`, not `executeQuery()`.

Author: mpdude

lib/Doctrine/Migrations/AbstractMigration.php

@@ -126,9 +126,10 @@ public function down(Schema $schema): void
     protected function addSql(
         string $sql,
         array $params = [],
-        array $types = []
+        array $types = [],
+        bool $executeAsStatement = false
     ): void {
-        $this->plannedSql[] = new Query($sql, $params, $types);
+        $this->plannedSql[] = new Query($sql, $params, $types, $executeAsStatement);
     }
 
     /** @return Query[] */

lib/Doctrine/Migrations/Query/Query.php

@@ -21,19 +21,22 @@ final class Query
     /** @var mixed[] */
     private array $types;
 
+    private bool $executeAsStatement;
+
     /**
      * @param mixed[] $parameters
      * @param mixed[] $types
      */
-    public function __construct(string $statement, array $parameters = [], array $types = [])
+    public function __construct(string $statement, array $parameters = [], array $types = [], bool $executeAsStatement = false)
     {
         if (count($types) > count($parameters)) {
             throw InvalidArguments::wrongTypesArgumentCount($statement, count($parameters), count($types));
         }
 
-        $this->statement  = $statement;
-        $this->parameters = $parameters;
-        $this->types      = $types;
+        $this->statement          = $statement;
+        $this->parameters         = $parameters;
+        $this->types              = $types;
+        $this->executeAsStatement = $executeAsStatement;
     }
 
     public function __toString(): string
@@ -57,4 +60,9 @@ public function getTypes(): array
     {
         return $this->types;
     }
+
+    public function getExecuteAsStatement(): bool
+    {
+        return $this->executeAsStatement;
+    }
 }

lib/Doctrine/Migrations/Version/DbalExecutor.php

@@ -290,8 +290,13 @@ private function executeResult(MigratorConfiguration $configuration): void
             $this->outputSqlQuery($query, $configuration);
 
             $stopwatchEvent = $this->stopwatch->start('query');
-            // executeQuery() must be used here because $query might return a result set, for instance REPAIR does
-            $this->connection->executeQuery($query->getStatement(), $query->getParameters(), $query->getTypes());
+
+            if ($query->getExecuteAsStatement()) {
+                $this->connection->executeStatement($query->getStatement(), $query->getParameters(), $query->getTypes());
+            } else {
+                $this->connection->executeQuery($query->getStatement(), $query->getParameters(), $query->getTypes());
+            }
+
             $stopwatchEvent->stop();
 
             if (! $configuration->getTimeAllQueries()) {

tests/Doctrine/Migrations/Tests/Version/ExecutorTest.php

@@ -17,6 +17,7 @@
 use Doctrine\Migrations\Query\Query;
 use Doctrine\Migrations\Tests\TestLogger;
 use Doctrine\Migrations\Tests\Version\Fixture\EmptyTestMigration;
+use Doctrine\Migrations\Tests\Version\Fixture\TestMigrationWithStatement;
 use Doctrine\Migrations\Tests\Version\Fixture\VersionExecutorTestMigration;
 use Doctrine\Migrations\Version\DbalExecutor;
 use Doctrine\Migrations\Version\Direction;
@@ -94,6 +95,25 @@ public function testExecuteWithNoQueries(): void
         ], $this->logger->logs);
     }
 
+    public function testExecuteWithStatement(): void
+    {
+        $migratorConfiguration = new MigratorConfiguration();
+
+        $migration = new TestMigrationWithStatement($this->connection, $this->logger);
+        $version   = new Version('xx');
+        $plan      = new MigrationPlan($version, $migration, Direction::UP);
+
+        $this->connection
+            ->expects(self::once())
+            ->method('executeStatement')
+            ->with('CREATE TRIGGER', [], []);
+
+        $this->versionExecutor->execute(
+            $plan,
+            $migratorConfiguration
+        );
+    }
+
     public function testExecuteUp(): void
     {
         $this->metadataStorage
@@ -214,6 +234,7 @@ public function testExecuteDryRun(): void
                 self::assertSame(Direction::UP, $result->getDirection());
 
                 yield new Query('INSERT INTO doctrine_migration_versions (version, executed_at, execution_time) VALUE (' . $result->getVersion() . ', NOW(), 0)');
+                yield new Query('CREATE TRIGGER something...', executeAsStatement: true);
             });
 
         $this->connection
@@ -224,6 +245,10 @@ public function testExecuteDryRun(): void
             ->expects(self::never())
             ->method('executeUpdate');
 
+        $this->connection
+            ->expects(self::never())
+            ->method('executeStatement');
+
         $migratorConfiguration = (new MigratorConfiguration())
             ->setDryRun(true)
             ->setTimeAllQueries(true);
@@ -237,7 +262,7 @@ public function testExecuteDryRun(): void
 
         $queries = $result->getSql();
 
-        self::assertCount(3, $queries);
+        self::assertCount(4, $queries);
         self::assertSame('SELECT 1', $queries[0]->getStatement());
         self::assertSame([1], $queries[0]->getParameters());
         self::assertSame([3], $queries[0]->getTypes());
@@ -250,6 +275,11 @@ public function testExecuteDryRun(): void
         self::assertSame([], $queries[2]->getParameters());
         self::assertSame([], $queries[2]->getTypes());
 
+        self::assertSame('CREATE TRIGGER something...', $queries[3]->getStatement());
+        self::assertSame([], $queries[3]->getParameters());
+        self::assertSame([], $queries[3]->getTypes());
+        self::assertTrue($queries[3]->getExecuteAsStatement());
+
         self::assertNotNull($result->getTime());
         self::assertSame(State::NONE, $result->getState());
         self::assertTrue($this->migration->preUpExecuted);

tests/Doctrine/Migrations/Tests/Version/Fixture/TestMigrationWithStatement.php

@@ -0,0 +1,16 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Migrations\Tests\Version\Fixture;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+class TestMigrationWithStatement extends AbstractMigration
+{
+    public function up(Schema $schema): void
+    {
+        $this->addSql('CREATE TRIGGER', executeAsStatement: true);
+    }
+}