Merge pull request #6602 from morozov/sqlserver-rename-to-quoted

Fix renaming to quoted names on SQL Server

Author: morozov

src/Platforms/SQLServerPlatform.php

@@ -22,6 +22,7 @@
 use Doctrine\DBAL\Types\Types;
 use InvalidArgumentException;
 
+use function array_map;
 use function array_merge;
 use function array_unique;
 use function array_values;
@@ -406,18 +407,20 @@ public function getAlterTableSQL(TableDiff $diff): array
         $tableNameSQL = $table->getQuotedName($this);
 
         foreach ($diff->getChangedColumns() as $columnDiff) {
-            $newColumn     = $columnDiff->getNewColumn();
-            $newColumnName = $newColumn->getQuotedName($this);
+            $newColumn   = $columnDiff->getNewColumn();
+            $oldColumn   = $columnDiff->getOldColumn();
+            $nameChanged = $columnDiff->hasNameChanged();
 
-            $oldColumn     = $columnDiff->getOldColumn();
-            $oldColumnName = $oldColumn->getQuotedName($this);
-            $nameChanged   = $columnDiff->hasNameChanged();
-
-            // Column names in SQL server are case insensitive and automatically uppercased on the server.
             if ($nameChanged) {
+                // sp_rename accepts the old name as a qualified name, so it should be quoted.
+                $oldColumnNameSQL = $oldColumn->getQuotedName($this);
+
+                // sp_rename accepts the new name as a literal value, so it cannot be quoted.
+                $newColumnName = $newColumn->getName();
+
                 $sql = array_merge(
                     $sql,
-                    $this->getRenameColumnSQL($tableNameSQL, $oldColumnName, $newColumnName),
+                    $this->getRenameColumnSQL($tableNameSQL, $oldColumnNameSQL, $newColumnName),
                 );
             }
 
@@ -492,11 +495,7 @@ public function getAlterTableSQL(TableDiff $diff): array
 
     public function getRenameTableSQL(string $oldName, string $newName): string
     {
-        return sprintf(
-            'sp_rename %s, %s',
-            $this->quoteStringLiteral($oldName),
-            $this->quoteStringLiteral($newName),
-        );
+        return $this->getRenameSQL($oldName, $newName);
     }
 
     /**
@@ -630,13 +629,7 @@ protected function getDropColumnCommentSQL(string $tableName, string $columnName
      */
     protected function getRenameIndexSQL(string $oldIndexName, Index $index, string $tableName): array
     {
-        return [sprintf(
-            "EXEC sp_rename N'%s.%s', N'%s', N'INDEX'",
-            $tableName,
-            $oldIndexName,
-            $index->getQuotedName($this),
-        ),
-        ];
+        return [$this->getRenameSQL($tableName . '.' . $oldIndexName, $index->getName(), 'INDEX')];
     }
 
     /**
@@ -650,12 +643,18 @@ protected function getRenameIndexSQL(string $oldIndexName, Index $index, string
      */
     protected function getRenameColumnSQL(string $tableName, string $oldColumnName, string $newColumnName): array
     {
-        return [sprintf(
-            "EXEC sp_rename %s, %s, 'COLUMN'",
-            $this->quoteStringLiteral($tableName . '.' . $oldColumnName),
-            $this->quoteStringLiteral($newColumnName),
-        ),
-        ];
+        return [$this->getRenameSQL($tableName . '.' . $oldColumnName, $newColumnName)];
+    }
+
+    /**
+     * Returns the SQL statement that will execute sp_rename with the given arguments.
+     */
+    private function getRenameSQL(string ...$arguments): string
+    {
+        return 'EXEC sp_rename '
+             . implode(', ', array_map(function (string $argument): string {
+                return 'N' . $this->quoteStringLiteral($argument);
+             }, $arguments));
     }
 
     /**

tests/Functional/Platform/RenameColumnTest.php

@@ -4,6 +4,7 @@
 
 namespace Doctrine\DBAL\Tests\Functional\Platform;
 
+use Doctrine\DBAL\Exception;
 use Doctrine\DBAL\Schema\Column;
 use Doctrine\DBAL\Schema\Table;
 use Doctrine\DBAL\Schema\TableDiff;
@@ -94,4 +95,31 @@ public static function columnNameProvider(): iterable
         yield ['C1', 'c1_x'];
         yield ['importantColumn', 'veryImportantColumn'];
     }
+
+    /** @throws Exception */
+    public function testRenameColumToQuoted(): void
+    {
+        $table = new Table('test_rename');
+        $table->addColumn('c1', Types::INTEGER);
+
+        $this->dropAndCreateTable($table);
+
+        $table->dropColumn('c1')
+            ->addColumn('"c2"', Types::INTEGER);
+
+        $schemaManager = $this->connection->createSchemaManager();
+        $comparator    = $schemaManager->createComparator();
+
+        $diff = $comparator->compareTables($schemaManager->introspectTable('test_rename'), $table);
+        self::assertFalse($diff->isEmpty());
+
+        $schemaManager->alterTable($diff);
+
+        $platform = $this->connection->getDatabasePlatform();
+
+        self::assertEquals(1, $this->connection->insert(
+            'test_rename',
+            [$platform->quoteSingleIdentifier('c2') => 1],
+        ));
+    }
 }

tests/Platforms/SQLServerPlatformTest.php

@@ -848,8 +848,8 @@ protected function getAlterTableRenameIndexSQL(): array
     protected function getQuotedAlterTableRenameIndexSQL(): array
     {
         return [
-            "EXEC sp_rename N'[table].[create]', N'[select]', N'INDEX'",
-            "EXEC sp_rename N'[table].[foo]', N'[bar]', N'INDEX'",
+            "EXEC sp_rename N'[table].[create]', N'select', N'INDEX'",
+            "EXEC sp_rename N'[table].[foo]', N'bar', N'INDEX'",
         ];
     }
 
@@ -867,8 +867,8 @@ protected function getAlterTableRenameIndexInSchemaSQL(): array
     protected function getQuotedAlterTableRenameIndexInSchemaSQL(): array
     {
         return [
-            "EXEC sp_rename N'[schema].[table].[create]', N'[select]', N'INDEX'",
-            "EXEC sp_rename N'[schema].[table].[foo]', N'[bar]', N'INDEX'",
+            "EXEC sp_rename N'[schema].[table].[create]', N'select', N'INDEX'",
+            "EXEC sp_rename N'[schema].[table].[foo]', N'bar', N'INDEX'",
         ];
     }