Fix owned entities with default values not saved in TPH with shared columns

Fixes #37525

Author: Copilot

src/EFCore.Relational/Update/ModificationCommand.cs

@@ -407,7 +407,7 @@ void HandleColumn(
                 {
                     // Note that for stored procedures we always need to send all parameters, regardless of whether the property
                     // actually changed.
-                    writeValue = columnPropagator?.TryPropagate(columnMapping, entry)
+                    writeValue = !columnPropagator?.TryPropagate(columnMapping, entry)
                         ?? (entry.EntityState == EntityState.Added
                             || entry.EntityState == EntityState.Deleted
                             || ColumnModification.IsModified(entry, property)
@@ -1217,16 +1217,24 @@ public void RecordValue(IColumnMapping mapping, IUpdateEntry entry)
 
                     break;
                 case EntityState.Added:
+                    var currentProviderValue = Update.ColumnModification.GetCurrentProviderValue(entry, property);
+                    if (_originalValueInitialized
+                        && mapping.Column.ProviderValueComparer.Equals(_originalValue, currentProviderValue))
+                    {
+                        // This Added entry has the same value as the original (pre-modification) value.
+                        // It will receive the new value via TryPropagate, so don't let it reset _currentValue or _write.
+                        break;
+                    }
+
                     if (_currentValue == null
                         || !property.GetValueComparer().Equals(
                             Update.ColumnModification.GetCurrentValue(entry, property),
                             property.Sentinel))
                     {
-                        _currentValue = Update.ColumnModification.GetCurrentProviderValue(entry, property);
+                        _currentValue = currentProviderValue;
                     }
 
-                    _write = !_originalValueInitialized
-                        || !mapping.Column.ProviderValueComparer.Equals(_originalValue, _currentValue);
+                    _write = true;
 
                     break;
                 case EntityState.Deleted:
@@ -1243,40 +1251,73 @@ public void RecordValue(IColumnMapping mapping, IUpdateEntry entry)
             }
         }
 
+        /// <summary>
+        ///     Attempts to propagate the recorded value from other enties in this command to the specified entry.
+        /// </summary>
+        /// <param name="mapping">The column mapping for which the value may be propagated.</param>
+        /// <param name="entry">The entry that may receive the propagated value.</param>
+        /// <returns>
+        ///     <see langword="true" /> if no value was recorded (column should not be written), or if the value was
+        ///     successfully propagated to <paramref name="entry" />.
+        ///     <see langword="false" /> if a value was recorded for this column (i.e. the column should be written)
+        ///     but propagation to <paramref name="entry" /> was skipped because the entry is the source of the write;
+        /// </returns>
         public bool TryPropagate(IColumnMappingBase mapping, IUpdateEntry entry)
         {
+            if (!_write)
+            {
+                return true;
+            }
+
             var property = mapping.Property;
-            if (_write
-                && (entry.EntityState == EntityState.Unchanged
-                    || (entry.EntityState == EntityState.Modified && !Update.ColumnModification.IsModified(entry, property))
-                    || (entry.EntityState == EntityState.Added
-                        && ((!_originalValueInitialized
-                                && property.GetValueComparer().Equals(
-                                    Update.ColumnModification.GetCurrentValue(entry, property),
-                                    property.Sentinel))
-                            || (_originalValueInitialized
-                                && mapping.Column.ProviderValueComparer.Equals(
-                                    Update.ColumnModification.GetCurrentProviderValue(entry, property),
-                                    _originalValue))))))
+            var shouldPropagate = entry.EntityState switch
             {
-                if ((property.GetAfterSaveBehavior() == PropertySaveBehavior.Save
-                        || entry.EntityState == EntityState.Added)
-                    && property.ValueGenerated != ValueGenerated.Never)
-                {
-                    var value = _currentValue;
-                    var converter = property.GetTypeMapping().Converter;
-                    if (converter != null)
-                    {
-                        value = converter.ConvertFromProvider(value);
-                    }
+                EntityState.Unchanged => true,
+                EntityState.Modified => !Update.ColumnModification.IsModified(entry, property),
+                EntityState.Added => ShouldPropagateForAddedEntry(mapping, entry, property),
+                _ => false
+            };
+
+            if (!shouldPropagate)
+            {
+                return false;
+            }
 
-                    Update.ColumnModification.SetStoreGeneratedValue(entry, property, value);
+            if ((property.GetAfterSaveBehavior() == PropertySaveBehavior.Save
+                    || entry.EntityState == EntityState.Added)
+                && property.ValueGenerated != ValueGenerated.Never)
+            {
+                var value = _currentValue;
+                var converter = property.GetTypeMapping().Converter;
+                if (converter != null)
+                {
+                    value = converter.ConvertFromProvider(value);
                 }
 
-                return false;
+                Update.ColumnModification.SetStoreGeneratedValue(entry, property, value);
             }
 
-            return _write;
+            return true;
+
+            bool ShouldPropagateForAddedEntry(IColumnMappingBase mapping, IUpdateEntry entry, IProperty property)
+            {
+                if (!_originalValueInitialized)
+                {
+                    // All entries are in the Added state, this entry still has the sentinel (default) value
+                    // and the propagated value is different — accept propagation
+                    return property.GetValueComparer().Equals(
+                            Update.ColumnModification.GetCurrentValue(entry, property),
+                            property.Sentinel)
+                        && !mapping.Column.ProviderValueComparer.Equals(
+                            _currentValue,
+                            Update.ColumnModification.GetCurrentProviderValue(entry, property));
+                }
+
+                // Entry's current value matches the original — it's stale, accept the new propagated value
+                return mapping.Column.ProviderValueComparer.Equals(
+                    Update.ColumnModification.GetCurrentProviderValue(entry, property),
+                    _originalValue);
+            }
         }
     }
 }

test/EFCore.Relational.Specification.Tests/Update/UpdatesRelationalTestBase.cs

@@ -260,6 +260,30 @@ public virtual Task Update_non_indexed_values()
             });
     }
 
+    [ConditionalTheory, InlineData(false), InlineData(true)] // Issue #37525
+    public virtual async Task Can_save_owned_entity_with_default_values_in_TPH_with_shared_columns(bool async)
+        => await ExecuteWithStrategyInTransactionAsync(
+            async context =>
+            {
+                var entity = new CrunchyNougat { Name = "Test" };
+                context.Add(entity);
+                _ = async ? await context.SaveChangesAsync() : context.SaveChanges();
+            },
+            async context =>
+            {
+                var entity = await context.Set<CrunchyNougat>().SingleAsync();
+                Assert.Null(entity.Filling);
+                entity.Filling = new NougatFilling();
+                _ = async ? await context.SaveChangesAsync() : context.SaveChanges();
+            },
+            async context =>
+            {
+                var entity = await context.Set<CrunchyNougat>().SingleAsync();
+                Assert.NotNull(entity.Filling);
+                Assert.Equal(NougatFillingKind.Unknown, entity.Filling.Kind);
+                Assert.False(entity.Filling.IsFresh);
+            });
+
     [ConditionalFact]
     public abstract void Identifiers_are_generated_correctly();
 
@@ -301,6 +325,23 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                     .HasColumnName("ZipCode");
             });
 
+            modelBuilder.Entity<CrunchyNougat>(b =>
+            {
+                b.OwnsOne(e => e.Filling, ob =>
+                {
+                    ob.Property(o => o.Kind).HasColumnName("FillingKind");
+                    ob.Property(o => o.IsFresh).HasColumnName("FillingIsFresh");
+                });
+            });
+            modelBuilder.Entity<SoftNougat>(b =>
+            {
+                b.OwnsOne(e => e.Filling, ob =>
+                {
+                    ob.Property(o => o.Kind).HasColumnName("FillingKind");
+                    ob.Property(o => o.IsFresh).HasColumnName("FillingIsFresh");
+                });
+            });
+
             modelBuilder
                 .Entity<
                     LoginEntityTypeWithAnExtremelyLongAndOverlyConvolutedNameThatIsUsedToVerifyThatTheStoreIdentifierGenerationLengthLimitIsWorkingCorrectlyDetails

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

@@ -0,0 +1,35 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.EntityFrameworkCore.TestModels.UpdatesModel;
+
+#nullable disable
+
+public abstract class Nougat
+{
+    public int Id { get; set; }
+    public string Name { get; set; }
+}
+
+public class CrunchyNougat : Nougat
+{
+    public NougatFilling Filling { get; set; }
+}
+
+public class SoftNougat : Nougat
+{
+    public NougatFilling Filling { get; set; }
+}
+
+public class NougatFilling
+{
+    public NougatFillingKind Kind { get; set; }
+    public bool IsFresh { get; set; }
+}
+
+public enum NougatFillingKind
+{
+    Unknown = 0,
+    Peanut = 1,
+    Almond = 2,
+}

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

@@ -15,6 +15,8 @@ public class UpdatesContext(DbContextOptions options) : PoolableDbContext(option
     public DbSet<ProductTableWithView> ProductTable { get; set; } = null!;
     public DbSet<ProductTableView> ProductTableView { get; set; } = null!;
     public DbSet<Rodney> Trotters { get; set; } = null!;
+    public DbSet<CrunchyNougat> CrunchyNougats { get; set; } = null!;
+    public DbSet<SoftNougat> SoftNougats { get; set; } = null!;
 
     public static Task SeedAsync(UpdatesContext context)
     {

test/EFCore.Specification.Tests/Update/UpdatesTestBase.cs

@@ -1042,6 +1042,22 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
             modelBuilder.Entity<LiftObscurer>().HasOne<Lift>().WithOne(x => x.Obscurer).HasForeignKey<LiftObscurer>(e => e.LiftId);
             modelBuilder.Entity<LiftBag>();
             modelBuilder.Entity<LiftPaper>();
+
+            modelBuilder.Entity<Nougat>();
+            modelBuilder.Entity<CrunchyNougat>(b =>
+            {
+                b.OwnsOne(e => e.Filling, ob =>
+                {
+                    ob.Property(o => o.Kind).HasConversion<string>();
+                });
+            });
+            modelBuilder.Entity<SoftNougat>(b =>
+            {
+                b.OwnsOne(e => e.Filling, ob =>
+                {
+                    ob.Property(o => o.Kind).HasConversion<string>();
+                });
+            });
         }
 
         protected override Task SeedAsync(UpdatesContext context)