FIX Insert order bug when using TPC - Issue #35978 #38070
FIX Insert order bug when using TPC - Issue #35978 #38070AndriySvyryd merged 2 commits intodotnet:mainfrom
Conversation
@dotnet-policy-service agree company="Sage" |
There was a problem hiding this comment.
Pull request overview
Fixes incorrect SaveChanges command ordering for TPC inheritance when a foreign key’s principal is an abstract TPC type mapped to no table (preventing FK constraint violations by ensuring principals are ordered before dependents on INSERT, and dependents before principals on DELETE).
Changes:
- Update
CommandBatchPreparer.AddForeignKeyEdges()to include the model-level FK dependency path when no mapped table-level FK constraints exist. - Remove
elsegating so model-level FK dependency detection runs alongside the table-level constraint path (withCanCreateDependencypreventing duplicates). - Add regression tests covering INSERT and DELETE ordering for the abstract-TPC-principal scenario.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs | Ensures dependency edges are created via model-level FK logic when table-level constraints are missing, fixing ordering for abstract TPC principals. |
| test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs | Adds regression tests validating correct batching/sort order for Added and Deleted entities under abstract-principal TPC FKs. |
andrewraper-Sage
force-pushed
the
tpc-insert-fix
branch
from
f374446 to
f671169
Compare
When using TPC with an abstract principal type mapped to no table, FK dependency edges were not created because the code relied on mapped FK constraints, which don't exist for abstract TPC types. Fix: - In the ADD predecessor registration loop, add a guard so that FKs with no mapped constraints (TPC abstract principal) are not skipped by the IsStoreGenerated check. - In the DELETE predecessor registration and edge matching loops, remove the else gates so the entry-level FK loops run for all commands, not just those without a table. Add GetMappedConstraints guards to avoid double-processing FKs that are already handled by the table-level path. - In CanCreateDependency, skip FKs where the other entity type is mapped to JSON. JSON-owned entities are stored inline and never have separate modification commands, so they cannot participate in inter-command dependency ordering. This prevents false cycles with entity splitting and JSON ownership. Fixes dotnet#35978
andrewraper-Sage
force-pushed
the
tpc-insert-fix
branch
from
a2b4c9d to
d296477
Compare
|
@AndriySvyryd I think I should have now address your comment and the concerns of Copilot. I am seeing a weird failure in the tests now. It appears that |
You still haven't restored the else blocks
That's unrelated, you can ignore this |
I've restored these, and the tests have all passed now as well |
AndriySvyryd
left a comment
AndriySvyryd
left a comment
There was a problem hiding this comment.
Thanks for your contribution!
You’re welcome! Is there a chance this could be cherry picked and included in the next v10 patch release? @AndriySvyryd |
That's unlikely, we haven't had that many reports considering that it was introduced in 9.0.0 and the fix is not trivial |
When using TPC inheritance with abstract base types, SaveChanges produces an incorrect INSERT order - child entities are inserted before their parents, causing FK constraint violations.
This happens because CommandBatchPreparer.AddForeignKeyEdges() fails to create dependency edges for FKs whose principal is an abstract TPC type mapped to no table. Since no IForeignKeyConstraint is created for such FKs, the table-level constraint path finds nothing.
The model-level FK fallback path should handle this, but for INSERT ordering it was skipped by a guard that assumed table-level constraints would cover it, and for DELETE ordering it was gated behind command.Table == null (unreachable for concrete TPC entities which always have a table).
The fix ensures the model-level FK path participates in dependency edge creation when no mapped table-level constraint exists: an additional foreignKey.GetMappedConstraints().Any() check on the INSERT principal registration guard, and removing the else gates on both DELETE paths so they always run alongside the table-level path (CanCreateDependency already prevents double-counting for FKs that do have mapped constraints).
Fixes #35978