From 217ce8d775425b02fa88a016c67de6839a9df764 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Fri, 9 Sep 2022 07:22:08 -0700 Subject: [PATCH] Deprecate renaming tables via TableDiff --- UPGRADE.md | 8 ++++ psalm.xml.dist | 8 ++++ src/Platforms/AbstractMySQLPlatform.php | 7 ++++ src/Platforms/AbstractPlatform.php | 8 ++++ src/Platforms/DB2Platform.php | 17 ++++++++ src/Platforms/OraclePlatform.php | 7 ++++ src/Platforms/PostgreSQLPlatform.php | 7 ++++ src/Platforms/SQLServerPlatform.php | 56 +++++++++++++++++-------- src/Platforms/SqlitePlatform.php | 15 +++++++ src/Schema/AbstractSchemaManager.php | 4 +- src/Schema/TableDiff.php | 20 ++++++++- 11 files changed, 135 insertions(+), 22 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 1e91fc70e42..3cafad388fb 100644 --- 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 index 2bbf2915c85..4deffbfbd3f 100644 --- a/psalm.xml.dist +++ b/psalm.xml.dist @@ -423,6 +423,10 @@ --> + + @@ -464,6 +468,10 @@ TODO: remove in 4.0.0 --> + + diff --git a/src/Platforms/AbstractMySQLPlatform.php b/src/Platforms/AbstractMySQLPlatform.php index 4892a51a3c0..33ccf60c8e9 100644 --- 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 index 3265532fbe4..347bc6b2fae 100644 --- 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 */ + 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 index ce3c5ad456b..c2365e22c24 100644 --- 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 index 35165c9464a..974170558a4 100644 --- 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 index 60a9bb55798..c679e49ada6 100644 --- 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 index 0e4d1ee34b1..6a65d300631 100644 --- 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 index f69858766f4..850d340aa82 100644 --- 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 index 7ac0c970a25..573d2ea5e5a 100644 --- 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 index f58be1af432..06e0a2dd493 100644 --- 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; }