Deprecate renaming tables via TableDiff

doctrine · Sep 18, 2022 · 217ce8d · 217ce8d
1 parent 7727e46
commit 217ce8d
Show file tree

Hide file tree

Showing 11 changed files with 135 additions and 22 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -13,6 +13,14 @@ awareness about deprecated code.
 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;
         }