Skip to content

Fix NullReferenceException when accessing null complex properties in TPH with shared columns#37695

Merged
AndriySvyryd merged 8 commits intomainfrom
copilot/port-pr-37389-to-main
Feb 27, 2026
Merged

Fix NullReferenceException when accessing null complex properties in TPH with shared columns#37695
AndriySvyryd merged 8 commits intomainfrom
copilot/port-pr-37389-to-main

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 13, 2026

When two derived types in TPH share a nullable complex property mapped to the same column, saving an entity with a null complex property throws a NullReferenceException during value propagation.

Changes

The fix adds an early return in ColumnValuePropagator.TryPropagate (ModificationCommand.cs) to skip propagation when the property belongs to a complex type whose containing complex property is currently null. This prevents the NRE at its source — the value propagation layer.

Test coverage

Added tests in ComplexTypesTrackingSqlServerTest for TPH scenarios with shared complex property columns:

  • Can_read_original_values_with_TPH_shared_complex_property_column_null — validates save and original values access when complex property is null
  • Can_read_original_values_with_TPH_shared_complex_property_column_with_value — validates save and original values access when complex property has a value

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…es in TPH

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copilot AI changed the title [WIP] Port changes from PR #37389 to main branch Fix NullReferenceException when accessing null complex properties in TPH with shared columns Feb 13, 2026
Copilot AI requested a review from AndriySvyryd February 13, 2026 05:01
…tself 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>
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>
…rning

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>
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>
@AndriySvyryd
Copy link
Copy Markdown
Member

@copilot Update the description with a summary of the changes and reference the issue being fixed: #37335
Also, instead of the current fix, change TryPropagate, it shouldn't do anything if the containing property for property is currently null

Instead of modifying PropertyAccessorsFactory and AcceptChanges, fix the issue at its source in TryPropagate. When a property belongs to a complex type whose containing complex property is currently null, TryPropagate now returns early without attempting to propagate the value, preventing the NRE.

Fixes #37335

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Feb 26, 2026

@copilot Update the description with a summary of the changes and reference the issue being fixed: #37335
Also, instead of the current fix, chan...

Done in 22c592e. Reverted changes to PropertyAccessorsFactory.cs and InternalEntryBase.AcceptChanges, and instead fixed TryPropagate in ModificationCommand.cs to skip propagation when the containing complex property is null. Updated the PR description to reference #37335.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a NullReferenceException that occurs when saving entities in a Table-Per-Hierarchy (TPH) inheritance scenario where two derived types share a nullable complex property mapped to the same database column. The exception was thrown during value propagation when the complex property was null.

Changes:

  • Added an early return in ColumnValuePropagator.TryPropagate to skip value propagation when a property belongs to a null complex type instance
  • Added comprehensive test coverage for both null and non-null complex property scenarios in TPH

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/EFCore.Relational/Update/ModificationCommand.cs Added null check for complex property instances before attempting to access their child properties during value propagation
test/EFCore.SqlServer.FunctionalTests/ComplexTypesTrackingSqlServerTest.cs Added test methods, model classes, and fixture configuration to verify the fix handles TPH with shared complex property columns

@AndriySvyryd AndriySvyryd marked this pull request as ready for review February 26, 2026 01:34
@AndriySvyryd AndriySvyryd requested a review from a team as a code owner February 26, 2026 01:34
@roji roji assigned AndriySvyryd and unassigned roji Feb 26, 2026
@AndriySvyryd AndriySvyryd removed their assignment Feb 27, 2026
@AndriySvyryd AndriySvyryd enabled auto-merge (squash) February 27, 2026 03:13
@AndriySvyryd AndriySvyryd merged commit 0518708 into main Feb 27, 2026
11 checks passed
@AndriySvyryd AndriySvyryd deleted the copilot/port-pr-37389-to-main branch February 27, 2026 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Map two classes with same nullable complex properties to same column will result in a NullReferenceException

4 participants