Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate renaming tables via TableDiff #5663

Merged
merged 1 commit into from
Sep 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
8 changes: 8 additions & 0 deletions psalm.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
7 changes: 7 additions & 0 deletions src/Platforms/AbstractMySQLPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
8 changes: 8 additions & 0 deletions src/Platforms/AbstractPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down
17 changes: 17 additions & 0 deletions src/Platforms/DB2Platform.php
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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.
*
Expand Down
7 changes: 7 additions & 0 deletions src/Platforms/OraclePlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
7 changes: 7 additions & 0 deletions src/Platforms/PostgreSQLPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
56 changes: 39 additions & 17 deletions src/Platforms/SQLServerPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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.
*
Expand Down
15 changes: 15 additions & 0 deletions src/Platforms/SqlitePlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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 '
Expand Down
4 changes: 1 addition & 3 deletions src/Schema/AbstractSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

/**
Expand Down
20 changes: 18 additions & 2 deletions src/Schema/TableDiff.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Doctrine\DBAL\Schema;

use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\Deprecations\Deprecation;

/**
* Table Diff.
Expand All @@ -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;

/**
Expand Down Expand Up @@ -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;
}
Expand Down