From d2964772a3365e1fd8a801194dbcb6a9e228762b Mon Sep 17 00:00:00 2001 From: Andrew Raper Date: Wed, 15 Apr 2026 16:39:43 +0100 Subject: [PATCH 1/2] Fix TPC insert/delete order when principal is abstract (#35978) 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 #35978 --- .../Update/Internal/CommandBatchPreparer.cs | 75 ++++++++-------- .../Update/CommandBatchPreparerTest.cs | 86 +++++++++++++++++++ 2 files changed, 127 insertions(+), 34 deletions(-) diff --git a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs index 6590593a40e..6be6bcc41bc 100644 --- a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs +++ b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs @@ -681,7 +681,8 @@ private void AddForeignKeyEdges() if (!CanCreateDependency(foreignKey, command, principal: true) || !IsModified(foreignKey.PrincipalKey.Properties, entry) || (command.Table != null - && !IsStoreGenerated(entry, foreignKey.PrincipalKey))) + && !IsStoreGenerated(entry, foreignKey.PrincipalKey) + && foreignKey.GetMappedConstraints().Any())) { continue; } @@ -726,31 +727,30 @@ private void AddForeignKeyEdges() } } } - else + + foreach (var entry in command.Entries) { - foreach (var entry in command.Entries) + foreach (var foreignKey in entry.EntityType.GetForeignKeys()) { - foreach (var foreignKey in entry.EntityType.GetForeignKeys()) + if (!CanCreateDependency(foreignKey, command, principal: false) + || !IsModified(foreignKey.Properties, entry) + || foreignKey.GetMappedConstraints().Any(c => c.Table == command.Table)) { - if (!CanCreateDependency(foreignKey, command, principal: false) - || !IsModified(foreignKey.Properties, entry)) - { - continue; - } + continue; + } - var dependentKeyValue = foreignKey.GetDependentKeyValueFactory() - ?.CreateDependentEquatableKey(entry, fromOriginalValues: true); + var dependentKeyValue = foreignKey.GetDependentKeyValueFactory() + ?.CreateDependentEquatableKey(entry, fromOriginalValues: true); - if (dependentKeyValue != null) + if (dependentKeyValue != null) + { + if (!originalPredecessorsMap.TryGetValue(dependentKeyValue, out var predecessorCommands)) { - if (!originalPredecessorsMap.TryGetValue(dependentKeyValue, out var predecessorCommands)) - { - predecessorCommands = []; - originalPredecessorsMap.Add(dependentKeyValue, predecessorCommands); - } - - predecessorCommands.Add(command); + predecessorCommands = []; + originalPredecessorsMap.Add(dependentKeyValue, predecessorCommands); } + + predecessorCommands.Add(command); } } } @@ -825,25 +825,24 @@ private void AddForeignKeyEdges() originalPredecessorsMap, principalKeyValue, command, foreignKey); } } - else + + // ReSharper disable once ForCanBeConvertedToForeach + for (var entryIndex = 0; entryIndex < command.Entries.Count; entryIndex++) { - // ReSharper disable once ForCanBeConvertedToForeach - for (var entryIndex = 0; entryIndex < command.Entries.Count; entryIndex++) + var entry = command.Entries[entryIndex]; + foreach (var foreignKey in entry.EntityType.GetReferencingForeignKeys()) { - var entry = command.Entries[entryIndex]; - foreach (var foreignKey in entry.EntityType.GetReferencingForeignKeys()) + if (!CanCreateDependency(foreignKey, command, principal: true) + || foreignKey.GetMappedConstraints().Any(c => c.PrincipalTable == command.Table)) { - if (!CanCreateDependency(foreignKey, command, principal: true)) - { - continue; - } - - var principalKeyValue = foreignKey.GetDependentKeyValueFactory() - .CreatePrincipalEquatableKey(entry, fromOriginalValues: true); - Check.DebugAssert(principalKeyValue != null, "null principalKeyValue"); - AddMatchingPredecessorEdge( - originalPredecessorsMap, principalKeyValue, command, foreignKey); + continue; } + + var principalKeyValue = foreignKey.GetDependentKeyValueFactory() + .CreatePrincipalEquatableKey(entry, fromOriginalValues: true); + Check.DebugAssert(principalKeyValue != null, "null principalKeyValue"); + AddMatchingPredecessorEdge( + originalPredecessorsMap, principalKeyValue, command, foreignKey); } } } @@ -873,6 +872,14 @@ private static bool CanCreateDependency(IForeignKey foreignKey, IReadOnlyModific { if (command.Table != null) { + // JSON-owned entities are stored inline in their owner's column and never have separate + // modification commands, so they cannot participate in inter-command dependency ordering. + var otherEntityType = principal ? foreignKey.DeclaringEntityType : foreignKey.PrincipalEntityType; + if (otherEntityType.IsMappedToJson()) + { + return false; + } + if (foreignKey.IsRowInternal(StoreObjectIdentifier.Table(command.TableName, command.Schema)) || (foreignKey.PrincipalEntityType.IsAssignableFrom(foreignKey.DeclaringEntityType) && foreignKey.PrincipalKey.Properties.SequenceEqual(foreignKey.Properties))) diff --git a/test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs b/test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs index a312f860c53..8f0be7b7a87 100644 --- a/test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs +++ b/test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs @@ -1328,4 +1328,90 @@ private class AnotherFakeEntity public int Id { get; set; } public int? AnotherId { get; set; } } + + [ConditionalFact] + public void BatchCommands_sorts_added_entities_with_TPC_abstract_principal() + { + var configuration = CreateContextServices(CreateTpcFKModel()); + var stateManager = configuration.GetRequiredService(); + + var principalEntry = stateManager.GetOrCreateEntry( + new ConcretePrincipal { Id = 1 }); + principalEntry.SetEntityState(EntityState.Added); + + var dependentEntry = stateManager.GetOrCreateEntry( + new TpcDependent { Id = 1, PrincipalId = 1 }); + dependentEntry.SetEntityState(EntityState.Added); + + var modelData = new UpdateAdapter(stateManager); + + var batches = CreateBatches([dependentEntry, principalEntry], modelData); + var batch = Assert.Single(batches); + + Assert.Equal( + [principalEntry, dependentEntry], + batch.ModificationCommands.Select(c => c.Entries.Single())); + } + + [ConditionalFact] + public void BatchCommands_sorts_deleted_entities_with_TPC_abstract_principal() + { + var configuration = CreateContextServices(CreateTpcFKModel()); + var stateManager = configuration.GetRequiredService(); + + var principalEntry = stateManager.GetOrCreateEntry( + new ConcretePrincipal { Id = 1 }); + principalEntry.SetEntityState(EntityState.Deleted); + + var dependentEntry = stateManager.GetOrCreateEntry( + new TpcDependent { Id = 1, PrincipalId = 1 }); + dependentEntry.SetEntityState(EntityState.Deleted); + + var modelData = new UpdateAdapter(stateManager); + + var batches = CreateBatches([principalEntry, dependentEntry], modelData); + var batch = Assert.Single(batches); + + Assert.Equal( + [dependentEntry, principalEntry], + batch.ModificationCommands.Select(c => c.Entries.Single())); + } + + private static IModel CreateTpcFKModel() + { + var modelBuilder = FakeRelationalTestHelpers.Instance.CreateConventionBuilder(); + + modelBuilder.Entity() + .UseTpcMappingStrategy() + .ToTable((string)null) + .Property(e => e.Id) + .ValueGeneratedNever(); + + modelBuilder.Entity() + .ToTable(nameof(ConcretePrincipal)); + + modelBuilder.Entity(b => + { + b.HasOne() + .WithMany() + .HasForeignKey(c => c.PrincipalId); + }); + + return modelBuilder.Model.FinalizeModel(); + } + + private abstract class AbstractPrincipal + { + public int Id { get; set; } + } + + private class ConcretePrincipal : AbstractPrincipal + { + } + + private class TpcDependent + { + public int Id { get; set; } + public int PrincipalId { get; set; } + } } From cc259bb94a5ab8a8652b0def541a2330d446165b Mon Sep 17 00:00:00 2001 From: Andrew Raper Date: Thu, 16 Apr 2026 16:36:52 +0100 Subject: [PATCH 2/2] reinstate else's --- .../Update/Internal/CommandBatchPreparer.cs | 111 +++++++++++++----- 1 file changed, 81 insertions(+), 30 deletions(-) diff --git a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs index 6be6bcc41bc..6b2e45614fc 100644 --- a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs +++ b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs @@ -726,31 +726,60 @@ private void AddForeignKeyEdges() predecessorCommands.Add(command); } } - } - foreach (var entry in command.Entries) - { - foreach (var foreignKey in entry.EntityType.GetForeignKeys()) + // Also handle FKs with no mapped constraints (e.g. TPC with abstract principal mapped to no table) + foreach (var entry in command.Entries) { - if (!CanCreateDependency(foreignKey, command, principal: false) - || !IsModified(foreignKey.Properties, entry) - || foreignKey.GetMappedConstraints().Any(c => c.Table == command.Table)) + foreach (var foreignKey in entry.EntityType.GetForeignKeys()) { - continue; - } + if (!CanCreateDependency(foreignKey, command, principal: false) + || !IsModified(foreignKey.Properties, entry) + || foreignKey.GetMappedConstraints().Any()) + { + continue; + } - var dependentKeyValue = foreignKey.GetDependentKeyValueFactory() - ?.CreateDependentEquatableKey(entry, fromOriginalValues: true); + var dependentKeyValue = foreignKey.GetDependentKeyValueFactory() + ?.CreateDependentEquatableKey(entry, fromOriginalValues: true); - if (dependentKeyValue != null) + if (dependentKeyValue != null) + { + if (!originalPredecessorsMap.TryGetValue(dependentKeyValue, out var predecessorCommands)) + { + predecessorCommands = []; + originalPredecessorsMap.Add(dependentKeyValue, predecessorCommands); + } + + predecessorCommands.Add(command); + } + } + } + } + else + { + foreach (var entry in command.Entries) + { + foreach (var foreignKey in entry.EntityType.GetForeignKeys()) { - if (!originalPredecessorsMap.TryGetValue(dependentKeyValue, out var predecessorCommands)) + if (!CanCreateDependency(foreignKey, command, principal: false) + || !IsModified(foreignKey.Properties, entry)) { - predecessorCommands = []; - originalPredecessorsMap.Add(dependentKeyValue, predecessorCommands); + continue; } - predecessorCommands.Add(command); + var dependentKeyValue = foreignKey.GetDependentKeyValueFactory() + ?.CreateDependentEquatableKey(entry, fromOriginalValues: true); + + if (dependentKeyValue != null) + { + if (!originalPredecessorsMap.TryGetValue(dependentKeyValue, out var predecessorCommands)) + { + predecessorCommands = []; + originalPredecessorsMap.Add(dependentKeyValue, predecessorCommands); + } + + predecessorCommands.Add(command); + } } } } @@ -824,25 +853,47 @@ private void AddForeignKeyEdges() AddMatchingPredecessorEdge( originalPredecessorsMap, principalKeyValue, command, foreignKey); } - } - // ReSharper disable once ForCanBeConvertedToForeach - for (var entryIndex = 0; entryIndex < command.Entries.Count; entryIndex++) - { - var entry = command.Entries[entryIndex]; - foreach (var foreignKey in entry.EntityType.GetReferencingForeignKeys()) + // Also handle FKs with no mapped constraints (e.g. TPC with abstract principal mapped to no table) + // ReSharper disable once ForCanBeConvertedToForeach + for (var entryIndex = 0; entryIndex < command.Entries.Count; entryIndex++) { - if (!CanCreateDependency(foreignKey, command, principal: true) - || foreignKey.GetMappedConstraints().Any(c => c.PrincipalTable == command.Table)) + var entry = command.Entries[entryIndex]; + foreach (var foreignKey in entry.EntityType.GetReferencingForeignKeys()) { - continue; + if (!CanCreateDependency(foreignKey, command, principal: true) + || foreignKey.GetMappedConstraints().Any()) + { + continue; + } + + var principalKeyValue = foreignKey.GetDependentKeyValueFactory() + .CreatePrincipalEquatableKey(entry, fromOriginalValues: true); + Check.DebugAssert(principalKeyValue != null, "null principalKeyValue"); + AddMatchingPredecessorEdge( + originalPredecessorsMap, principalKeyValue, command, foreignKey); } + } + } + else + { + // ReSharper disable once ForCanBeConvertedToForeach + for (var entryIndex = 0; entryIndex < command.Entries.Count; entryIndex++) + { + var entry = command.Entries[entryIndex]; + foreach (var foreignKey in entry.EntityType.GetReferencingForeignKeys()) + { + if (!CanCreateDependency(foreignKey, command, principal: true)) + { + continue; + } - var principalKeyValue = foreignKey.GetDependentKeyValueFactory() - .CreatePrincipalEquatableKey(entry, fromOriginalValues: true); - Check.DebugAssert(principalKeyValue != null, "null principalKeyValue"); - AddMatchingPredecessorEdge( - originalPredecessorsMap, principalKeyValue, command, foreignKey); + var principalKeyValue = foreignKey.GetDependentKeyValueFactory() + .CreatePrincipalEquatableKey(entry, fromOriginalValues: true); + Check.DebugAssert(principalKeyValue != null, "null principalKeyValue"); + AddMatchingPredecessorEdge( + originalPredecessorsMap, principalKeyValue, command, foreignKey); + } } } }