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

…29619) Fixes #29530 (cherry picked from commit 6b3b852)
dotnet · Dec 6, 2022 · 166e7da · 166e7da
1 parent bf20e32
commit 166e7da
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 11 deletions.
diff --git a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs
@@ -35,6 +35,9 @@ public class SqlServerMigrationsSqlGenerator : MigrationsSqlGenerator
 
     private readonly ICommandBatchPreparer _commandBatchPreparer;
 
+    private static readonly bool QuirkEnabled29619
+        = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue29619", out var enabled) && enabled;
+
     /// <summary>
     ///     Creates a new <see cref="SqlServerMigrationsSqlGenerator" /> instance.
     /// </summary>
@@ -369,17 +372,45 @@ protected override void Generate(
                 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 (QuirkEnabled29619)
+            {
+                builder
+                    .Append("UPDATE ")
+                    .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema))
+                    .Append(" SET ")
+                    .Append(defaultValueSql)
+                    .Append(" WHERE ")
+                    .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Name))
+                    .Append(" IS NULL")
+                    .AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
+            }
+            else
+            {
+                var updateBuilder = new StringBuilder()
+                    .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");
+
+                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 @@ 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
@@ -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 =>