From 9f3d14818c6e48a7d3dc4fd0a5e80ba0629d8640 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 04:53:07 +0000 Subject: [PATCH 1/6] Initial plan From b799177830bb078b457adaa20687b6095b9d8646 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 04:59:51 +0000 Subject: [PATCH 2/6] Port PR #37389: Fix NullReferenceException with null complex properties in TPH Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- .../Internal/PropertyAccessorsFactory.cs | 12 ++- .../ComplexTypesTrackingSqlServerTest.cs | 89 +++++++++++++++++++ 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/src/EFCore/Metadata/Internal/PropertyAccessorsFactory.cs b/src/EFCore/Metadata/Internal/PropertyAccessorsFactory.cs index 59c46d11b10..b1128d8b157 100644 --- a/src/EFCore/Metadata/Internal/PropertyAccessorsFactory.cs +++ b/src/EFCore/Metadata/Internal/PropertyAccessorsFactory.cs @@ -302,8 +302,18 @@ public static Expression CreateMemberAccess( break; } } - else + else if (!fromDeclaringType + || !addNullCheck + || property is not IComplexProperty complexProperty + || instanceExpression.Type.IsValueType + || complexProperty.ClrType.IsValueType) { + // Disable null check for all cases except: + // - fromDeclaringType is true AND + // - addNullCheck is true AND + // - property is a complex property AND + // - instance is a reference type AND + // - complex property type is a reference type addNullCheck = false; } diff --git a/test/EFCore.SqlServer.FunctionalTests/ComplexTypesTrackingSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/ComplexTypesTrackingSqlServerTest.cs index c8523e7e4b3..b7336637835 100644 --- a/test/EFCore.SqlServer.FunctionalTests/ComplexTypesTrackingSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/ComplexTypesTrackingSqlServerTest.cs @@ -8,13 +8,102 @@ public class ComplexTypesTrackingSqlServerTest( ITestOutputHelper testOutputHelper) : ComplexTypesTrackingSqlServerTestBase(fixture, testOutputHelper) { + [ConditionalFact] + public virtual async Task Can_read_original_values_with_TPH_shared_complex_property_column_null() + { + await using var context = CreateContext(); + + await context.Database.CreateExecutionStrategy().ExecuteAsync( + context, + async context => + { + await using var transaction = await context.Database.BeginTransactionAsync(); + + context.Add(new InheritedItem1 { Name = "Item1" }); + await context.SaveChangesAsync(); + + context.ChangeTracker.Clear(); + var item = await context.Set().FirstAsync(); + var entry = context.ChangeTracker.Entries().First(); + var originalItem = (InheritedItemBase)entry.OriginalValues.ToObject(); + + Assert.NotNull(originalItem); + Assert.Equal("Item1", originalItem.Name); + }); + } + + [ConditionalFact] + public virtual async Task Can_read_original_values_with_TPH_shared_complex_property_column_with_value() + { + await using var context = CreateContext(); + + await context.Database.CreateExecutionStrategy().ExecuteAsync( + context, + async context => + { + await using var transaction = await context.Database.BeginTransactionAsync(); + + context.Add(new InheritedItem1 { Name = "Item1", SharedPrice = new SharedPrice { Amount = "10.99" } }); + await context.SaveChangesAsync(); + + context.ChangeTracker.Clear(); + var item = await context.Set().FirstAsync(); + var entry = context.ChangeTracker.Entries().First(); + var originalItem = (InheritedItemBase)entry.OriginalValues.ToObject(); + + Assert.NotNull(originalItem); + Assert.Equal("Item1", originalItem.Name); + var item1 = Assert.IsType(originalItem); + Assert.NotNull(item1.SharedPrice); + Assert.Equal("10.99", item1.SharedPrice.Amount); + }); + } + public class SqlServerFixture : SqlServerFixtureBase { protected override string StoreName => nameof(ComplexTypesTrackingSqlServerTest); + + protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context) + { + base.OnModelCreating(modelBuilder, context); + + modelBuilder.Entity().HasDiscriminator("Discriminator") + .HasValue("Item1") + .HasValue("Item2"); + + modelBuilder.Entity().ComplexProperty( + x => x.SharedPrice, + p => p.Property(a => a.Amount).HasColumnName("SharedPriceAmount")); + + modelBuilder.Entity().ComplexProperty( + x => x.SharedPrice, + p => p.Property(a => a.Amount).HasColumnName("SharedPriceAmount")); + } } } +public abstract class InheritedItemBase +{ + public int Id { get; set; } + public required string Name { get; set; } +} + +public class InheritedItem1 : InheritedItemBase +{ + public SharedPrice? SharedPrice { get; set; } +} + +public class InheritedItem2 : InheritedItemBase +{ + public SharedPrice? SharedPrice { get; set; } +} + +public class SharedPrice +{ + public required string Amount { get; init; } +} + public class ComplexTypesTrackingProxiesSqlServerTest( ComplexTypesTrackingProxiesSqlServerTest.SqlServerFixture fixture, ITestOutputHelper testOutputHelper) From e13fe858a10846d9e100143a440dc09d74d087d7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 20:27:14 +0000 Subject: [PATCH 3/6] Fix: Check if property's declaring type is complex, not if property itself is complex The NRE was occurring during SaveChanges when setting properties inside a null complex type. The original fix incorrectly checked if the property itself was a complex property, but it should check if the property is declared in a complex type. When accessing Amount inside SharedPrice, Amount is a scalar property but its declaring type (SharedPrice) is a complex type. Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- src/EFCore/Metadata/Internal/PropertyAccessorsFactory.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/EFCore/Metadata/Internal/PropertyAccessorsFactory.cs b/src/EFCore/Metadata/Internal/PropertyAccessorsFactory.cs index b1128d8b157..829307631fe 100644 --- a/src/EFCore/Metadata/Internal/PropertyAccessorsFactory.cs +++ b/src/EFCore/Metadata/Internal/PropertyAccessorsFactory.cs @@ -304,16 +304,16 @@ public static Expression CreateMemberAccess( } else if (!fromDeclaringType || !addNullCheck - || property is not IComplexProperty complexProperty + || property?.DeclaringType is not IRuntimeComplexType declaringComplexType || instanceExpression.Type.IsValueType - || complexProperty.ClrType.IsValueType) + || declaringComplexType.ClrType.IsValueType) { // Disable null check for all cases except: // - fromDeclaringType is true AND // - addNullCheck is true AND - // - property is a complex property AND + // - property is declared in a complex type AND // - instance is a reference type AND - // - complex property type is a reference type + // - declaring complex type is a reference type addNullCheck = false; } From ee993b3d2bde5c10717e50fd3954b77bfa3a617a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 22:36:46 +0000 Subject: [PATCH 4/6] Skip setting properties in null complex types during AcceptChanges The NRE was occurring in AcceptChanges when trying to set store-generated values on properties inside null complex types. Added check to skip properties declared in complex types when the complex type instance is null, following the same pattern used elsewhere in the codebase (lines 1612-1613, 1627-1628). Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- src/EFCore/ChangeTracking/Internal/InternalEntryBase.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/EFCore/ChangeTracking/Internal/InternalEntryBase.cs b/src/EFCore/ChangeTracking/Internal/InternalEntryBase.cs index ac2b693833e..38ec5d3db69 100644 --- a/src/EFCore/ChangeTracking/Internal/InternalEntryBase.cs +++ b/src/EFCore/ChangeTracking/Internal/InternalEntryBase.cs @@ -1477,7 +1477,9 @@ public virtual void AcceptChanges() var storeGeneratedIndex = property.GetStoreGeneratedIndex(); if (storeGeneratedIndex != -1 && _stateData.IsPropertyFlagged(property.GetIndex(), PropertyFlag.IsStoreGenerated) - && _storeGeneratedValues.TryGetValue(storeGeneratedIndex, out var value)) + && _storeGeneratedValues.TryGetValue(storeGeneratedIndex, out var value) + && (property.DeclaringType is not IComplexType complexType + || GetCurrentValue(complexType.ComplexProperty) != null)) { this[property] = value; } From 7b248203f53ffadfaf4ac3923c324f50ad8dcc37 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 23:25:09 +0000 Subject: [PATCH 5/6] Add filter to First() queries to avoid FirstWithoutOrderByAndFilterWarning Added .Where(i => i.Name == "Item1") filter before FirstAsync() calls to avoid the warning about using First without OrderBy and filter operators. This makes the tests more deterministic and follows EF Core testing best practices. Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- .../ComplexTypesTrackingSqlServerTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/EFCore.SqlServer.FunctionalTests/ComplexTypesTrackingSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/ComplexTypesTrackingSqlServerTest.cs index b7336637835..f71c7a6537f 100644 --- a/test/EFCore.SqlServer.FunctionalTests/ComplexTypesTrackingSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/ComplexTypesTrackingSqlServerTest.cs @@ -23,7 +23,7 @@ await context.Database.CreateExecutionStrategy().ExecuteAsync( await context.SaveChangesAsync(); context.ChangeTracker.Clear(); - var item = await context.Set().FirstAsync(); + var item = await context.Set().Where(i => i.Name == "Item1").FirstAsync(); var entry = context.ChangeTracker.Entries().First(); var originalItem = (InheritedItemBase)entry.OriginalValues.ToObject(); @@ -47,7 +47,7 @@ await context.Database.CreateExecutionStrategy().ExecuteAsync( await context.SaveChangesAsync(); context.ChangeTracker.Clear(); - var item = await context.Set().FirstAsync(); + var item = await context.Set().Where(i => i.Name == "Item1").FirstAsync(); var entry = context.ChangeTracker.Entries().First(); var originalItem = (InheritedItemBase)entry.OriginalValues.ToObject(); From 4e16643c0895ece9c20166e000d45a4e7aa4072d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 14 Feb 2026 00:54:54 +0000 Subject: [PATCH 6/6] Only add null checks for nullable complex properties Modified PropertyAccessorsFactory to only add null checks when the complex property is actually nullable (IsNullable=true). This prevents adding unnecessary null checks in compiled models for required complex properties while still protecting against NRE for nullable complex properties in TPH scenarios with shared columns. Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- src/EFCore/Metadata/Internal/PropertyAccessorsFactory.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/EFCore/Metadata/Internal/PropertyAccessorsFactory.cs b/src/EFCore/Metadata/Internal/PropertyAccessorsFactory.cs index 829307631fe..ec5d4b78b9f 100644 --- a/src/EFCore/Metadata/Internal/PropertyAccessorsFactory.cs +++ b/src/EFCore/Metadata/Internal/PropertyAccessorsFactory.cs @@ -304,16 +304,18 @@ public static Expression CreateMemberAccess( } else if (!fromDeclaringType || !addNullCheck - || property?.DeclaringType is not IRuntimeComplexType declaringComplexType + || property?.DeclaringType is not IComplexType declaringComplexType || instanceExpression.Type.IsValueType - || declaringComplexType.ClrType.IsValueType) + || declaringComplexType.ClrType.IsValueType + || !declaringComplexType.ComplexProperty.IsNullable) { // Disable null check for all cases except: // - fromDeclaringType is true AND // - addNullCheck is true AND // - property is declared in a complex type AND // - instance is a reference type AND - // - declaring complex type is a reference type + // - declaring complex type is a reference type AND + // - the complex property is nullable addNullCheck = false; }