[release/10.0] Fix owned entities with default values not saved in TPH with shared columns#37751
[release/10.0] Fix owned entities with default values not saved in TPH with shared columns#37751Copilot wants to merge 5 commits intorelease/10.0from
Conversation
AndriySvyryd
left a comment
There was a problem hiding this comment.
@copilot Update progress by rewriting the PR summary using this template:
Fixes #{number}
**Description**
{Issue description}
**Customer impact**
{How does the reported issue affects customer's app? Are there workarounds?}
**How found**
{Was it customer reported or found during verification? How many customers are affected?}
**Regression**
{Is it a regression from a released version? Which one?}
**Testing**
{How the changes were tested}
**Risk**
{Low/Medium/High, argument why}
test/EFCore.Relational.Specification.Tests/Update/UpdatesRelationalTestBase.cs
Show resolved
Hide resolved
…ern (#37525) Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Updated the PR description using the requested template in commit fb4099b. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression in EF Core 8/9/10 where owned entities with default values were not saved when using Table-Per-Hierarchy (TPH) inheritance with shared column names. The issue occurred because the ColumnValuePropagator.TryPropagate method incorrectly suppressed writes for sentinel values even when the Added entry was the sole writer for a shared column.
Changes:
- Added a guard condition to
TryPropagatethat only suppresses sentinel value writes when another entry has recorded a different value for the shared column - Added comprehensive test coverage with a TPH model using owned entities, shared column names, and value converters
- Added test model classes (
Nougat,CrunchyNougat,SoftNougat,NougatFilling,NougatFillingKind) to the Updates test infrastructure
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/EFCore.Relational/Update/ModificationCommand.cs |
Fixed ColumnValuePropagator.TryPropagate to add an additional guard that compares _currentValue with the entry's provider value before suppressing sentinel writes |
test/EFCore.Specification.Tests/Update/UpdatesTestBase.cs |
Added model configuration for Nougat entities with owned Filling and string value converter on enum property |
test/EFCore.Specification.Tests/TestModels/UpdatesModel/UpdatesContext.cs |
Added DbSet properties for CrunchyNougat and SoftNougat |
test/EFCore.Specification.Tests/TestModels/UpdatesModel/Nougat.cs |
Added TPH test model with owned entities containing default values (enum = 0, bool = false) |
test/EFCore.Relational.Specification.Tests/Update/UpdatesRelationalTestBase.cs |
Added test method and model configuration with shared column names (FillingKind, FillingIsFresh) for the TPH scenario |
Fixes #37525
Description
When using Table-Per-Hierarchy (TPH) inheritance with owned entities (
OwnsOne) where different entity types share column names via.HasColumnName(), setting an owned entity fromnullto a new instance with all CLR default values (e.g.,EnumValue.Unknown = 0,IsBool = false) silently dropped the UPDATE columns — the owned entity remainednullin the database.ColumnValuePropagator.TryPropagatetreats sentinel values on Added entries as "nothing to write." This is correct when another entry recorded a different value for the shared column, but wrong when the Added entry is the sole writer — the sentinel is the intended value.Customer impact
Customers using TPH with owned entities sharing column names cannot save owned entities with default values. The data silently remains
nullin the database, leading to data corruption. No workaround other than avoiding default values or not sharing column names.How found
Customer reported.
Regression
Yes, from EF Core 7.
Testing
Test added.
Risk
Low. The fix only affects saving changes in TPH with shared columns. Quirk mode added
Original prompt
This section details on the original issue you should resolve
<issue_title>OwnOne entities with default values not saved in TPH scenario</issue_title>
<issue_description>### Bug description
When using Table-Per-Hierarchy (TPH) inheritance with owned entities (OwnsOne), EF Core 8 generates an UPDATE statement but omits the owned entity columns when:
This leads to data corruption, preventing us from effectively upgrading from EF core 7 to EF core 8/9/10.
Please find a sample repro project attached.
EfCoreEnumNullTest.zip
EF Core version
8/9/10
Database provider
SqlServer
Target framework
.NET 10
IDE
rider</issue_description>
<agent_instructions>Start by creating a test that fails in the described way based on the comment #37525 (comment)</agent_instructions>
Comments on the Issue (you are @copilot in this section)
@roji # AI answerReproduction Confirmed
I've successfully reproduced this issue. This is a regression affecting EF Core 8 and 9, which has been fixed in EF Core 10.
Symptoms
When using TPH (Table-Per-Hierarchy) inheritance with owned entities (
OwnsOne) where<Nullable>disable</Nullable>is set in the project, and setting an owned entity with CLR default values (e.g.,EnumValue.Unknown = 0,IsBool = false), EF Core 8/9 fails to generate an UPDATE statement. This results in the database retaining NULL values instead of the expected default values.Version Testing Results
EF Core 7: ✅ Works correctly - generates UPDATE statement with owned entity columns
EF Core 8: ❌ Broken - no UPDATE statement generated, values remain NULL
EF Core 9: ❌ Broken - no UPDATE statement generated, values remain NULL
EF Core 10: ✅ Fixed - generates UPDATE statement with owned entity columns
Minimal repro
</...
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.