From 6b3b8529579a43ade47da726d21dc08b64b8cb33 Mon Sep 17 00:00:00 2001
From: Shay Rojansky <roji@roji.org>
Date: Tue, 22 Nov 2022 00:43:53 +0100
Subject: [PATCH] Fix making column required on SQL Server with idempotent
 migrations (#29619)

Fixes #29530
---
 .../SqlServerMigrationsSqlGenerator.cs        | 19 ++++++++--
 .../MigrationsSqlGeneratorTestBase.cs         |  3 ++
 .../SqlServerMigrationsSqlGeneratorTest.cs    | 35 +++++++++++++++++++
 3 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs
index 5ad499c5698..17538b4c676 100644
--- a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs
+++ b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs
@@ -369,7 +369,7 @@ protected override void Generate(
                 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(
                 .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
index 87d089f75fa..9aaaa4b2fb4 100644
--- a/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsSqlGeneratorTestBase.cs
+++ b/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsSqlGeneratorTestBase.cs
@@ -750,6 +750,9 @@ protected MigrationsSqlGeneratorTestBase(
         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
index 1303ce6615d..4d59e2c4911 100644
--- 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 =>