FIX Insert order bug when using TPC - Issue #35978

[andrewraper-Sage]

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

[andrewraper-Sage]

@andrewraper-Sage please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@dotnet-policy-service agree [company="{your company}"]

Options:

• (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.

@dotnet-policy-service agree

• (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.

@dotnet-policy-service agree company="Microsoft"

Contributor License Agreement

@dotnet-policy-service agree company="Sage"

[Copilot]

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 else gating so model-level FK dependency detection runs alongside the table-level constraint path (with CanCreateDependency preventing 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.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[Copilot]

Removing the else means the model-level FK scanning loops now run even when command.Table != null. Although CanCreateDependency() filters out most FKs with mapped constraints, this still adds per-command work on common SaveChanges paths. If possible, consider an early skip/guard so the model-level path only runs when there are unmapped constraints for the given command (to avoid unnecessary iteration on large graphs).

[andrewraper-Sage]

This makes a valid point, however, the added complexity likely doesn't trade off with the optimization that would be gained.

[AndriySvyryd]

It's not only about perf, removing the else check also muddles this, already complicated logic. Currently there are two modes in the method:

1. command.Table != null is the common case
2. command.Table == null is used when there is not relational model, which should only be the case sometimes when applying seed data changes in migrations.

So, to keep the logic clear only change the first case. There, you can add another loop that goes over the FKs that are not mapped and would be between two different tables if mapped. Something like:

foreach (var foreignKey in principal
                ? entry.EntityType.GetReferencingForeignKeys()
                : (IEnumerable<IForeignKey>)entry.EntityType.GetForeignKeys())
{
    if (!foreignKey.GetMappedConstraints().Any()
        || (principal ? foreignKey.DeclaringEntityType : foreignKey.PrincipalEntityType).GetTableMappings().Any(m => m.Table == command.Table))
    {
        return true;
    }

    ...
}

The logic inside would indeed look a lot like the logic for the case without a relational model, but instead of trying to force the execution path to use it you can extract it to a local method that's used from both places.

Same applies for the changes below