SQLServer renameTable() problem

[aimeos]

Bug Report

Q	A
Version	3.9/4.2

Summary

SQL Server uses sp_rename function for renaming tables but using quotes for target table name results in wrong table name.

How to reproduce

$schemaManager->renameTable('[test]', '[dbo.test2]');
// -> sp_rename '[test]', '[dbo.test2]'

The new table is now named "[dbo.test2]" instead of "test2". This does only occur in SQLServer platform, all other platforms use quoting as expected so this is inconsistent between SQLServer platform and all other platforms.

Expected behaviour

Quotes for the target table name should be automatically removed by the SQLServer platform here to get consistent results:

dbal/src/Platforms/SQLServerPlatform.php

Line 498 in dadd353

$this->quoteStringLiteral($newName),

$schemaManager->renameTable('[test]', '[dbo.test2]');
// -> sp_rename '[test]', 'test2'

[morozov]

Can sp_rename move the table to a different schema? See the details in #6602.

As currently designed, the behavior of renameTable() isn't portable across supported platforms. Unlike other platforms where the second parameter is a SQL expression representing the new name, the SQL Server platform (due to the implementation details) accepts a literal.

[aimeos]

No, sp_rename can't move tables to a different schema and only accepts a literal as new name.

[morozov]

@aimeos so what do you expect to happen as a result of such a call to renameTable()?

[aimeos]

@morozov Using sp_rename '[test]', 'dbo.test2' will cause an error thrown by SQL Server

[morozov]

There won't be an error. "dbo.test2" is a valid table name, so that will be the new name. Could you check if #6602 solves your problem?

[aimeos]

Yes, it fixes the problem. You are right, dbo.test2 is a vaild column name for sp_rename() and maybe renaming columns could be improved further by using
$newColumnName = $newColumn->getShortestName($newColumn->getNamespaceName());
instead of
$newColumnName = $newColumn->getName();

https://github.com/doctrine/dbal/pull/6602/files#diff-269c4df546691d197440efad484aabdcef49ff891af599481406d8b1c1a2296dR419

[morozov]

maybe renaming columns could be improved further by using
$newColumnName = $newColumn->getShortestName($newColumn->getNamespaceName());
instead of
$newColumnName = $newColumn->getName();

This would implicitly drop part of the data supplied by the user. Not the best idea.

The proper approach is to parse the name as an UnqualifiedName (see #6646). This way, dbo.test would be considered an invalid value unless quoted.

[aimeos]

The proper approach is to parse the name as an UnqualifiedName (see #6646). This way, dbo.test would be considered an invalid value unless quoted.

If the value is quoted and passed to the sp_rename function that way, it will again rename to w wrong table name. When dbo.test or [dbo].[test] is passed to renameColumn(), the only valid value for the second parameter for sp_rename is test

[morozov]

You're missing this part:

The proper approach is to parse the name [...]

Please take a look at #6646 for context.