From 0cdb0048971fedd0509a5e4faeb455a5391e7965 Mon Sep 17 00:00:00 2001 From: Atiqur Rahman Foyshal Date: Fri, 19 Jun 2026 13:34:07 +0600 Subject: [PATCH] Fix alter column type JSON to nVarchar --- .../SqlServerMigrationsSqlGenerator.cs | 184 ++++++++++++++---- .../Migrations/MigrationsSqlServerTest.cs | 53 +++++ .../SqlServerMigrationsSqlGeneratorTest.cs | 138 +++++++++++++ 3 files changed, 341 insertions(+), 34 deletions(-) diff --git a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs index 83ec82f5399..33b62cbae84 100644 --- a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs +++ b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs @@ -294,6 +294,7 @@ protected override void Generate( var narrowed = false; var oldColumnSupported = IsOldColumnSupported(model); + string? oldType = null; // SQL Server can't ALTER COLUMN on a computed column when the expression is unchanged; see #33425. var computedColumnIsNoOp = operation.ComputedColumnSql != null @@ -308,7 +309,7 @@ protected override void Generate( throw new InvalidOperationException(SqlServerStrings.AlterIdentityColumn); } - var oldType = operation.OldColumn.ColumnType + oldType = operation.OldColumn.ColumnType ?? GetColumnType( operation.Schema, operation.Table, @@ -429,43 +430,158 @@ protected override void Generate( if (alterStatementNeeded) { - builder - .Append("ALTER TABLE ") - .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema)) - .Append(" ALTER COLUMN "); + // SQL Server can't ALTER COLUMN from json to a character type; use rename-add-copy-drop instead. See #38364. + if ((oldType ?? operation.OldColumn.ColumnType) + ?.Equals("json", StringComparison.OrdinalIgnoreCase) == true + && !columnType.Equals("json", StringComparison.OrdinalIgnoreCase)) + { + var tempColumnName = "ef_temp_" + operation.Name; + + Rename( + Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema) + + "." + + Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Name), + tempColumnName, + "COLUMN", + builder); + + builder + .Append("ALTER TABLE ") + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema)) + .Append(" ADD "); + + ColumnDefinition( + operation.Schema, + operation.Table, + operation.Name, + new AddColumnOperation + { + Schema = operation.Schema, + Table = operation.Table, + Name = operation.Name, + ClrType = operation.ClrType, + ColumnType = operation.ColumnType, + IsUnicode = operation.IsUnicode, + IsFixedLength = operation.IsFixedLength, + MaxLength = operation.MaxLength, + Precision = operation.Precision, + Scale = operation.Scale, + IsRowVersion = operation.IsRowVersion, + IsNullable = true, + Collation = operation.Collation + }, + model, + builder); + + builder.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator); + + var updateSql = new StringBuilder() + .Append("UPDATE ") + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema)) + .Append(" SET ") + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Name)) + .Append(" = CONVERT(") + .Append(columnType) + .Append(", ") + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(tempColumnName)) + .Append(")") + .ToString(); + + builder + .Append("EXEC(N'") + .Append(updateSql.Replace("'", "''")) + .Append("')"); + + builder.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator); + + builder + .Append("ALTER TABLE ") + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema)) + .Append(" DROP COLUMN ") + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(tempColumnName)) + .AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator); - // NB: ComputedColumnSql, IsStored, DefaultValue, DefaultValueSql, Comment, ValueGenerationStrategy, and Identity are - // handled elsewhere. Don't copy them here. - var definitionOperation = new AlterColumnOperation + if (!operation.IsNullable) + { + builder + .Append("ALTER TABLE ") + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema)) + .Append(" ALTER COLUMN "); + + // NB: ComputedColumnSql, IsStored, DefaultValue, DefaultValueSql, Comment, ValueGenerationStrategy, and Identity are + // handled elsewhere. Don't copy them here. + var definitionOperation = new AlterColumnOperation + { + Schema = operation.Schema, + Table = operation.Table, + Name = operation.Name, + ClrType = operation.ClrType, + ColumnType = operation.ColumnType, + IsUnicode = operation.IsUnicode, + IsFixedLength = operation.IsFixedLength, + MaxLength = operation.MaxLength, + Precision = operation.Precision, + Scale = operation.Scale, + IsRowVersion = operation.IsRowVersion, + IsNullable = false, + Collation = operation.Collation, + OldColumn = operation.OldColumn + }; + definitionOperation.AddAnnotations( + operation.GetAnnotations().Where(a => a.Name != SqlServerAnnotationNames.ValueGenerationStrategy + && a.Name != SqlServerAnnotationNames.Identity)); + + ColumnDefinition( + operation.Schema, + operation.Table, + operation.Name, + definitionOperation, + model, + builder); + + builder.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator); + } + } + else { - Schema = operation.Schema, - Table = operation.Table, - Name = operation.Name, - ClrType = operation.ClrType, - ColumnType = operation.ColumnType, - IsUnicode = operation.IsUnicode, - IsFixedLength = operation.IsFixedLength, - MaxLength = operation.MaxLength, - Precision = operation.Precision, - Scale = operation.Scale, - IsRowVersion = operation.IsRowVersion, - IsNullable = operation.IsNullable, - Collation = operation.Collation, - OldColumn = operation.OldColumn - }; - definitionOperation.AddAnnotations( - operation.GetAnnotations().Where(a => a.Name != SqlServerAnnotationNames.ValueGenerationStrategy - && a.Name != SqlServerAnnotationNames.Identity)); + builder + .Append("ALTER TABLE ") + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema)) + .Append(" ALTER COLUMN "); - ColumnDefinition( - operation.Schema, - operation.Table, - operation.Name, - definitionOperation, - model, - builder); + // NB: ComputedColumnSql, IsStored, DefaultValue, DefaultValueSql, Comment, ValueGenerationStrategy, and Identity are + // handled elsewhere. Don't copy them here. + var definitionOperation = new AlterColumnOperation + { + Schema = operation.Schema, + Table = operation.Table, + Name = operation.Name, + ClrType = operation.ClrType, + ColumnType = operation.ColumnType, + IsUnicode = operation.IsUnicode, + IsFixedLength = operation.IsFixedLength, + MaxLength = operation.MaxLength, + Precision = operation.Precision, + Scale = operation.Scale, + IsRowVersion = operation.IsRowVersion, + IsNullable = operation.IsNullable, + Collation = operation.Collation, + OldColumn = operation.OldColumn + }; + definitionOperation.AddAnnotations( + operation.GetAnnotations().Where(a => a.Name != SqlServerAnnotationNames.ValueGenerationStrategy + && a.Name != SqlServerAnnotationNames.Identity)); - builder.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator); + ColumnDefinition( + operation.Schema, + operation.Table, + operation.Name, + definitionOperation, + model, + builder); + + builder.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator); + } } if (!Equals(operation.DefaultValue, oldDefaultValue) || operation.DefaultValueSql != oldDefaultValueSql) diff --git a/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs index 6f641d2868e..90c1364d238 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs @@ -1599,6 +1599,59 @@ public override async Task Convert_string_column_to_a_json_column_containing_col AssertSql(); } + [ConditionalFact(typeof(SqlServerTestEnvironment), nameof(SqlServerTestEnvironment.IsJsonTypeSupported))] + public virtual async Task Convert_json_column_back_to_string_column() + { + // Use explicit operation rather than model diffing: the snapshot round-trip of a source model + // with HasColumnType("json") does not reliably preserve identity annotations, causing the + // model differ to emit a spurious AlterColumnOperation for the PK that hits the identity check. + await Test( + builder => + { + builder.Entity( + "Entity", e => + { + e.Property("Id").ValueGeneratedNever(); + e.HasKey("Id"); + e.Property("Name").HasColumnType("json"); + }); + }, + new AlterColumnOperation + { + Table = "Entity", + Name = "Name", + ClrType = typeof(string), + ColumnType = "nvarchar(max)", + IsNullable = true, + OldColumn = new AddColumnOperation + { + ClrType = typeof(string), + ColumnType = "json", + IsNullable = true + } + }, + model => + { + var table = Assert.Single(model.Tables); + var column = Assert.Single(table.Columns, c => c.Name == "Name"); + Assert.Equal("nvarchar(max)", column.StoreType); + Assert.True(column.IsNullable); + }); + + AssertSql( + """ +DECLARE @var nvarchar(max); +SELECT @var = QUOTENAME(OBJECT_NAME([c].[default_object_id])) +FROM [sys].[columns] [c] +WHERE [c].[object_id] = OBJECT_ID(N'[Entity]') AND [c].[name] = N'Name'; +IF @var IS NOT NULL EXEC(N'ALTER TABLE [Entity] DROP CONSTRAINT ' + @var + ';'); +EXEC sp_rename N'[Entity].[Name]', N'ef_temp_Name', 'COLUMN'; +ALTER TABLE [Entity] ADD [Name] nvarchar(max) NULL; +EXEC(N'UPDATE [Entity] SET [Name] = CONVERT(nvarchar(max), [ef_temp_Name])'); +ALTER TABLE [Entity] DROP COLUMN [ef_temp_Name]; +"""); + } + [Fact] public virtual async Task Alter_column_make_required_with_index_with_included_properties() { diff --git a/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs b/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs index 2567dda3368..ec1f86c91e5 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs @@ -1979,6 +1979,144 @@ public void Invalid_column_type_for_unmappable_clr_type_throws_meaningful_except Assert.Equal(RelationalStrings.UnsupportedTypeForColumn("TestTable", "TestColumn", "FileStream"), ex.Message); } + [Fact] + public virtual void AlterColumnOperation_json_to_nvarchar_not_null() + { + Generate( + modelBuilder => modelBuilder.HasAnnotation(CoreAnnotationNames.ProductVersion, "2.1.0"), + new AlterColumnOperation + { + Table = "People", + Name = "Settings", + ClrType = typeof(string), + ColumnType = "nvarchar(max)", + IsNullable = false, + OldColumn = new AddColumnOperation + { + ClrType = typeof(string), + ColumnType = "json", + IsNullable = false + } + }); + + AssertSql( + """ +DECLARE @var nvarchar(max); +SELECT @var = QUOTENAME(OBJECT_NAME([c].[default_object_id])) +FROM [sys].[columns] [c] +WHERE [c].[object_id] = OBJECT_ID(N'[People]') AND [c].[name] = N'Settings'; +IF @var IS NOT NULL EXEC(N'ALTER TABLE [People] DROP CONSTRAINT ' + @var + ';'); +EXEC sp_rename N'[People].[Settings]', N'ef_temp_Settings', 'COLUMN'; +ALTER TABLE [People] ADD [Settings] nvarchar(max) NULL; +EXEC(N'UPDATE [People] SET [Settings] = CONVERT(nvarchar(max), [ef_temp_Settings])'); +ALTER TABLE [People] DROP COLUMN [ef_temp_Settings]; +ALTER TABLE [People] ALTER COLUMN [Settings] nvarchar(max) NOT NULL; +"""); + } + + [Fact] + public virtual void AlterColumnOperation_json_to_nvarchar_nullable() + { + Generate( + modelBuilder => modelBuilder.HasAnnotation(CoreAnnotationNames.ProductVersion, "2.1.0"), + new AlterColumnOperation + { + Table = "People", + Name = "Settings", + ClrType = typeof(string), + ColumnType = "nvarchar(max)", + IsNullable = true, + OldColumn = new AddColumnOperation + { + ClrType = typeof(string), + ColumnType = "json", + IsNullable = false + } + }); + + AssertSql( + """ +DECLARE @var nvarchar(max); +SELECT @var = QUOTENAME(OBJECT_NAME([c].[default_object_id])) +FROM [sys].[columns] [c] +WHERE [c].[object_id] = OBJECT_ID(N'[People]') AND [c].[name] = N'Settings'; +IF @var IS NOT NULL EXEC(N'ALTER TABLE [People] DROP CONSTRAINT ' + @var + ';'); +EXEC sp_rename N'[People].[Settings]', N'ef_temp_Settings', 'COLUMN'; +ALTER TABLE [People] ADD [Settings] nvarchar(max) NULL; +EXEC(N'UPDATE [People] SET [Settings] = CONVERT(nvarchar(max), [ef_temp_Settings])'); +ALTER TABLE [People] DROP COLUMN [ef_temp_Settings]; +"""); + } + + [Fact] + public virtual void AlterColumnOperation_json_to_nvarchar_idempotent() + { + Generate( + modelBuilder => modelBuilder.HasAnnotation(CoreAnnotationNames.ProductVersion, "2.1.0"), + [new AlterColumnOperation + { + Table = "People", + Name = "Settings", + ClrType = typeof(string), + ColumnType = "nvarchar(max)", + IsNullable = false, + OldColumn = new AddColumnOperation + { + ClrType = typeof(string), + ColumnType = "json", + IsNullable = false + } + }], + MigrationsSqlGenerationOptions.Idempotent); + + AssertSql( + """ +DECLARE @var nvarchar(max); +SELECT @var = QUOTENAME(OBJECT_NAME([c].[default_object_id])) +FROM [sys].[columns] [c] +WHERE [c].[object_id] = OBJECT_ID(N'[People]') AND [c].[name] = N'Settings'; +IF @var IS NOT NULL EXEC(N'ALTER TABLE [People] DROP CONSTRAINT ' + @var + ';'); +EXEC sp_rename N'[People].[Settings]', N'ef_temp_Settings', 'COLUMN'; +ALTER TABLE [People] ADD [Settings] nvarchar(max) NULL; +EXEC(N'UPDATE [People] SET [Settings] = CONVERT(nvarchar(max), [ef_temp_Settings])'); +ALTER TABLE [People] DROP COLUMN [ef_temp_Settings]; +ALTER TABLE [People] ALTER COLUMN [Settings] nvarchar(max) NOT NULL; +"""); + } + + [Fact] + public virtual void AlterColumnOperation_nvarchar_to_json_uses_alter_column() + { + // Up migration (nvarchar → json) should still use plain ALTER COLUMN since SQL Server + // allows implicit conversion from nvarchar to json. + Generate( + modelBuilder => modelBuilder.HasAnnotation(CoreAnnotationNames.ProductVersion, "2.1.0"), + new AlterColumnOperation + { + Table = "People", + Name = "Settings", + ClrType = typeof(string), + ColumnType = "json", + IsNullable = false, + OldColumn = new AddColumnOperation + { + ClrType = typeof(string), + ColumnType = "nvarchar(max)", + IsNullable = false + } + }); + + AssertSql( + """ +DECLARE @var nvarchar(max); +SELECT @var = QUOTENAME(OBJECT_NAME([c].[default_object_id])) +FROM [sys].[columns] [c] +WHERE [c].[object_id] = OBJECT_ID(N'[People]') AND [c].[name] = N'Settings'; +IF @var IS NOT NULL EXEC(N'ALTER TABLE [People] DROP CONSTRAINT ' + @var + ';'); +ALTER TABLE [People] ALTER COLUMN [Settings] json NOT NULL; +"""); + } + private static void CreateGotModel(ModelBuilder b) => b.HasDefaultSchema("dbo").Entity( "Person", pb =>