Fix making column required on SQL Server with idempotent migrations (d…

…otnet#29619) Fixes dotnet#29530
roji · Nov 21, 2022 · 6b3b852 · 6b3b852
1 parent 7745e6a
commit 6b3b852
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 3 deletions.
diff --git a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs
@@ -369,7 +369,7 @@ protected override void Generate(AddCheckConstraintOperation operation, IModel?
                 defaultValueSql = typeMapping.GenerateSqlLiteral(operation.DefaultValue);
             }
 
-            builder
+            var updateBuilder = new StringBuilder()
                 .Append("UPDATE ")
                 .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema))
                 .Append(" SET ")
@@ -378,8 +378,21 @@ protected override void Generate(AddCheckConstraintOperation operation, IModel?
                 .Append(defaultValueSql)
                 .Append(" WHERE ")
                 .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Name))
-                .Append(" IS NULL")
-                .AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
+                .Append(" IS NULL");
+
+            if (Options.HasFlag(MigrationsSqlGenerationOptions.Idempotent))
+            {
+                builder
+                    .Append("EXEC(N'")
+                    .Append(updateBuilder.ToString().TrimEnd('\n', '\r', ';').Replace("'", "''"))
+                    .Append("')");
+            }
+            else
+            {
+                builder.Append(updateBuilder.ToString());
+            }
+
+            builder.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
         }
 
         if (alterStatementNeeded)

diff --git a/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsSqlGeneratorTestBase.cs b/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsSqlGeneratorTestBase.cs
@@ -750,6 +750,9 @@ private static void CreateGotModel(ModelBuilder b)
         ContextOptions = options;
     }
 
+    protected virtual void Generate(MigrationOperation operation, MigrationsSqlGenerationOptions options)
+        => Generate(null, new[] { operation }, options);
+
     protected virtual void Generate(params MigrationOperation[] operation)
         => Generate(null, operation);
 

diff --git a/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs b/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs
@@ -1180,6 +1180,41 @@ public virtual void CreateIndex_generates_exec_when_legacy_filter_and_idempotent
 """);
     }
 
+    [ConditionalFact]
+    public virtual void AlterColumn_make_required_with_idempotent()
+    {
+        Generate(
+            new AlterColumnOperation
+            {
+                Table = "Person",
+                Name = "Name",
+                ClrType = typeof(string),
+                IsNullable = false,
+                DefaultValue = "",
+                OldColumn = new AddColumnOperation
+                {
+                    Table = "Person",
+                    Name = "Name",
+                    ClrType = typeof(string),
+                    IsNullable = true
+                }
+            },
+            MigrationsSqlGenerationOptions.Idempotent);
+
+        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'[Person]') AND [c].[name] = N'Name');
+IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [Person] DROP CONSTRAINT [' + @var0 + '];');
+EXEC(N'UPDATE [Person] SET [Name] = N'''' WHERE [Name] IS NULL');
+ALTER TABLE [Person] ALTER COLUMN [Name] nvarchar(max) NOT NULL;
+ALTER TABLE [Person] ADD DEFAULT N'' FOR [Name];
+""");
+    }
+
     private static void CreateGotModel(ModelBuilder b)
         => b.HasDefaultSchema("dbo").Entity(
             "Person", pb =>