From e121bddffc5e183b668b6f455d515806b8d18687 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Mon, 1 Mar 2021 19:50:23 +0100 Subject: [PATCH 1/2] Fix default constraint logic in SQL Server migrations (#24274) Fixes #24272 (cherry picked from commit 2eabf1ff774ea16c92041cb58de065e170e98f60) --- .../SqlServerMigrationsSqlGenerator.cs | 10 ++++---- .../MigrationsSqlServerTest.cs | 23 +++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs index acff3ed46f7..06f084f7d3c 100644 --- a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs +++ b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs @@ -339,11 +339,14 @@ protected override void Generate(AddCheckConstraintOperation operation, IModel m || 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(AddCheckConstraintOperation operation, IModel m 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/MigrationsSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/MigrationsSqlServerTest.cs index b54e54542ab..bb709b6250b 100644 --- a/test/EFCore.SqlServer.FunctionalTests/MigrationsSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/MigrationsSqlServerTest.cs @@ -945,6 +945,29 @@ public virtual async Task Alter_column_change_default() 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("Name").HasDefaultValue("Doe"), + builder => { }, + builder => builder.Entity("People").Property("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';"); + } + public override async Task Drop_column() { await base.Drop_column(); From 8997cd70fd1d5a715f0bef89c268c50fd5a82d82 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Mon, 1 Mar 2021 20:06:42 +0100 Subject: [PATCH 2/2] Quirk for #24272 --- .../Migrations/SqlServerMigrationsSqlGenerator.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs index 06f084f7d3c..0d1299d90a3 100644 --- a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs +++ b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs @@ -38,6 +38,9 @@ public class SqlServerMigrationsSqlGenerator : MigrationsSqlGenerator private IReadOnlyList _operations; private int _variableCounter; + private static readonly bool _useOldAlterColumnDefaultValueLogic = + AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue24272", out var enabled) && enabled; + /// /// Creates a new instance. /// @@ -391,7 +394,11 @@ protected override void Generate(AddCheckConstraintOperation operation, IModel m builder.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator); } - if (!Equals(operation.DefaultValue, oldDefaultValue) || operation.DefaultValueSql != oldDefaultValueSql) + var addDefaultValue = _useOldAlterColumnDefaultValueLogic + ? operation.DefaultValue != null || operation.DefaultValueSql != null + : !Equals(operation.DefaultValue, oldDefaultValue) || operation.DefaultValueSql != oldDefaultValueSql; + + if (addDefaultValue) { builder .Append("ALTER TABLE ")