Fix alter column type JSON to nVarchar

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,17 @@ protected override void Generate(
 
         if (alterStatementNeeded)
         {
-            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
+            // SQL Server can't ALTER COLUMN from json to a non JSON type; use rename-add-copy-drop instead. See #38364.
+            if ((oldType ?? operation.OldColumn.ColumnType)
+                    ?.Equals("json", StringComparison.OrdinalIgnoreCase) == true
+                && !columnType.Equals("json", StringComparison.OrdinalIgnoreCase))
             {
-                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));
-
-            ColumnDefinition(
-                operation.Schema,
-                operation.Table,
-                operation.Name,
-                definitionOperation,
-                model,
-                builder);
-
-            builder.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
+                AlterColumnFromJson(operation, columnType, model, builder);
+            }
+            else
+            {
+                AppendAlterColumnDefinition(operation, operation.IsNullable, model, builder);
+            }
         }
 
         if (!Equals(operation.DefaultValue, oldDefaultValue) || operation.DefaultValueSql != oldDefaultValueSql)
@@ -514,6 +489,134 @@ protected override void Generate(
         builder.EndCommand(suppressTransaction: IsMemoryOptimized(operation, model, operation.Schema, operation.Table));
     }
 
+    private void AlterColumnFromJson(
+        AlterColumnOperation operation,
+        string columnType,
+        IModel? model,
+        MigrationCommandListBuilder builder)
+    {
+        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 ");
+
+        var addColumnOperation = 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,
+            Comment = operation.Comment
+        };
+        addColumnOperation.AddAnnotations(
+            operation.GetAnnotations().Where(a => a.Name != SqlServerAnnotationNames.Identity));
+
+        ColumnDefinition(
+            operation.Schema,
+            operation.Table,
+            operation.Name,
+            addColumnOperation,
+            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);
+
+        if (!operation.IsNullable)
+        {
+            AppendAlterColumnDefinition(operation, false, model, builder);
+        }
+    }
+
+    private void AppendAlterColumnDefinition(
+        AlterColumnOperation operation,
+        bool isNullable,
+        IModel? model,
+        MigrationCommandListBuilder builder)
+    {
+        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 = isNullable,
+            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);
+    }
+
     /// <summary>
     ///     Builds commands for the given <see cref="RenameIndexOperation" />
     ///     by making calls on the given <see cref="MigrationCommandListBuilder" />.

test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs

@@ -1599,6 +1599,74 @@ 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<int>("Id").ValueGeneratedNever();
+                        e.HasKey("Id");
+                        e.Property<string>("Name").HasColumnType("json");
+                    });
+            },
+            new MigrationOperation[]
+            {
+                new AlterColumnOperation
+                {
+                    Table = "Entity",
+                    Name = "Name",
+                    ClrType = typeof(string),
+                    ColumnType = "nvarchar(450)",
+                    IsNullable = true,
+                    OldColumn = new AddColumnOperation
+                    {
+                        ClrType = typeof(string),
+                        ColumnType = "json",
+                        IsNullable = true
+                    }
+                },
+                new CreateIndexOperation
+                {
+                    Table = "Entity",
+                    Name = "IX_Entity_Name",
+                    Columns = new[] { "Name" }
+                }
+            },
+            model =>
+            {
+                var table = Assert.Single(model.Tables);
+                var column = Assert.Single(table.Columns, c => c.Name == "Name");
+                Assert.Equal("nvarchar(450)", column.StoreType);
+                Assert.True(column.IsNullable);
+                var index = Assert.Single(table.Indexes);
+                Assert.Contains(column, index.Columns);
+            });
+
+        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(450) NULL;
+EXEC(N'UPDATE [Entity] SET [Name] = CONVERT(nvarchar(450), [ef_temp_Name])');
+ALTER TABLE [Entity] DROP COLUMN [ef_temp_Name];
+""",
+            //
+            """
+CREATE INDEX [IX_Entity_Name] ON [Entity] ([Name]);
+""");
+    }
+
     [Fact]
     public virtual async Task Alter_column_make_required_with_index_with_included_properties()
     {