From ff5d43cd4fe77415bcc032645c19f7b3853064c6 Mon Sep 17 00:00:00 2001 From: Sergei Tigrov Date: Fri, 10 Nov 2023 11:28:29 +0700 Subject: [PATCH] Fix alter column with default null (#282) * Fix alter column with default null * Add line to CHANGELOG.md --- CHANGELOG.md | 1 + src/DDLQueryBuilder.php | 13 +- tests/CommandTest.php | 24 ++ tests/QueryBuilderTest.php | 476 +++++++++++++++++++------------------ 4 files changed, 279 insertions(+), 235 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3675b7e4..2443f51b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Bug #275: Refactor `DMLQueryBuilder`, related with yiisoft/db#746 (@Tigrov) - Bug #278: Remove `RECURSIVE` expression from CTE queries (@Tigrov) - Bug #280: Fix type boolean (@terabytesoftw) +- Bug #282: Fix `DDLQueryBuilder::alterColumn()` for columns with default null (@Tigrov) - Enh #283: Move methods from `Command` to `AbstractPdoCommand` class (@Tigrov) ## 1.0.1 July 24, 2023 diff --git a/src/DDLQueryBuilder.php b/src/DDLQueryBuilder.php index 7430d31e3..ad610ec0f 100644 --- a/src/DDLQueryBuilder.php +++ b/src/DDLQueryBuilder.php @@ -53,8 +53,7 @@ public function addDefaultValue(string $table, string $name, string $column, mix */ public function alterColumn(string $table, string $column, ColumnInterface|string $type): string { - $sqlAfter = [$this->dropConstraintsForColumn($table, $column, 'D')]; - + $sqlAfter = []; $columnName = $this->quoter->quoteColumnName($column); $tableName = $this->quoter->quoteTableName($table); $constraintBase = preg_replace('/[^a-z0-9_]/i', '', $table . '_' . $column); @@ -85,11 +84,11 @@ public function alterColumn(string $table, string $column, ColumnInterface|strin } } - return 'ALTER TABLE ' . $tableName - . ' ALTER COLUMN ' - . $columnName . ' ' - . $this->queryBuilder->getColumnType($type) . "\n" - . implode("\n", $sqlAfter); + return implode("\n", [ + $this->dropConstraintsForColumn($table, $column, 'D'), + "ALTER TABLE $tableName ALTER COLUMN $columnName {$this->queryBuilder->getColumnType($type)}", + ...$sqlAfter, + ]); } /** diff --git a/tests/CommandTest.php b/tests/CommandTest.php index 2c1896c4e..5ce86c4c1 100644 --- a/tests/CommandTest.php +++ b/tests/CommandTest.php @@ -12,6 +12,7 @@ use Yiisoft\Db\Exception\InvalidConfigException; use Yiisoft\Db\Exception\NotSupportedException; use Yiisoft\Db\Expression\Expression; +use Yiisoft\Db\Mssql\Column; use Yiisoft\Db\Mssql\Connection; use Yiisoft\Db\Mssql\Dsn; use Yiisoft\Db\Mssql\Driver; @@ -353,4 +354,27 @@ public function testShowDatabases(): void $this->assertSame('sqlsrv:Server=localhost,1433;', $db->getDriver()->getDsn()); $this->assertSame(['yiitest'], $command->showDatabases()); } + + /** @link https://github.com/yiisoft/db-migration/issues/11 */ + public function testAlterColumnWithDefaultNull() + { + $db = $this->getConnection(); + $command = $db->createCommand(); + + if ($db->getTableSchema('column_with_constraint', true) !== null) { + $command->dropTable('column_with_constraint')->execute(); + } + + $command->createTable('column_with_constraint', ['id' => 'pk'])->execute(); + $command->addColumn('column_with_constraint', 'field', (new Column('integer'))->null()->asString())->execute(); + $command->alterColumn('column_with_constraint', 'field', (new Column('string', 40))->notNull()->asString())->execute(); + + $fieldCol = $db->getTableSchema('column_with_constraint', true)->getColumn('field'); + + $this->assertFalse($fieldCol->isAllowNull()); + $this->assertNull($fieldCol->getDefaultValue()); + $this->assertSame('nvarchar(40)', $fieldCol->getDbType()); + + $command->dropTable('column_with_constraint'); + } } diff --git a/tests/QueryBuilderTest.php b/tests/QueryBuilderTest.php index 1591f9c42..cf1a397bf 100644 --- a/tests/QueryBuilderTest.php +++ b/tests/QueryBuilderTest.php @@ -602,53 +602,57 @@ public function testAlterColumn(): void $qb = $db->getQueryBuilder(); - $expected = "ALTER TABLE [foo1] ALTER COLUMN [bar] varchar(255) -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"; + $expected = <<alterColumn('foo1', 'bar', 'varchar(255)'); $this->assertEquals($expected, $sql); - $expected = "ALTER TABLE [foo1] ALTER COLUMN [bar] nvarchar(255) NOT 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"; + $expected = <<alterColumn( 'foo1', 'bar', @@ -656,29 +660,31 @@ public function testAlterColumn(): void ); $this->assertEquals($expected, $sql); - $expected = "ALTER TABLE [foo1] ALTER COLUMN [bar] nvarchar(255) -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 [CK_foo1_bar] CHECK (LEN(bar) > 5)"; + $expected = << 5) + SQL; $sql = $qb->alterColumn( 'foo1', 'bar', @@ -686,29 +692,31 @@ public function testAlterColumn(): void ); $this->assertEquals($expected, $sql); - $expected = "ALTER TABLE [foo1] ALTER COLUMN [bar] nvarchar(255) -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 '' FOR [bar]"; + $expected = <<alterColumn( 'foo1', 'bar', @@ -716,29 +724,31 @@ public function testAlterColumn(): void ); $this->assertEquals($expected, $sql); - $expected = "ALTER TABLE [foo1] ALTER COLUMN [bar] nvarchar(255) -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 'AbCdE' FOR [bar]"; + $expected = <<alterColumn( 'foo1', 'bar', @@ -746,29 +756,31 @@ public function testAlterColumn(): void ); $this->assertEquals($expected, $sql); - $expected = "ALTER TABLE [foo1] ALTER COLUMN [bar] datetime -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 CURRENT_TIMESTAMP FOR [bar]"; + $expected = <<alterColumn( 'foo1', 'bar', @@ -776,29 +788,31 @@ public function testAlterColumn(): void ); $this->assertEquals($expected, $sql); - $expected = "ALTER TABLE [foo1] ALTER COLUMN [bar] nvarchar(30) -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 [UQ_foo1_bar] UNIQUE ([bar])"; + $expected = <<alterColumn( 'foo1', 'bar', @@ -1007,29 +1021,31 @@ public function testAlterColumnWithNull(): void { $qb = $this->getConnection()->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]"; + $expected = <<alterColumn( 'foo1', 'bar', @@ -1037,29 +1053,31 @@ public function testAlterColumnWithNull(): void ); $this->assertEquals($expected, $sql); - $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]"; + $expected = <<alterColumn( 'foo1', 'bar', @@ -1076,29 +1094,31 @@ public function testAlterColumnWithExpression(): void { $qb = $this->getConnection()->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]"; + $expected = <<alterColumn( 'foo1', 'bar',