Fix #16796: Fixed addition and removal of table and column comments in MSSQL

framework/CHANGELOG.md

@@ -7,6 +7,7 @@ Yii Framework 2 Change Log
 - Bug #17219: Fixed quoting of table names with spaces in MSSQL (alexkart)
 - Bug #10020: Fixed quoting of column names with dots in MSSQL (alexkart)
 - Bug #17424: Subdomain support for `User::loginRequired` (alex-code)
+- Bug #16796: Fixed addition and removal of table and column comments in MSSQL (sdlins)
 - Bug #17449: Fixed order of SQL column build syntax for MySQL migration (choken)
 - Bug #17437: Fixed generating namespaced migrations (bizley)
 

framework/db/mssql/QueryBuilder.php

@@ -250,13 +250,62 @@ class QueryBuilder extends \yii\db\QueryBuilder
         return $command;
     }
 
+     /**
+     * Builds a SQL command for adding or updating a comment to a table or a column. The command built will check if a comment
+     * already exists. If so, it will be updated, otherwise, it will be added.
+     *
+     * @param string $comment the text of the comment to be added. The comment will be properly quoted by the method.
+     * @param string $table the table to be commented or whose column is to be commented. The table name will be
+     * properly quoted by the method.
+     * @param string $column optional. The name of the column to be commented. If empty, the command will add the
+     * comment to the table instead. The column name will be properly quoted by the method.
+     * @return string the SQL statement for adding a comment.
+     * @throws InvalidArgumentException if the table does not exist.
+     * @since 2.0.24
+     */
+    protected function buildAddCommentSql($comment, $table, $column = null)
+    {
+        $tableSchema = $this->db->schema->getTableSchema($table);
+
+        if ($tableSchema === null) {
+            throw new InvalidArgumentException("Table not found: $table");
+        }
+
+        $schemaName = $tableSchema->schemaName ? "N'" . $tableSchema->schemaName . "'": 'SCHEMA_NAME()';
+        $tableName = "N" . $this->db->quoteValue($tableSchema->name);
+        $columnName = $column ? "N" . $this->db->quoteValue($column) : null;
+        $comment = "N" . $this->db->quoteValue($comment);
+
+        $functionParams = "
+            @name = N'MS_description',
+            @value = $comment,
+            @level0type = N'SCHEMA', @level0name = $schemaName,
+            @level1type = N'TABLE', @level1name = $tableName"
+            . ($column ? ", @level2type = N'COLUMN', @level2name = $columnName" : '') . ';';
+
+        return "
+            IF NOT EXISTS (
+                    SELECT 1
+                    FROM fn_listextendedproperty (
+                        N'MS_description',
+                        'SCHEMA', $schemaName,
+                        'TABLE', $tableName,
+                        " . ($column ? "'COLUMN', $columnName " : ' DEFAULT, DEFAULT ') . "
+                    )
+            )
+                EXEC sys.sp_addextendedproperty $functionParams
+            ELSE
+                EXEC sys.sp_updateextendedproperty $functionParams
+        ";
+    }
+
     /**
      * {@inheritdoc}
      * @since 2.0.8
      */
     public function addCommentOnColumn($table, $column, $comment)
     {
-        return "sp_updateextendedproperty @name = N'MS_Description', @value = {$this->db->quoteValue($comment)}, @level1type = N'Table',  @level1name = {$this->db->quoteTableName($table)}, @level2type = N'Column', @level2name = {$this->db->quoteColumnName($column)}";
+        return $this->buildAddCommentSql($comment, $table, $column);
     }
 
     /**
@@ -265,7 +314,48 @@ class QueryBuilder extends \yii\db\QueryBuilder
      */
     public function addCommentOnTable($table, $comment)
     {
-        return "sp_updateextendedproperty @name = N'MS_Description', @value = {$this->db->quoteValue($comment)}, @level1type = N'Table',  @level1name = {$this->db->quoteTableName($table)}";
+        return $this->buildAddCommentSql($comment, $table);
+    }
+
+    /**
+     * Builds a SQL command for removing a comment from a table or a column. The command built will check if a comment
+     * already exists before trying to perform the removal.
+     *
+     * @param string $table the table that will have the comment removed or whose column will have the comment removed.
+     * The table name will be properly quoted by the method.
+     * @param string $column optional. The name of the column whose comment will be removed. If empty, the command
+     * will remove the comment from the table instead. The column name will be properly quoted by the method.
+     * @return string the SQL statement for removing the comment.
+     * @throws InvalidArgumentException if the table does not exist.
+     * @since 2.0.24
+     */
+    protected function buildRemoveCommentSql($table, $column = null)
+    {
+        $tableSchema = $this->db->schema->getTableSchema($table);
+
+        if ($tableSchema === null) {
+            throw new InvalidArgumentException("Table not found: $table");
+        }
+
+        $schemaName = $tableSchema->schemaName ? "N'" . $tableSchema->schemaName . "'": 'SCHEMA_NAME()';
+        $tableName = "N" . $this->db->quoteValue($tableSchema->name);
+        $columnName = $column ? "N" . $this->db->quoteValue($column) : null;
+
+        return "
+            IF EXISTS (
+                    SELECT 1
+                    FROM fn_listextendedproperty (
+                        N'MS_description',
+                        'SCHEMA', $schemaName,
+                        'TABLE', $tableName,
+                        " . ($column ? "'COLUMN', $columnName " : ' DEFAULT, DEFAULT ') . "
+                    )
+            )
+                EXEC sys.sp_dropextendedproperty
+                    @name = N'MS_description',
+                    @level0type = N'SCHEMA', @level0name = $schemaName,
+                    @level1type = N'TABLE', @level1name = $tableName"
+                    . ($column ? ", @level2type = N'COLUMN', @level2name = $columnName" : '') . ';';
     }
 
     /**
@@ -274,7 +364,7 @@ class QueryBuilder extends \yii\db\QueryBuilder
      */
     public function dropCommentFromColumn($table, $column)
     {
-        return "sp_dropextendedproperty @name = N'MS_Description', @level1type = N'Table',  @level1name = {$this->db->quoteTableName($table)}, @level2type = N'Column', @level2name = {$this->db->quoteColumnName($column)}";
+        return $this->buildRemoveCommentSql($table, $column);
     }
 
     /**
@@ -283,7 +373,7 @@ class QueryBuilder extends \yii\db\QueryBuilder
      */
     public function dropCommentFromTable($table)
     {
-        return "sp_dropextendedproperty @name = N'MS_Description', @level1type = N'Table',  @level1name = {$this->db->quoteTableName($table)}";
+        return $this->buildRemoveCommentSql($table);
     }
 
     /**

framework/db/mssql/Schema.php

@@ -411,7 +411,7 @@ SQL;
     protected function findColumns($table)
     {
         $columnsTableName = 'INFORMATION_SCHEMA.COLUMNS';
-        $whereSql = "[t1].[table_name] = '{$table->name}'";
+        $whereSql = "[t1].[table_name] = " . $this->db->quoteValue($table->name);
         if ($table->catalogName !== null) {
             $columnsTableName = "{$table->catalogName}.{$columnsTableName}";
             $whereSql .= " AND [t1].[table_catalog] = '{$table->catalogName}'";

tests/data/mssql.sql

@@ -23,6 +23,7 @@ IF OBJECT_ID('[T_constraints_2]', 'U') IS NOT NULL DROP TABLE [T_constraints_2];
 IF OBJECT_ID('[T_constraints_1]', 'U') IS NOT NULL DROP TABLE [T_constraints_1];
 IF OBJECT_ID('[T_upsert]', 'U') IS NOT NULL DROP TABLE [T_upsert];
 IF OBJECT_ID('[table.with.special.characters]', 'U') IS NOT NULL DROP TABLE [table.with.special.characters];
+IF OBJECT_ID('[stranger ''table]', 'U') IS NOT NULL DROP TABLE [stranger 'table];
 
 CREATE TABLE [dbo].[profile] (
     [id] [int] IDENTITY NOT NULL,
@@ -362,3 +363,8 @@ INSERT INTO [validator_ref] (a_field, ref) VALUES ('ref_to_3', 3);
 INSERT INTO [validator_ref] (a_field, ref) VALUES ('ref_to_4', 4);
 INSERT INTO [validator_ref] (a_field, ref) VALUES ('ref_to_4', 4);
 INSERT INTO [validator_ref] (a_field, ref) VALUES ('ref_to_5', 5);
+
+CREATE TABLE [dbo].[stranger 'table] (
+    [id] [int],
+    [stranger 'field] [varchar] (32)
+);

tests/framework/db/mssql/QueryBuilderTest.php

@@ -68,36 +68,178 @@ class QueryBuilderTest extends \yiiunit\framework\db\QueryBuilderTest
         $this->assertEquals($expectedQueryParams, $actualQueryParams);
     }
 
-    public function testCommentColumn()
+    protected function getCommmentsFromTable($table)
     {
-        // related to https://github.com/yiisoft/yii2/pull/17364
+        $db = $this->getConnection(false, false);
-        $this->markTestSkipped('Should be fixed');
+        $sql = "SELECT *
+            FROM fn_listextendedproperty (
+                N'MS_description',
+                'SCHEMA', N'dbo',
+                'TABLE', N" . $db->quoteValue($table) . ",
+                DEFAULT, DEFAULT
+        )";
+        return $db->createCommand($sql)->queryAll();
+    }
 
-        $qb = $this->getQueryBuilder();
+    protected function getCommentsFromColumn($table, $column)
+    {
+        $db = $this->getConnection(false, false);
+        $sql = "SELECT *
+            FROM fn_listextendedproperty (
+                N'MS_description',
+                'SCHEMA', N'dbo',
+                'TABLE', N" . $db->quoteValue($table) . ",
+                'COLUMN', N" . $db->quoteValue($column) . "
+        )";
+        return $db->createCommand($sql)->queryAll();
+    }
 
-        $expected = "sp_updateextendedproperty @name = N'MS_Description', @value = 'This is my column.', @level1type = N'Table',  @level1name = comment, @level2type = N'Column', @level2name = text";
+    protected function runAddCommentOnTable($comment, $table)
-        $sql = $qb->addCommentOnColumn('comment', 'text', 'This is my column.');
+    {
-        $this->assertEquals($expected, $sql);
+        $qb = $this->getQueryBuilder();
+        $db = $this->getConnection(false, false);
+        $sql = $qb->addCommentOnTable($table, $comment);
+        return $db->createCommand($sql)->execute();
+    }
 
-        $expected = "sp_dropextendedproperty @name = N'MS_Description', @level1type = N'Table',  @level1name = comment, @level2type = N'Column', @level2name = text";
+    protected function runAddCommentOnColumn($comment, $table, $column)
-        $sql = $qb->dropCommentFromColumn('comment', 'text');
+    {
-        $this->assertEquals($expected, $sql);
+        $qb = $this->getQueryBuilder();
+        $db = $this->getConnection(false, false);
+        $sql = $qb->addCommentOnColumn($table, $column, $comment);
+        return $db->createCommand($sql)->execute();
     }
 
-    public function testCommentTable()
+    protected function runDropCommentFromTable($table)
     {
-        // related to https://github.com/yiisoft/yii2/pull/17364
+        $qb = $this->getQueryBuilder();
-        $this->markTestSkipped('Should be fixed');
+        $db = $this->getConnection(false, false);
+        $sql = $qb->dropCommentFromTable($table);
+        return $db->createCommand($sql)->execute();
+    }
 
+    protected function runDropCommentFromColumn($table, $column)
+    {
         $qb = $this->getQueryBuilder();
+        $db = $this->getConnection(false, false);
+        $sql = $qb->dropCommentFromColumn($table, $column);
+        return $db->createCommand($sql)->execute();
+    }
 
-        $expected = "sp_updateextendedproperty @name = N'MS_Description', @value = 'This is my table.', @level1type = N'Table',  @level1name = comment";
+    public function testCommentAdditionOnTableAndOnColumn()
-        $sql = $qb->addCommentOnTable('comment', 'This is my table.');
+    {
-        $this->assertEquals($expected, $sql);
+        $table = 'profile';
+        $tableComment = 'A comment for profile table.';
+        $this->runAddCommentOnTable($tableComment, $table);
+        $resultTable = $this->getCommmentsFromTable($table);
+        $this->assertEquals([
+            'objtype' => 'TABLE',
+            'objname' => $table,
+            'name' => 'MS_description',
+            'value' => $tableComment,
+        ], $resultTable[0]);
+
+        $column = 'description';
+        $columnComment = 'A comment for description column in profile table.';
+        $this->runAddCommentOnColumn($columnComment, $table, $column);
+        $resultColumn = $this->getCommentsFromColumn($table, $column);
+        $this->assertEquals([
+            'objtype' => 'COLUMN',
+            'objname' => $column,
+            'name' => 'MS_description',
+            'value' => $columnComment,
+        ], $resultColumn[0]);
+
+        // Add another comment to the same table to test update
+        $tableComment2 = 'Another comment for profile table.';
+        $this->runAddCommentOnTable($tableComment2, $table);
+        $result = $this->getCommmentsFromTable($table);
+        $this->assertEquals([
+            'objtype' => 'TABLE',
+            'objname' => $table,
+            'name' => 'MS_description',
+            'value' => $tableComment2,
+        ], $result[0]);
+
+        // Add another comment to the same column to test update
+        $columnComment2 = 'Another comment for description column in profile table.';
+        $this->runAddCommentOnColumn($columnComment2, $table, $column);
+        $result = $this->getCommentsFromColumn($table, $column);
+        $this->assertEquals([
+            'objtype' => 'COLUMN',
+            'objname' => $column,
+            'name' => 'MS_description',
+            'value' => $columnComment2,
+        ], $result[0]);
+    }
 
-        $expected = "sp_dropextendedproperty @name = N'MS_Description', @level1type = N'Table',  @level1name = comment";
+    public function testCommentAdditionOnQuotedTableOrColumn()
-        $sql = $qb->dropCommentFromTable('comment');
+    {
-        $this->assertEquals($expected, $sql);
+        $table = 'stranger \'table';
+        $tableComment = 'A comment for stranger \'table.';
+        $this->runAddCommentOnTable($tableComment, $table);
+        $resultTable = $this->getCommmentsFromTable($table);
+        $this->assertEquals([
+            'objtype' => 'TABLE',
+            'objname' => $table,
+            'name' => 'MS_description',
+            'value' => $tableComment,
+        ], $resultTable[0]);
+
+        $column = 'stranger \'field';
+        $columnComment = 'A comment for stranger \'field column in stranger \'table.';
+        $this->runAddCommentOnColumn($columnComment, $table, $column);
+        $resultColumn = $this->getCommentsFromColumn($table, $column);
+        $this->assertEquals([
+            'objtype' => 'COLUMN',
+            'objname' => $column,
+            'name' => 'MS_description',
+            'value' => $columnComment,
+        ], $resultColumn[0]);
+    }
+
+    public function testCommentRemovalFromTableAndFromColumn()
+    {
+        $table = 'profile';
+        $tableComment = 'A comment for profile table.';
+        $this->runAddCommentOnTable($tableComment, $table);
+        $this->runDropCommentFromTable($table);
+        $result = $this->getCommmentsFromTable($table);
+        $this->assertEquals([], $result);
+
+        $column = 'description';
+        $columnComment = 'A comment for description column in profile table.';
+        $this->runAddCommentOnColumn($columnComment, $table, $column);
+        $this->runDropCommentFromColumn($table, $column);
+        $result = $this->getCommentsFromColumn($table, $column);
+        $this->assertEquals([], $result);
+    }
+
+    public function testCommentRemovalFromQuotedTableOrColumn()
+    {
+        $table = 'stranger \'table';
+        $tableComment = 'A comment for stranger \'table.';
+        $this->runAddCommentOnTable($tableComment, $table);
+        $this->runDropCommentFromTable($table);
+        $result = $this->getCommmentsFromTable($table);
+        $this->assertEquals([], $result);
+
+        $column = 'stranger \'field';
+        $columnComment = 'A comment for stranger \'field in stranger \'table.';
+        $this->runAddCommentOnColumn($columnComment, $table, $column);
+        $this->runDropCommentFromColumn($table, $column);
+        $result = $this->getCommentsFromColumn($table, $column);
+        $this->assertEquals([], $result);
+    }
+
+    public function testCommentColumn()
+    {
+        $this->markTestSkipped("Testing the behavior, not sql generation anymore.");
+    }
+
+    public function testCommentTable()
+    {
+        $this->markTestSkipped("Testing the behavior, not sql generation anymore.");
     }
 
     /**