Fix default constraint logic in SQL Server migrations (#24274)

Fixes #24272
dotnet · Mar 1, 2021 · 2eabf1f · 2eabf1f
1 parent c65e02d
commit 2eabf1f
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 4 deletions.
diff --git a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs
@@ -339,11 +339,14 @@ protected override void Generate(
                 || operation.Collation != operation.OldColumn.Collation
                 || HasDifferences(operation.GetAnnotations(), operation.OldColumn.GetAnnotations());
 
+            var (oldDefaultValue, oldDefaultValueSql) = (operation.OldColumn.DefaultValue, operation.OldColumn.DefaultValueSql);
+
             if (alterStatementNeeded
-                || !Equals(operation.DefaultValue, operation.OldColumn.DefaultValue)
-                || operation.DefaultValueSql != operation.OldColumn.DefaultValueSql)
+                || !Equals(operation.DefaultValue, oldDefaultValue)
+                || operation.DefaultValueSql != oldDefaultValueSql)
             {
                 DropDefaultConstraint(operation.Schema, operation.Table, operation.Name, builder);
+                (oldDefaultValue, oldDefaultValueSql) = (null, null);
             }
 
             if (alterStatementNeeded)
@@ -388,8 +391,7 @@ protected override void Generate(
                 builder.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
             }
 
-            if (operation.DefaultValue != null
-                || operation.DefaultValueSql != null)
+            if (!Equals(operation.DefaultValue, oldDefaultValue) || operation.DefaultValueSql != oldDefaultValueSql)
             {
                 builder
                     .Append("ALTER TABLE ")

diff --git a/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs
@@ -964,6 +964,29 @@ FROM [sys].[default_constraints] [d]
 ALTER TABLE [People] ADD DEFAULT N'Doe' FOR [Name];");
         }
 
+        [ConditionalFact]
+        public virtual async Task Alter_column_change_comment_with_default()
+        {
+            await Test(
+                builder => builder.Entity("People").Property<string>("Name").HasDefaultValue("Doe"),
+                builder => { },
+                builder => builder.Entity("People").Property<string>("Name")
+                    .HasComment("Some comment"),
+                model =>
+                {
+                    var nameColumn = Assert.Single(Assert.Single(model.Tables).Columns);
+                    Assert.Equal("(N'Doe')", nameColumn.DefaultValueSql);
+                    Assert.Equal("Some comment", nameColumn.Comment);
+                });
+
+            AssertSql(
+                @"DECLARE @defaultSchema AS sysname;
+SET @defaultSchema = SCHEMA_NAME();
+DECLARE @description AS sql_variant;
+SET @description = N'Some comment';
+EXEC sp_addextendedproperty 'MS_Description', @description, 'SCHEMA', @defaultSchema, 'TABLE', N'People', 'COLUMN', N'Name';");
+        }
+
         [ConditionalFact]
         public virtual async Task Alter_column_make_sparse()
         {