Update discriminator columns when PK-to-PK dependent type is changed

Fixes #29789

Also tests for #29874 and #29875

src/EFCore.Relational/Update/ModificationCommand.cs

@@ -385,7 +385,10 @@ private List<IColumnModification> GenerateColumnModifications()
 
         foreach (var entry in _entries.Where(x => !x.EntityType.IsMappedToJson()))
         {
-            var nonMainEntry = !_mainEntryAdded || entry != _entries[0];
+            var nonMainEntry = (!_mainEntryAdded || entry != _entries[0])
+                || (updating
+                    && (entry.EntityState == EntityState.Deleted
+                        || entry.EntityState == EntityState.Added));
 
             var optionalDependentWithAllNull = false;
 

test/EFCore.InMemory.FunctionalTests/UpdatesInMemoryTestBase.cs

@@ -22,17 +22,36 @@ protected override void ExecuteWithStrategyInTransaction(
         Action<UpdatesContext> nestedTestOperation1 = null,
         Action<UpdatesContext> nestedTestOperation2 = null)
     {
-        base.ExecuteWithStrategyInTransaction(testOperation, nestedTestOperation1, nestedTestOperation2);
-        Fixture.Reseed();
+        try
+        {
+            base.ExecuteWithStrategyInTransaction(testOperation, nestedTestOperation1, nestedTestOperation2);
+        }
+        finally
+        {
+            Fixture.Reseed();
+        }
     }
 
     protected override async Task ExecuteWithStrategyInTransactionAsync(
         Func<UpdatesContext, Task> testOperation,
         Func<UpdatesContext, Task> nestedTestOperation1 = null,
         Func<UpdatesContext, Task> nestedTestOperation2 = null)
     {
-        await base.ExecuteWithStrategyInTransactionAsync(testOperation, nestedTestOperation1, nestedTestOperation2);
-        Fixture.Reseed();
+        try
+        {
+            await base.ExecuteWithStrategyInTransactionAsync(testOperation, nestedTestOperation1, nestedTestOperation2);
+        }
+        finally
+        {
+            Fixture.Reseed();
+        }
+    }
+
+    // Issue #29875
+    public override Task Can_change_type_of_pk_to_pk_dependent_by_replacing_with_new_dependent(bool async)
+    {
+        return Assert.ThrowsAsync<DbUpdateConcurrencyException>(
+            () => base.Can_change_type_of_pk_to_pk_dependent_by_replacing_with_new_dependent(async));
     }
 
     public abstract class UpdatesInMemoryFixtureBase : UpdatesFixtureBase

test/EFCore.Specification.Tests/TestModels/UpdatesModel/Gift.cs

@@ -0,0 +1,30 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+#nullable enable
+
+namespace Microsoft.EntityFrameworkCore.TestModels.UpdatesModel;
+
+public class Gift
+{
+    public int Id { get; set; }
+    public string? Recipient { get; set; }
+
+    public GiftObscurer? Obscurer { get; set; }
+}
+
+public abstract class GiftObscurer
+{
+    public int Id { get; set; }
+    public string? Pattern { get; set; }
+}
+
+public class GiftBag : GiftObscurer
+{
+    public int Size { get; set; }
+}
+
+public class GiftPaper : GiftObscurer
+{
+    public int Thickness { get; set; }
+}

test/EFCore.Specification.Tests/TestModels/UpdatesModel/Lift.cs

@@ -0,0 +1,32 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+#nullable enable
+
+namespace Microsoft.EntityFrameworkCore.TestModels.UpdatesModel;
+
+public class Lift
+{
+    public int Id { get; set; }
+    public string? Recipient { get; set; }
+
+    public LiftObscurer Obscurer { get; set; } = null!;
+}
+
+public abstract class LiftObscurer
+{
+    public int Id { get; set; }
+
+    public int LiftId { get; set; }
+    public string? Pattern { get; set; }
+}
+
+public class LiftBag : LiftObscurer
+{
+    public int Size { get; set; }
+}
+
+public class LiftPaper : LiftObscurer
+{
+    public int Thickness { get; set; }
+}

test/EFCore.Specification.Tests/UpdatesTestBase.cs

@@ -59,6 +59,60 @@ public virtual async Task Can_delete_and_add_for_same_key(bool async)
                 Assert.Equal(EntityState.Detached, context.Entry(rodney1).State);
             });
 
+    [ConditionalTheory] // Issue #29789
+    [InlineData(false)]
+    [InlineData(true)]
+    public virtual async Task Can_change_type_of_pk_to_pk_dependent_by_replacing_with_new_dependent(bool async)
+        => await ExecuteWithStrategyInTransactionAsync(
+            async context =>
+            {
+                var gift = new Gift { Recipient = "Alice", Obscurer = new GiftPaper { Pattern = "Stripes" } };
+                await context.AddAsync(gift);
+                _ = async ? await context.SaveChangesAsync() : context.SaveChanges();
+            },
+            async context =>
+            {
+                var gift = await context.Set<Gift>().Include(e => e.Obscurer).SingleAsync();
+                var bag = new GiftBag { Pattern = "Gold stars" };
+                gift.Obscurer = bag;
+                _ = async ? await context.SaveChangesAsync() : context.SaveChanges();
+            },
+            async context =>
+            {
+                var gift = await context.Set<Gift>().Include(e => e.Obscurer).SingleAsync();
+
+                Assert.IsType<GiftBag>(gift.Obscurer);
+                Assert.Equal(gift.Id, gift.Obscurer.Id);
+                Assert.Single(context.Set<GiftObscurer>());
+            });
+
+    [ConditionalTheory]
+    [InlineData(false)]
+    [InlineData(true)]
+    public virtual async Task Can_change_type_of__dependent_by_replacing_with_new_dependent(bool async)
+        => await ExecuteWithStrategyInTransactionAsync(
+            async context =>
+            {
+                var lift = new Lift { Recipient = "Alice", Obscurer = new LiftPaper { Pattern = "Stripes" } };
+                await context.AddAsync(lift);
+                _ = async ? await context.SaveChangesAsync() : context.SaveChanges();
+            },
+            async context =>
+            {
+                var lift = await context.Set<Lift>().Include(e => e.Obscurer).SingleAsync();
+                var bag = new LiftBag { Pattern = "Gold stars" };
+                lift.Obscurer = bag;
+                _ = async ? await context.SaveChangesAsync() : context.SaveChanges();
+            },
+            async context =>
+            {
+                var lift = await context.Set<Lift>().Include(e => e.Obscurer).SingleAsync();
+
+                Assert.IsType<LiftBag>(lift.Obscurer);
+                Assert.Equal(lift.Id, lift.Obscurer.LiftId);
+                Assert.Single(context.Set<LiftObscurer>());
+            });
+
     [ConditionalFact]
     public virtual void Mutation_of_tracked_values_does_not_mutate_values_in_store()
     {
@@ -772,6 +826,16 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                         .WithOne(l => l.Profile)
                         .IsRequired();
                 });
+
+            modelBuilder.Entity<Gift>();
+            modelBuilder.Entity<GiftObscurer>().HasOne<Gift>().WithOne(x => x.Obscurer).HasForeignKey<GiftObscurer>(e => e.Id);
+            modelBuilder.Entity<GiftBag>();
+            modelBuilder.Entity<GiftPaper>();
+
+            modelBuilder.Entity<Lift>();
+            modelBuilder.Entity<LiftObscurer>().HasOne<Lift>().WithOne(x => x.Obscurer).HasForeignKey<LiftObscurer>(e => e.LiftId);
+            modelBuilder.Entity<LiftBag>();
+            modelBuilder.Entity<LiftPaper>();
         }
 
         protected override void Seed(UpdatesContext context)

test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerTPCTest.cs

@@ -110,8 +110,9 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
         {
             base.OnModelCreating(modelBuilder, context);
 
-            modelBuilder.Entity<Category>()
-                .UseTpcMappingStrategy();
+            modelBuilder.Entity<Category>().UseTpcMappingStrategy();
+            // modelBuilder.Entity<GiftObscurer>().UseTpcMappingStrategy(); Issue #29874
+            modelBuilder.Entity<LiftObscurer>().UseTpcMappingStrategy();
         }
     }
 }

test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerTPTTest.cs

@@ -15,6 +15,10 @@ public UpdatesSqlServerTPTTest(UpdatesSqlServerTPTFixture fixture, ITestOutputHe
     {
     }
 
+    [ConditionalTheory(Skip = "Issue #29874. Skipped because the database is in a bad state, but the test may or may not fail.")]
+    public override Task Can_change_type_of_pk_to_pk_dependent_by_replacing_with_new_dependent(bool async)
+        => Task.CompletedTask;
+
     public override void Save_with_shared_foreign_key()
     {
         base.Save_with_shared_foreign_key();
@@ -97,8 +101,9 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
         {
             base.OnModelCreating(modelBuilder, context);
 
-            modelBuilder.Entity<Category>()
-                .UseTptMappingStrategy();
+            modelBuilder.Entity<Category>().UseTptMappingStrategy();
+            modelBuilder.Entity<GiftObscurer>().UseTptMappingStrategy();
+            modelBuilder.Entity<LiftObscurer>().UseTptMappingStrategy();
         }
     }
 }