Merge branch '3.5.x' into 4.0.x

UPGRADE.md

@@ -840,6 +840,13 @@ Relying on the availability of the `LOCATE()` on SQLite deprecated. SQLite does
 but the function `INSTR()` can be a drop-in replacement in most situations. Use
 `AbstractPlatform::getLocateExpression()` if you need a portable solution.
 
+## Deprecated `SchemaDiff::toSql()` and `SchemaDiff::toSaveSql()`
+
+Using `SchemaDiff::toSql()` to generate SQL representing the diff has been deprecated.
+Use `AbstractPlatform::getAlterSchemaSQL()` instead.
+
+`SchemaDiff::toSaveSql()` has been deprecated without a replacement.
+
 ## Deprecated `SchemaDiff::$orphanedForeignKeys`
 
 Relying on the schema diff tracking foreign keys referencing the tables that have been dropped is deprecated.
@@ -940,6 +947,36 @@ The "unique" and "check" column properties have been deprecated. Use unique cons
 Relying on the default precision and scale of decimal columns provided by the DBAL is deprecated.
 When declaring decimal columns, specify the precision and scale explicitly.
 
+## Deprecated `Comparator::diffTable()` method.
+
+The `Comparator::diffTable()` method has been deprecated in favor of `Comparator::compareTables()`
+and `TableDiff::isEmpty()`.
+
+Instead of having to check whether the diff is equal to the boolean `false`, you can optionally check
+if the returned table diff is empty.
+
+### Before
+
+```php
+$diff = $comparator->diffTable($oldTable, $newTable);
+
+// mandatory check
+if ($diff !== false) {
+    // we have a diff
+}
+```
+
+### After
+
+```php
+$diff = $comparator->compareTables($oldTable, $newTable);
+
+// optional check
+if (! $diff->isEmpty()) {
+    // we have a diff
+}
+```
+
 ## Deprecated not passing `$fromColumn` to the `TableDiff` constructor.
 
 Not passing `$fromColumn` to the `TableDiff` constructor has been deprecated.

psalm.xml.dist

@@ -39,6 +39,14 @@
                     See https://github.com/doctrine/dbal/pull/4317
                 -->
                 <file name="tests/Functional/LegacyAPITest.php"/>
+                <!--
+                    TODO: remove in 4.0.0
+                -->
+                <referencedMethod name="Doctrine\DBAL\Schema\SchemaDiff::toSql"/>
+                <!--
+                    TODO: remove in 4.0.0
+                -->
+                <referencedMethod name="Doctrine\DBAL\Schema\Comparator::diffTable"/>
             </errorLevel>
         </DeprecatedMethod>
         <DocblockTypeContradiction>

src/Platforms/AbstractPlatform.php

@@ -32,6 +32,7 @@
 use Doctrine\DBAL\Schema\ForeignKeyConstraint;
 use Doctrine\DBAL\Schema\Identifier;
 use Doctrine\DBAL\Schema\Index;
+use Doctrine\DBAL\Schema\SchemaDiff;
 use Doctrine\DBAL\Schema\Sequence;
 use Doctrine\DBAL\Schema\Table;
 use Doctrine\DBAL\Schema\TableDiff;
@@ -1097,6 +1098,16 @@ public function getCreateTemporaryTableSnippetSQL(): string
         return 'CREATE TEMPORARY TABLE';
     }
 
+    /**
+     * Generates SQL statements that can be used to apply the diff.
+     *
+     * @return list<string>
+     */
+    public function getAlterSchemaSQL(SchemaDiff $diff): array
+    {
+        return $diff->toSql($this);
+    }
+
     /**
      * Returns the SQL to create a sequence on this platform.
      */
@@ -1134,7 +1145,11 @@ public function getCreateIndexSQL(Index $index, string $table): string
         $columns = $index->getColumns();
 
         if (count($columns) === 0) {
-            throw new InvalidArgumentException('Incomplete definition. "columns" required.');
+            throw new InvalidArgumentException(sprintf(
+                'Incomplete or invalid index definition %s on table %s',
+                $name,
+                $table,
+            ));
         }
 
         if ($index->isPrimary()) {

src/Schema/AbstractSchemaManager.php

@@ -598,7 +598,7 @@ public function dropSchemaObjects(Schema $schema): void
      */
     public function alterSchema(SchemaDiff $schemaDiff): void
     {
-        $this->executeStatements($schemaDiff->toSql($this->platform));
+        $this->executeStatements($this->platform->getAlterSchemaSQL($schemaDiff));
     }
 
     /**

src/Schema/Comparator.php

@@ -6,6 +6,7 @@
 
 use Doctrine\DBAL\Exception;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\Deprecations\Deprecation;
 
 use function array_map;
 use function assert;
@@ -144,12 +145,35 @@ public function diffSequence(Sequence $sequence1, Sequence $sequence2): bool
      *
      * If there are no differences this method returns null.
      *
+     * @deprecated Use {@see compareTables()} and, optionally, {@see TableDiff::isEmpty()} instead.
+     *
      * @throws Exception
      */
     public function diffTable(Table $oldTable, Table $newTable): ?TableDiff
     {
-        $hasChanges = false;
+        Deprecation::triggerIfCalledFromOutside(
+            'doctrine/dbal',
+            'https://github.com/doctrine/dbal/pull/5770',
+            '%s is deprecated. Use compareTables() instead.',
+            __METHOD__,
+        );
+
+        $diff = $this->compareTables($oldTable, $newTable);
+
+        if ($diff->isEmpty()) {
+            return null;
+        }
 
+        return $diff;
+    }
+
+    /**
+     * Compares the tables and returns the difference between them.
+     *
+     * @throws Exception
+     */
+    public function compareTables(Table $oldTable, Table $newTable): TableDiff
+    {
         $addedColumns        = [];
         $modifiedColumns     = [];
         $droppedColumns      = [];
@@ -172,8 +196,6 @@ public function diffTable(Table $oldTable, Table $newTable): ?TableDiff
             }
 
             $addedColumns[$newColumnName] = $newColumn;
-
-            $hasChanges = true;
         }
 
         // See if there are any removed columns in the new table
@@ -184,7 +206,6 @@ public function diffTable(Table $oldTable, Table $newTable): ?TableDiff
             if (! $newTable->hasColumn($oldColumnName)) {
                 $droppedColumns[$oldColumnName] = $oldColumn;
 
-                $hasChanges = true;
                 continue;
             }
 
@@ -195,8 +216,6 @@ public function diffTable(Table $oldTable, Table $newTable): ?TableDiff
             }
 
             $modifiedColumns[] = new ColumnDiff($oldColumn, $newColumn);
-
-            $hasChanges = true;
         }
 
         $renamedColumns = $this->detectRenamedColumns($addedColumns, $droppedColumns);
@@ -211,8 +230,6 @@ public function diffTable(Table $oldTable, Table $newTable): ?TableDiff
             }
 
             $addedIndexes[$newIndexName] = $newIndex;
-
-            $hasChanges = true;
         }
 
         // See if there are any removed indexes in the new table
@@ -224,7 +241,6 @@ public function diffTable(Table $oldTable, Table $newTable): ?TableDiff
             ) {
                 $droppedIndexes[$oldIndexName] = $oldIndex;
 
-                $hasChanges = true;
                 continue;
             }
 
@@ -237,8 +253,6 @@ public function diffTable(Table $oldTable, Table $newTable): ?TableDiff
             }
 
             $modifiedIndexes[] = $newIndex;
-
-            $hasChanges = true;
         }
 
         $renamedIndexes = $this->detectRenamedIndexes($addedIndexes, $droppedIndexes);
@@ -254,7 +268,6 @@ public function diffTable(Table $oldTable, Table $newTable): ?TableDiff
                     if (strtolower($oldForeignKey->getName()) === strtolower($newForeignKey->getName())) {
                         $modifiedForeignKeys[] = $newForeignKey;
 
-                        $hasChanges = true;
                         unset($oldForeignKeys[$oldKey], $newForeignKeys[$newKey]);
                     }
                 }
@@ -263,18 +276,10 @@ public function diffTable(Table $oldTable, Table $newTable): ?TableDiff
 
         foreach ($oldForeignKeys as $oldForeignKey) {
             $droppedForeignKeys[] = $oldForeignKey;
-
-            $hasChanges = true;
         }
 
         foreach ($newForeignKeys as $newForeignKey) {
             $addedForeignKeys[] = $newForeignKey;
-
-            $hasChanges = true;
-        }
-
-        if (! $hasChanges) {
-            return null;
         }
 
         return new TableDiff(

src/Schema/SchemaDiff.php

@@ -5,15 +5,21 @@
 namespace Doctrine\DBAL\Schema;
 
 use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\Deprecations\Deprecation;
 
+use function array_filter;
 use function array_merge;
 use function array_values;
+use function count;
 
 /**
  * Differences between two schemas.
  */
 class SchemaDiff
 {
+    /** @var array<TableDiff> */
+    private readonly array $alteredTables;
+
     /**
      * Constructs an SchemaDiff object.
      *
@@ -32,12 +38,15 @@ public function __construct(
         private readonly array $createdSchemas,
         private readonly array $droppedSchemas,
         private readonly array $createdTables,
-        private readonly array $alteredTables,
+        array $alteredTables,
         private readonly array $droppedTables,
         private readonly array $createdSequences,
         private readonly array $alteredSequences,
         private readonly array $droppedSequences,
     ) {
+        $this->alteredTables = array_filter($alteredTables, static function (TableDiff $diff): bool {
+            return ! $diff->isEmpty();
+        });
     }
 
     /** @return array<string> */
@@ -88,6 +97,21 @@ public function getDroppedSequences(): array
         return $this->droppedSequences;
     }
 
+    /**
+     * Returns whether the diff is empty (contains no changes).
+     */
+    public function isEmpty(): bool
+    {
+        return count($this->createdSchemas) === 0
+            && count($this->droppedSchemas) === 0
+            && count($this->createdTables) === 0
+            && count($this->alteredTables) === 0
+            && count($this->droppedTables) === 0
+            && count($this->createdSequences) === 0
+            && count($this->alteredSequences) === 0
+            && count($this->droppedSequences) === 0;
+    }
+
     /**
      * The to save sql mode ensures that the following things don't happen:
      *
@@ -97,16 +121,36 @@ public function getDroppedSequences(): array
      *
      * This way it is ensured that assets are deleted which might not be relevant to the metadata schema at all.
      *
+     * @deprecated
+     *
      * @return list<string>
      */
     public function toSaveSql(AbstractPlatform $platform): array
     {
+        Deprecation::trigger(
+            'doctrine/dbal',
+            'https://github.com/doctrine/dbal/pull/5766',
+            '%s is deprecated.',
+            __METHOD__,
+        );
+
         return $this->_toSql($platform, true);
     }
 
-    /** @return list<string> */
+    /**
+     * @deprecated Use {@link AbstractPlatform::getAlterSchemaSQL()} instead.
+     *
+     * @return list<string>
+     */
     public function toSql(AbstractPlatform $platform): array
     {
+        Deprecation::triggerIfCalledFromOutside(
+            'doctrine/dbal',
+            'https://github.com/doctrine/dbal/pull/5766',
+            '%s is deprecated. Use AbstractPlatform::getAlterSchemaSQL() instead.',
+            __METHOD__,
+        );
+
         return $this->_toSql($platform, false);
     }
 

src/Schema/TableDiff.php

@@ -5,6 +5,7 @@
 namespace Doctrine\DBAL\Schema;
 
 use function array_filter;
+use function count;
 
 /**
  * Table Diff.
@@ -142,4 +143,22 @@ public function getDroppedForeignKeys(): array
     {
         return $this->droppedForeignKeys;
     }
+
+    /**
+     * Returns whether the diff is empty (contains no changes).
+     */
+    public function isEmpty(): bool
+    {
+        return count($this->addedColumns) === 0
+            && count($this->modifiedColumns) === 0
+            && count($this->droppedColumns) === 0
+            && count($this->renamedColumns) === 0
+            && count($this->addedIndexes) === 0
+            && count($this->modifiedIndexes) === 0
+            && count($this->droppedIndexes) === 0
+            && count($this->renamedIndexes) === 0
+            && count($this->addedForeignKeys) === 0
+            && count($this->modifiedForeignKeys) === 0
+            && count($this->droppedForeignKeys) === 0;
+    }
 }

tests/Functional/Schema/SchemaManagerFunctionalTestCase.php

@@ -680,13 +680,15 @@ public function testAlterTableScenario(): void
 
     public function testTableInNamespace(): void
     {
-        if (! $this->connection->getDatabasePlatform()->supportsSchemas()) {
+        $platform = $this->connection->getDatabasePlatform();
+
+        if (! $platform->supportsSchemas()) {
             self::markTestSkipped('Schema definition is not supported by this platform.');
         }
 
         $diff = new SchemaDiff(['testschema'], [], [], [], [], [], [], []);
 
-        foreach ($diff->toSql($this->connection->getDatabasePlatform()) as $sql) {
+        foreach ($platform->getAlterSchemaSQL($diff) as $sql) {
             $this->connection->executeStatement($sql);
         }