diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index 871ec89..ef9e84a 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -25,6 +25,7 @@ Yii Framework 2 Change Log - Enh #18569: Add `NumberValidator::$allowArray` (raidkon) - Bug #18613: Do not call static methods non-statically in `BaseActiveRecord` (samdark) - Bug #18619: Do not modify `yii\web\Cookie::$path` on `yii\web\Response::sendCookies()` (mikk150) +- Bug #18604: Function alterColumn for MSSQL build incorrect query with default values `NULL` and other expressions (darkdef) 2.0.41.1 March 04, 2021 diff --git a/framework/db/ColumnSchemaBuilder.php b/framework/db/ColumnSchemaBuilder.php index edb28ef..cb0e849 100644 --- a/framework/db/ColumnSchemaBuilder.php +++ b/framework/db/ColumnSchemaBuilder.php @@ -331,35 +331,46 @@ class ColumnSchemaBuilder extends BaseObject } /** - * Builds the default value specification for the column. - * @return string string with default value of column. + * Return the default value for the column. + * @return string|null string with default value of column. */ - protected function buildDefaultString() + protected function buildDefaultValue() { if ($this->default === null) { - return $this->isNotNull === false ? ' DEFAULT NULL' : ''; + return $this->isNotNull === false ? 'NULL' : null; } - $string = ' DEFAULT '; switch (gettype($this->default)) { - case 'integer': - $string .= (string) $this->default; - break; case 'double': // ensure type cast always has . as decimal separator in all locales - $string .= StringHelper::floatToString($this->default); + $defaultValue = StringHelper::floatToString($this->default); break; case 'boolean': - $string .= $this->default ? 'TRUE' : 'FALSE'; + $defaultValue = $this->default ? 'TRUE' : 'FALSE'; break; + case 'integer': case 'object': - $string .= (string) $this->default; + $defaultValue = (string) $this->default; break; default: - $string .= "'{$this->default}'"; + $defaultValue = "'{$this->default}'"; + } + + return $defaultValue; + } + + /** + * Builds the default value specification for the column. + * @return string string with default value of column. + */ + protected function buildDefaultString() + { + $defaultValue = $this->buildDefaultValue(); + if ($defaultValue === null) { + return ''; } - return $string; + return ' DEFAULT ' . $defaultValue; } /** diff --git a/framework/db/mssql/ColumnSchemaBuilder.php b/framework/db/mssql/ColumnSchemaBuilder.php new file mode 100644 index 0000000..35a8ac1 --- /dev/null +++ b/framework/db/mssql/ColumnSchemaBuilder.php @@ -0,0 +1,75 @@ + + * @since 2.0.42 + */ +class ColumnSchemaBuilder extends AbstractColumnSchemaBuilder +{ + protected $format = '{type}{length}{notnull}{unique}{default}{check}{append}'; + + /** + * Builds the full string for the column's schema. + * @return string + */ + public function __toString() + { + if ($this->getTypeCategory() === self::CATEGORY_PK) { + $format = '{type}{check}{comment}{append}'; + } else { + $format = $this->format; + } + + return $this->buildCompleteString($format); + } + + /** + * Changes default format string to MSSQL ALTER COMMAND + */ + public function setAlterColumnFormat() + { + $this->format = '{type}{length}{notnull}{append}'; + } + + /** + * Getting the `Default` value for constraint + * @return string|Expression|null + */ + public function getDefaultValue() + { + if ($this->default instanceof Expression) { + return $this->default; + } + + return $this->buildDefaultValue(); + } + + /** + * Get the `Check` value for constraint + * @return string|null + */ + public function getCheckValue() + { + return $this->check !== null ? (string) $this->check : null; + } + + /** + * @return bool + */ + public function isUnique() + { + return $this->isUnique; + } +} diff --git a/framework/db/mssql/QueryBuilder.php b/framework/db/mssql/QueryBuilder.php index df39971..8e250a5 100644 --- a/framework/db/mssql/QueryBuilder.php +++ b/framework/db/mssql/QueryBuilder.php @@ -174,35 +174,40 @@ class QueryBuilder extends \yii\db\QueryBuilder */ public function alterColumn($table, $column, $type) { - $sqlAfter = []; + $sqlAfter = [$this->dropConstraintsForColumn($table, $column, 'D')]; $columnName = $this->db->quoteColumnName($column); $tableName = $this->db->quoteTableName($table); - $constraintBase = preg_replace('/[^a-z0-9_]/i', '', $table . '_' . $column); - $type = $this->getColumnType($type); + if ($type instanceof \yii\db\mssql\ColumnSchemaBuilder) { + $type->setAlterColumnFormat(); - if (preg_match('/\s+DEFAULT\s+(["\']?\w*["\']?)/i', $type, $matches)) { - $type = preg_replace('/\s+DEFAULT\s+(["\']?\w*["\']?)/i', '', $type); - $sqlAfter[] = $this->dropConstraintsForColumn($table, $column, 'D'); - $sqlAfter[] = $this->addDefaultValue("DF_{$constraintBase}", $table, $column, $matches[1]); - } else { - $sqlAfter[] = $this->dropConstraintsForColumn($table, $column, 'D'); - } - if (preg_match('/\s+CHECK\s+\((.+)\)/i', $type, $matches)) { - $type = preg_replace('/\s+CHECK\s+\((.+)\)/i', '', $type); - $sqlAfter[] = "ALTER TABLE {$tableName} ADD CONSTRAINT " . $this->db->quoteColumnName("CK_{$constraintBase}") . " CHECK ({$matches[1]})"; - } + $defaultValue = $type->getDefaultValue(); + if ($defaultValue !== null) { + $sqlAfter[] = $this->addDefaultValue( + "DF_{$constraintBase}", + $table, + $column, + $defaultValue instanceof Expression ? $defaultValue : new Expression($defaultValue) + ); + } - $type = preg_replace('/\s+UNIQUE/i', '', $type, -1, $count); - if ($count) { - $sqlAfter[] = "ALTER TABLE {$tableName} ADD CONSTRAINT " . $this->db->quoteColumnName("UQ_{$constraintBase}") . " UNIQUE ({$columnName})"; + $checkValue = $type->getCheckValue(); + if ($checkValue !== null) { + $sqlAfter[] = "ALTER TABLE {$tableName} ADD CONSTRAINT " . + $this->db->quoteColumnName("CK_{$constraintBase}") . + " CHECK (" . ($defaultValue instanceof Expression ? $checkValue : new Expression($checkValue)) . ")"; + } + + if ($type->isUnique()) { + $sqlAfter[] = "ALTER TABLE {$tableName} ADD CONSTRAINT " . $this->db->quoteColumnName("UQ_{$constraintBase}") . " UNIQUE ({$columnName})"; + } } - return 'ALTER TABLE ' . $this->db->quoteTableName($table) . ' ALTER COLUMN ' - . $this->db->quoteColumnName($column) . ' ' + return 'ALTER TABLE ' . $tableName . ' ALTER COLUMN ' + . $columnName . ' ' . $this->getColumnType($type) . "\n" . implode("\n", $sqlAfter); } @@ -661,5 +666,4 @@ END"; return $this->dropConstraintsForColumn($table, $column) . "\nALTER TABLE " . $this->db->quoteTableName($table) . " DROP COLUMN " . $this->db->quoteColumnName($column); } - } diff --git a/framework/db/mssql/Schema.php b/framework/db/mssql/Schema.php index 365d96e..a2ad449 100644 --- a/framework/db/mssql/Schema.php +++ b/framework/db/mssql/Schema.php @@ -804,4 +804,12 @@ SQL; return $result; } + + /** + * {@inheritdoc} + */ + public function createColumnSchemaBuilder($type, $length = null) + { + return new ColumnSchemaBuilder($type, $length, $this->db); + } } diff --git a/tests/framework/db/mssql/QueryBuilderTest.php b/tests/framework/db/mssql/QueryBuilderTest.php index 624ca72..3a17357 100644 --- a/tests/framework/db/mssql/QueryBuilderTest.php +++ b/tests/framework/db/mssql/QueryBuilderTest.php @@ -552,7 +552,7 @@ WHILE 1=1 BEGIN IF @constraintName IS NULL BREAK EXEC (N'ALTER TABLE ' + @tableName + ' DROP CONSTRAINT [' + @constraintName + ']') END -ALTER TABLE [foo1] ADD CONSTRAINT [DF_foo1_bar] DEFAULT '''''' FOR [bar]"; +ALTER TABLE [foo1] ADD CONSTRAINT [DF_foo1_bar] DEFAULT '' FOR [bar]"; $sql = $qb->alterColumn('foo1', 'bar', $this->string(255)->defaultValue('')); $this->assertEquals($expected, $sql); @@ -579,7 +579,7 @@ WHILE 1=1 BEGIN IF @constraintName IS NULL BREAK EXEC (N'ALTER TABLE ' + @tableName + ' DROP CONSTRAINT [' + @constraintName + ']') END -ALTER TABLE [foo1] ADD CONSTRAINT [DF_foo1_bar] DEFAULT '''AbCdE''' FOR [bar]"; +ALTER TABLE [foo1] ADD CONSTRAINT [DF_foo1_bar] DEFAULT 'AbCdE' FOR [bar]"; $sql = $qb->alterColumn('foo1', 'bar', $this->string(255)->defaultValue('AbCdE')); $this->assertEquals($expected, $sql); @@ -606,7 +606,7 @@ WHILE 1=1 BEGIN IF @constraintName IS NULL BREAK EXEC (N'ALTER TABLE ' + @tableName + ' DROP CONSTRAINT [' + @constraintName + ']') END -ALTER TABLE [foo1] ADD CONSTRAINT [DF_foo1_bar] DEFAULT 'CURRENT_TIMESTAMP' FOR [bar]"; +ALTER TABLE [foo1] ADD CONSTRAINT [DF_foo1_bar] DEFAULT CURRENT_TIMESTAMP FOR [bar]"; $sql = $qb->alterColumn('foo1', 'bar', $this->timestamp()->defaultExpression('CURRENT_TIMESTAMP')); $this->assertEquals($expected, $sql); @@ -656,6 +656,70 @@ ALTER TABLE [foo1] ADD CONSTRAINT [UQ_foo1_bar] UNIQUE ([bar])"; $this->assertEquals(false, $schema->getColumn('bar')->allowNull); } + public function testAlterColumnWithNull() + { + $qb = $this->getQueryBuilder(); + + $expected = "ALTER TABLE [foo1] ALTER COLUMN [bar] int NULL +DECLARE @tableName VARCHAR(MAX) = '[foo1]' +DECLARE @columnName VARCHAR(MAX) = 'bar' + +WHILE 1=1 BEGIN + DECLARE @constraintName NVARCHAR(128) + SET @constraintName = (SELECT TOP 1 OBJECT_NAME(cons.[object_id]) + FROM ( + SELECT sc.[constid] object_id + FROM [sys].[sysconstraints] sc + JOIN [sys].[columns] c ON c.[object_id]=sc.[id] AND c.[column_id]=sc.[colid] AND c.[name]=@columnName + WHERE sc.[id] = OBJECT_ID(@tableName) + UNION + SELECT object_id(i.[name]) FROM [sys].[indexes] i + JOIN [sys].[columns] c ON c.[object_id]=i.[object_id] AND c.[name]=@columnName + JOIN [sys].[index_columns] ic ON ic.[object_id]=i.[object_id] AND i.[index_id]=ic.[index_id] AND c.[column_id]=ic.[column_id] + WHERE i.[is_unique_constraint]=1 and i.[object_id]=OBJECT_ID(@tableName) + ) cons + JOIN [sys].[objects] so ON so.[object_id]=cons.[object_id] + WHERE so.[type]='D') + IF @constraintName IS NULL BREAK + EXEC (N'ALTER TABLE ' + @tableName + ' DROP CONSTRAINT [' + @constraintName + ']') +END +ALTER TABLE [foo1] ADD CONSTRAINT [DF_foo1_bar] DEFAULT NULL FOR [bar]"; + $sql = $qb->alterColumn('foo1', 'bar', $this->integer()->null()->defaultValue(NULL)); + $this->assertEquals($expected, $sql); + } + + public function testAlterColumnWithExpression() + { + $qb = $this->getQueryBuilder(); + + $expected = "ALTER TABLE [foo1] ALTER COLUMN [bar] int NULL +DECLARE @tableName VARCHAR(MAX) = '[foo1]' +DECLARE @columnName VARCHAR(MAX) = 'bar' + +WHILE 1=1 BEGIN + DECLARE @constraintName NVARCHAR(128) + SET @constraintName = (SELECT TOP 1 OBJECT_NAME(cons.[object_id]) + FROM ( + SELECT sc.[constid] object_id + FROM [sys].[sysconstraints] sc + JOIN [sys].[columns] c ON c.[object_id]=sc.[id] AND c.[column_id]=sc.[colid] AND c.[name]=@columnName + WHERE sc.[id] = OBJECT_ID(@tableName) + UNION + SELECT object_id(i.[name]) FROM [sys].[indexes] i + JOIN [sys].[columns] c ON c.[object_id]=i.[object_id] AND c.[name]=@columnName + JOIN [sys].[index_columns] ic ON ic.[object_id]=i.[object_id] AND i.[index_id]=ic.[index_id] AND c.[column_id]=ic.[column_id] + WHERE i.[is_unique_constraint]=1 and i.[object_id]=OBJECT_ID(@tableName) + ) cons + JOIN [sys].[objects] so ON so.[object_id]=cons.[object_id] + WHERE so.[type]='D') + IF @constraintName IS NULL BREAK + EXEC (N'ALTER TABLE ' + @tableName + ' DROP CONSTRAINT [' + @constraintName + ']') +END +ALTER TABLE [foo1] ADD CONSTRAINT [DF_foo1_bar] DEFAULT CAST(GETDATE() AS INT) FOR [bar]"; + $sql = $qb->alterColumn('foo1', 'bar', $this->integer()->null()->defaultValue(new Expression('CAST(GETDATE() AS INT)'))); + $this->assertEquals($expected, $sql); + } + public function testAlterColumnWithCheckConstraintOnDb() { $connection = $this->getConnection();