Merge pull request #5663 from morozov/deprecate-table-diff-new-name

Deprecate renaming tables via TableDiff
doctrine · Sep 18, 2022 · 01e7f90 · 01e7f90
2 parents a4a784a + 217ce8d
commit 01e7f90
Show file tree

Hide file tree

Showing 11 changed files with 135 additions and 22 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -17,6 +17,14 @@ 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 renaming tables via `TableDiff` and `AbstractPlatform::alterTable()`.
+
+Renaming tables via setting the `$newName` property on a `TableDiff` and passing it to `AbstractPlatform::alterTable()`
+is deprecated. The implementations of `AbstractSchemaManager::alterTable()` should use `AbstractPlatform::renameTable()`
+instead.
+
+The `TableDiff::$newName` property and the `TableDiff::getNewName()` method have been deprecated.
+
 ## Marked `Comparator` methods as internal.
 
 The following `Comparator` methods have been marked as internal:

diff --git a/psalm.xml.dist b/psalm.xml.dist
@@ -423,6 +423,10 @@
                 -->
                 <referencedMethod name="Doctrine\DBAL\Schema\ColumnDiff::hasChanged"/>
                 <referencedMethod name="Doctrine\DBAL\Schema\Comparator::diffColumn"/>
+                <!--
+                    TODO: remove in 4.0.0
+                -->
+                <referencedMethod name="Doctrine\DBAL\Schema\TableDiff::getNewName"/>
             </errorLevel>
         </DeprecatedMethod>
         <DeprecatedProperty>
@@ -464,6 +468,10 @@
                     TODO: remove in 4.0.0
                 -->
                 <referencedProperty name="Doctrine\DBAL\Schema\ColumnDiff::$changedProperties"/>
+                <!--
+                    TODO: remove in 4.0.0
+                -->
+                <referencedProperty name="Doctrine\DBAL\Schema\TableDiff::$newName"/>
             </errorLevel>
         </DeprecatedProperty>
         <DocblockTypeContradiction>

diff --git a/src/Platforms/AbstractMySQLPlatform.php b/src/Platforms/AbstractMySQLPlatform.php
@@ -610,6 +610,13 @@ public function getAlterTableSQL(TableDiff $diff)
         $newName    = $diff->getNewName();
 
         if ($newName !== false) {
+            Deprecation::trigger(
+                'doctrine/dbal',
+                'https://github.com/doctrine/dbal/pull/5663',
+                'Generation of SQL that renames a table using %s is deprecated. Use getRenameTableSQL() instead.',
+                __METHOD__,
+            );
+
             $queryParts[] = 'RENAME TO ' . $newName->getQuotedName($this);
         }
 

diff --git a/src/Platforms/AbstractPlatform.php b/src/Platforms/AbstractPlatform.php
@@ -2592,6 +2592,14 @@ public function getAlterTableSQL(TableDiff $diff)
         throw Exception::notSupported(__METHOD__);
     }
 
+    /** @return list<string> */
+    public function getRenameTableSQL(string $oldName, string $newName): array
+    {
+        return [
+            sprintf('ALTER TABLE %s RENAME TO %s', $oldName, $newName),
+        ];
+    }
+
     /**
      * @param mixed[] $columnSql
      *

diff --git a/src/Platforms/DB2Platform.php b/src/Platforms/DB2Platform.php
@@ -643,6 +643,13 @@ public function getAlterTableSQL(TableDiff $diff)
             $newName = $diff->getNewName();
 
             if ($newName !== false) {
+                Deprecation::trigger(
+                    'doctrine/dbal',
+                    'https://github.com/doctrine/dbal/pull/5663',
+                    'Generation of "rename table" SQL using %s is deprecated. Use getRenameTableSQL() instead.',
+                    __METHOD__,
+                );
+
                 $sql[] = sprintf(
                     'RENAME TABLE %s TO %s',
                     $diff->getName($this)->getQuotedName($this),
@@ -660,6 +667,16 @@ public function getAlterTableSQL(TableDiff $diff)
         return array_merge($sql, $tableSql, $columnSql);
     }
 
+    /**
+     * {@inheritDoc}
+     */
+    public function getRenameTableSQL(string $oldName, string $newName): array
+    {
+        return [
+            sprintf('RENAME TABLE %s TO %s', $oldName, $newName),
+        ];
+    }
+
     /**
      * Gathers the table alteration SQL for a given column diff.
      *

diff --git a/src/Platforms/OraclePlatform.php b/src/Platforms/OraclePlatform.php
@@ -977,6 +977,13 @@ public function getAlterTableSQL(TableDiff $diff)
             $newName = $diff->getNewName();
 
             if ($newName !== false) {
+                Deprecation::trigger(
+                    'doctrine/dbal',
+                    'https://github.com/doctrine/dbal/pull/5663',
+                    'Generation of "rename table" SQL using %s is deprecated. Use getRenameTableSQL() instead.',
+                    __METHOD__,
+                );
+
                 $sql[] = sprintf(
                     'ALTER TABLE %s RENAME TO %s',
                     $diff->getName($this)->getQuotedName($this),

diff --git a/src/Platforms/PostgreSQLPlatform.php b/src/Platforms/PostgreSQLPlatform.php
@@ -656,6 +656,13 @@ public function getAlterTableSQL(TableDiff $diff)
             $newName = $diff->getNewName();
 
             if ($newName !== false) {
+                Deprecation::trigger(
+                    'doctrine/dbal',
+                    'https://github.com/doctrine/dbal/pull/5663',
+                    'Generation of "rename table" SQL using %s is deprecated. Use getRenameTableSQL() instead.',
+                    __METHOD__,
+                );
+
                 $sql[] = sprintf(
                     'ALTER TABLE %s RENAME TO %s',
                     $diff->getName($this)->getQuotedName($this),

diff --git a/src/Platforms/SQLServerPlatform.php b/src/Platforms/SQLServerPlatform.php
@@ -661,24 +661,14 @@ public function getAlterTableSQL(TableDiff $diff)
         $newName = $diff->getNewName();
 
         if ($newName !== false) {
-            $sql[] = "sp_rename '" . $diff->getName($this)->getQuotedName($this) . "', '" . $newName->getName() . "'";
+            Deprecation::trigger(
+                'doctrine/dbal',
+                'https://github.com/doctrine/dbal/pull/5663',
+                'Generation of "rename table" SQL using %s is deprecated. Use getRenameTableSQL() instead.',
+                __METHOD__,
+            );
 
-            /**
-             * Rename table's default constraints names
-             * to match the new table name.
-             * This is necessary to ensure that the default
-             * constraints can be referenced in future table
-             * alterations as the table name is encoded in
-             * default constraints' names.
-             */
-            $sql[] = "DECLARE @sql NVARCHAR(MAX) = N''; " .
-                "SELECT @sql += N'EXEC sp_rename N''' + dc.name + ''', N''' " .
-                "+ REPLACE(dc.name, '" . $this->generateIdentifierName($diff->name) . "', " .
-                "'" . $this->generateIdentifierName($newName->getName()) . "') + ''', ''OBJECT'';' " .
-                'FROM sys.default_constraints dc ' .
-                'JOIN sys.tables tbl ON dc.parent_object_id = tbl.object_id ' .
-                "WHERE tbl.name = '" . $newName->getName() . "';" .
-                'EXEC sp_executesql @sql';
+            $sql = array_merge($sql, $this->getRenameTableSQL($diff->name, $newName->getName()));
         }
 
         $sql = array_merge(
@@ -690,6 +680,38 @@ public function getAlterTableSQL(TableDiff $diff)
         return array_merge($sql, $tableSql, $columnSql);
     }
 
+    /**
+     * {@inheritDoc}
+     */
+    public function getRenameTableSQL(string $oldName, string $newName): array
+    {
+        return [
+            sprintf('sp_rename %s, %s', $this->quoteStringLiteral($oldName), $this->quoteStringLiteral($newName)),
+
+            /* Rename table's default constraints names
+             * to match the new table name.
+             * This is necessary to ensure that the default
+             * constraints can be referenced in future table
+             * alterations as the table name is encoded in
+             * default constraints' names. */
+            sprintf(
+                <<<'SQL'
+                DECLARE @sql NVARCHAR(MAX) = N'';
+                SELECT @sql += N'EXEC sp_rename N''' + dc.name + ''', N'''
+                    + REPLACE(dc.name, '%s', '%s') + ''', ''OBJECT'';'
+                    FROM sys.default_constraints dc
+                    JOIN sys.tables tbl
+                        ON dc.parent_object_id = tbl.object_id
+                    WHERE tbl.name = %s;
+                EXEC sp_executesql @sql
+                SQL,
+                $this->generateIdentifierName($oldName),
+                $this->generateIdentifierName($newName),
+                $this->quoteStringLiteral($newName),
+            ),
+        ];
+    }
+
     /**
      * Returns the SQL clause for adding a default constraint in an ALTER TABLE statement.
      *

diff --git a/src/Platforms/SqlitePlatform.php b/src/Platforms/SqlitePlatform.php
@@ -1104,6 +1104,13 @@ public function getAlterTableSQL(TableDiff $diff)
             $newName = $diff->getNewName();
 
             if ($newName !== false) {
+                Deprecation::trigger(
+                    'doctrine/dbal',
+                    'https://github.com/doctrine/dbal/pull/5663',
+                    'Generation of "rename table" SQL using %s is deprecated. Use getRenameTableSQL() instead.',
+                    __METHOD__,
+                );
+
                 $sql[] = sprintf(
                     'ALTER TABLE %s RENAME TO %s',
                     $newTable->getQuotedName($this),
@@ -1234,6 +1241,14 @@ private function getSimpleAlterTableSQL(TableDiff $diff)
 
         if (! $this->onSchemaAlterTable($diff, $tableSql)) {
             if ($diff->newName !== false) {
+                Deprecation::trigger(
+                    'doctrine/dbal',
+                    'https://github.com/doctrine/dbal/pull/5663',
+                    'Generation of SQL that renames a table using %s is deprecated.'
+                        . ' Use getRenameTableSQL() instead.',
+                    __METHOD__,
+                );
+
                 $newTable = new Identifier($diff->newName);
 
                 $sql[] = 'ALTER TABLE ' . $table->getQuotedName($this) . ' RENAME TO '

diff --git a/src/Schema/AbstractSchemaManager.php b/src/Schema/AbstractSchemaManager.php
@@ -1255,9 +1255,7 @@ public function alterTable(TableDiff $tableDiff)
      */
     public function renameTable($name, $newName)
     {
-        $tableDiff          = new TableDiff($name);
-        $tableDiff->newName = $newName;
-        $this->alterTable($tableDiff);
+        $this->_execSql($this->_platform->getRenameTableSQL($name, $newName));
     }
 
     /**

diff --git a/src/Schema/TableDiff.php b/src/Schema/TableDiff.php
@@ -3,6 +3,7 @@
 namespace Doctrine\DBAL\Schema;
 
 use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\Deprecations\Deprecation;
 
 /**
  * Table Diff.
@@ -12,7 +13,11 @@ class TableDiff
     /** @var string */
     public $name;
 
-    /** @var string|false */
+    /**
+     * @deprecated Rename tables via {@link AbstractSchemaManager::renameTable()} instead.
+     *
+     * @var string|false
+     */
     public $newName = false;
 
     /**
@@ -140,9 +145,20 @@ public function getName(AbstractPlatform $platform)
         );
     }
 
-    /** @return Identifier|false */
+    /**
+     * @deprecated Rename tables via {@link AbstractSchemaManager::renameTable()} instead.
+     *
+     * @return Identifier|false
+     */
     public function getNewName()
     {
+        Deprecation::triggerIfCalledFromOutside(
+            'doctrine/dbal',
+            'https://github.com/doctrine/dbal/pull/5663',
+            '%s is deprecated. Rename tables via AbstractSchemaManager::renameTable() instead.',
+            __METHOD__,
+        );
+
         if ($this->newName === false) {
             return false;
         }