From a00c229e18a91029dfffa083df47d5e459542932 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Wed, 4 Jan 2023 19:31:56 +0100 Subject: [PATCH] Fix making column required on SQL Server with idempotent migrations (#29619) (#29784) Fixes #29530 (cherry picked from commit 6b3b8529579a43ade47da726d21dc08b64b8cb33) --- .../SqlServerMigrationsSqlGenerator.cs | 54 +++++++++++++++---- .../MigrationsSqlGeneratorTestBase.cs | 3 ++ .../SqlServerMigrationsSqlGeneratorTest.cs | 35 ++++++++++++ 3 files changed, 81 insertions(+), 11 deletions(-) diff --git a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs index 5ad499c5698..e3035010547 100644 --- 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; + /// /// Creates a new instance. /// @@ -369,17 +372,46 @@ protected override void Generate(AddCheckConstraintOperation operation, IModel? 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(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Name)) + .Append(" = ") + .Append(defaultValueSql) + .Append(" WHERE ") + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Name)) + .Append(" IS NULL"); + } + 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 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 @@ 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 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 =>