From ee3306963ba1bb79355a3c166b6074753cc8717d Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Fri, 26 Aug 2022 19:24:14 +0300 Subject: [PATCH] Allow making a column non-nullable with SQL Server (#28890) Closes #21765 --- .../SqlServerMigrationsSqlGenerator.cs | 36 +++++++++++++++++++ .../Migrations/MigrationsTestBase.cs | 23 ++++++++++++ .../Migrations/MigrationsSqlServerTest.cs | 22 ++++++++++++ 3 files changed, 81 insertions(+) diff --git a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs index 03f4ce80b9d..d8d9a828beb 100644 --- a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs +++ b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs @@ -343,6 +343,42 @@ protected override void Generate( (oldDefaultValue, oldDefaultValueSql) = (null, null); } + // The column is being made non-nullable. Generate an update statement before doing that, to convert any existing null values to + // the default value (otherwise SQL Server fails). + if (!operation.IsNullable + && operation.OldColumn.IsNullable + && (operation.DefaultValueSql is not null || operation.DefaultValue is not null)) + { + string defaultValueSql; + if (operation.DefaultValueSql is not null) + { + defaultValueSql = operation.DefaultValueSql; + } + else + { + Check.DebugAssert(operation.DefaultValue is not null, "operation.DefaultValue is not null"); + + var typeMapping = (columnType != null + ? Dependencies.TypeMappingSource.FindMapping(operation.DefaultValue.GetType(), columnType) + : null) + ?? Dependencies.TypeMappingSource.GetMappingForValue(operation.DefaultValue); + + defaultValueSql = typeMapping.GenerateSqlLiteral(operation.DefaultValue); + } + + builder + .Append("UPDATE ") + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema)) + .Append(" SET ") + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Name)) + .Append(" = ") + .Append(defaultValueSql) + .Append(" WHERE ") + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Name)) + .Append(" IS NULL") + .AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator); + } + if (alterStatementNeeded) { builder diff --git a/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs b/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs index 4a6f1e852d1..42aee11f319 100644 --- a/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs @@ -690,6 +690,29 @@ public virtual Task Alter_column_make_required() Assert.False(column.IsNullable); }); + [ConditionalFact] + public virtual Task Alter_column_make_required_with_null_data() + => Test( + builder => builder.Entity( + "People", e => + { + e.Property("Id"); + e.Property("SomeColumn"); + e.HasData(new Dictionary + { + { "Id", 1 }, + { "SomeColumn", null } + }); + }), + builder => { }, + builder => builder.Entity("People").Property("SomeColumn").IsRequired(), + model => + { + var table = Assert.Single(model.Tables); + var column = Assert.Single(table.Columns, c => c.Name != "Id"); + Assert.False(column.IsNullable); + }); + [ConditionalFact] public virtual Task Alter_column_make_required_with_index() => Test( diff --git a/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs index 246503ab212..63e98ed88d1 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs @@ -861,6 +861,23 @@ FROM [sys].[default_constraints] [d] INNER JOIN [sys].[columns] [c] ON [d].[parent_column_id] = [c].[column_id] AND [d].[parent_object_id] = [c].[object_id] WHERE ([d].[parent_object_id] = OBJECT_ID(N'[People]') AND [c].[name] = N'SomeColumn'); IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [People] DROP CONSTRAINT [' + @var0 + '];'); +UPDATE [People] SET [SomeColumn] = N'' WHERE [SomeColumn] IS NULL; +ALTER TABLE [People] ALTER COLUMN [SomeColumn] nvarchar(max) NOT NULL; +ALTER TABLE [People] ADD DEFAULT N'' FOR [SomeColumn];"); + } + + public override async Task Alter_column_make_required_with_null_data() + { + await base.Alter_column_make_required_with_null_data(); + + AssertSql( + @"DECLARE @var0 sysname; +SELECT @var0 = [d].[name] +FROM [sys].[default_constraints] [d] +INNER JOIN [sys].[columns] [c] ON [d].[parent_column_id] = [c].[column_id] AND [d].[parent_object_id] = [c].[object_id] +WHERE ([d].[parent_object_id] = OBJECT_ID(N'[People]') AND [c].[name] = N'SomeColumn'); +IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [People] DROP CONSTRAINT [' + @var0 + '];'); +UPDATE [People] SET [SomeColumn] = N'' WHERE [SomeColumn] IS NULL; ALTER TABLE [People] ALTER COLUMN [SomeColumn] nvarchar(max) NOT NULL; ALTER TABLE [People] ADD DEFAULT N'' FOR [SomeColumn];"); } @@ -878,6 +895,7 @@ FROM [sys].[default_constraints] [d] INNER JOIN [sys].[columns] [c] ON [d].[parent_column_id] = [c].[column_id] AND [d].[parent_object_id] = [c].[object_id] WHERE ([d].[parent_object_id] = OBJECT_ID(N'[People]') AND [c].[name] = N'SomeColumn'); IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [People] DROP CONSTRAINT [' + @var0 + '];'); +UPDATE [People] SET [SomeColumn] = N'' WHERE [SomeColumn] IS NULL; ALTER TABLE [People] ALTER COLUMN [SomeColumn] nvarchar(450) NOT NULL; ALTER TABLE [People] ADD DEFAULT N'' FOR [SomeColumn]; CREATE INDEX [IX_People_SomeColumn] ON [People] ([SomeColumn]);"); @@ -896,6 +914,7 @@ FROM [sys].[default_constraints] [d] INNER JOIN [sys].[columns] [c] ON [d].[parent_column_id] = [c].[column_id] AND [d].[parent_object_id] = [c].[object_id] WHERE ([d].[parent_object_id] = OBJECT_ID(N'[People]') AND [c].[name] = N'FirstName'); IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [People] DROP CONSTRAINT [' + @var0 + '];'); +UPDATE [People] SET [FirstName] = N'' WHERE [FirstName] IS NULL; ALTER TABLE [People] ALTER COLUMN [FirstName] nvarchar(450) NOT NULL; ALTER TABLE [People] ADD DEFAULT N'' FOR [FirstName]; CREATE INDEX [IX_People_FirstName_LastName] ON [People] ([FirstName], [LastName]);"); @@ -1127,6 +1146,7 @@ FROM [sys].[default_constraints] [d] INNER JOIN [sys].[columns] [c] ON [d].[parent_column_id] = [c].[column_id] AND [d].[parent_object_id] = [c].[object_id] WHERE ([d].[parent_object_id] = OBJECT_ID(N'[People]') AND [c].[name] = N'SomeColumn'); IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [People] DROP CONSTRAINT [' + @var0 + '];'); +UPDATE [People] SET [SomeColumn] = N'' WHERE [SomeColumn] IS NULL; ALTER TABLE [People] ALTER COLUMN [SomeColumn] nvarchar(450) NOT NULL; ALTER TABLE [People] ADD DEFAULT N'' FOR [SomeColumn]; CREATE INDEX [IX_People_SomeColumn] ON [People] ([SomeColumn]) INCLUDE ([SomeOtherColumn]);"); @@ -2053,6 +2073,7 @@ FROM [sys].[default_constraints] [d] INNER JOIN [sys].[columns] [c] ON [d].[parent_column_id] = [c].[column_id] AND [d].[parent_object_id] = [c].[object_id] WHERE ([d].[parent_object_id] = OBJECT_ID(N'[People]') AND [c].[name] = N'SomeField'); IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [People] DROP CONSTRAINT [' + @var0 + '];'); +UPDATE [People] SET [SomeField] = N'' WHERE [SomeField] IS NULL; ALTER TABLE [People] ALTER COLUMN [SomeField] nvarchar(450) NOT NULL; ALTER TABLE [People] ADD DEFAULT N'' FOR [SomeField];", // @@ -7921,6 +7942,7 @@ FROM [sys].[default_constraints] [d] INNER JOIN [sys].[columns] [c] ON [d].[parent_column_id] = [c].[column_id] AND [d].[parent_object_id] = [c].[object_id] WHERE ([d].[parent_object_id] = OBJECT_ID(N'[Entity]') AND [c].[name] = N'Name'); IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [Entity] DROP CONSTRAINT [' + @var0 + '];'); +UPDATE [Entity] SET [Name] = N'' WHERE [Name] IS NULL; ALTER TABLE [Entity] ALTER COLUMN [Name] nvarchar(max) NOT NULL; ALTER TABLE [Entity] ADD DEFAULT N'' FOR [Name];"); }