Skip to content

Commit d296477

Browse files
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
1 parent a630adf commit d296477

2 files changed

Lines changed: 127 additions & 34 deletions

File tree

src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs

Lines changed: 41 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -681,7 +681,8 @@ private void AddForeignKeyEdges()
681681
if (!CanCreateDependency(foreignKey, command, principal: true)
682682
|| !IsModified(foreignKey.PrincipalKey.Properties, entry)
683683
|| (command.Table != null
684-
&& !IsStoreGenerated(entry, foreignKey.PrincipalKey)))
684+
&& !IsStoreGenerated(entry, foreignKey.PrincipalKey)
685+
&& foreignKey.GetMappedConstraints().Any()))
685686
{
686687
continue;
687688
}
@@ -726,31 +727,30 @@ private void AddForeignKeyEdges()
726727
}
727728
}
728729
}
729-
else
730+
731+
foreach (var entry in command.Entries)
730732
{
731-
foreach (var entry in command.Entries)
733+
foreach (var foreignKey in entry.EntityType.GetForeignKeys())
732734
{
733-
foreach (var foreignKey in entry.EntityType.GetForeignKeys())
735+
if (!CanCreateDependency(foreignKey, command, principal: false)
736+
|| !IsModified(foreignKey.Properties, entry)
737+
|| foreignKey.GetMappedConstraints().Any(c => c.Table == command.Table))
734738
{
735-
if (!CanCreateDependency(foreignKey, command, principal: false)
736-
|| !IsModified(foreignKey.Properties, entry))
737-
{
738-
continue;
739-
}
739+
continue;
740+
}
740741

741-
var dependentKeyValue = foreignKey.GetDependentKeyValueFactory()
742-
?.CreateDependentEquatableKey(entry, fromOriginalValues: true);
742+
var dependentKeyValue = foreignKey.GetDependentKeyValueFactory()
743+
?.CreateDependentEquatableKey(entry, fromOriginalValues: true);
743744

744-
if (dependentKeyValue != null)
745+
if (dependentKeyValue != null)
746+
{
747+
if (!originalPredecessorsMap.TryGetValue(dependentKeyValue, out var predecessorCommands))
745748
{
746-
if (!originalPredecessorsMap.TryGetValue(dependentKeyValue, out var predecessorCommands))
747-
{
748-
predecessorCommands = [];
749-
originalPredecessorsMap.Add(dependentKeyValue, predecessorCommands);
750-
}
751-
752-
predecessorCommands.Add(command);
749+
predecessorCommands = [];
750+
originalPredecessorsMap.Add(dependentKeyValue, predecessorCommands);
753751
}
752+
753+
predecessorCommands.Add(command);
754754
}
755755
}
756756
}
@@ -825,25 +825,24 @@ private void AddForeignKeyEdges()
825825
originalPredecessorsMap, principalKeyValue, command, foreignKey);
826826
}
827827
}
828-
else
828+
829+
// ReSharper disable once ForCanBeConvertedToForeach
830+
for (var entryIndex = 0; entryIndex < command.Entries.Count; entryIndex++)
829831
{
830-
// ReSharper disable once ForCanBeConvertedToForeach
831-
for (var entryIndex = 0; entryIndex < command.Entries.Count; entryIndex++)
832+
var entry = command.Entries[entryIndex];
833+
foreach (var foreignKey in entry.EntityType.GetReferencingForeignKeys())
832834
{
833-
var entry = command.Entries[entryIndex];
834-
foreach (var foreignKey in entry.EntityType.GetReferencingForeignKeys())
835+
if (!CanCreateDependency(foreignKey, command, principal: true)
836+
|| foreignKey.GetMappedConstraints().Any(c => c.PrincipalTable == command.Table))
835837
{
836-
if (!CanCreateDependency(foreignKey, command, principal: true))
837-
{
838-
continue;
839-
}
840-
841-
var principalKeyValue = foreignKey.GetDependentKeyValueFactory()
842-
.CreatePrincipalEquatableKey(entry, fromOriginalValues: true);
843-
Check.DebugAssert(principalKeyValue != null, "null principalKeyValue");
844-
AddMatchingPredecessorEdge(
845-
originalPredecessorsMap, principalKeyValue, command, foreignKey);
838+
continue;
846839
}
840+
841+
var principalKeyValue = foreignKey.GetDependentKeyValueFactory()
842+
.CreatePrincipalEquatableKey(entry, fromOriginalValues: true);
843+
Check.DebugAssert(principalKeyValue != null, "null principalKeyValue");
844+
AddMatchingPredecessorEdge(
845+
originalPredecessorsMap, principalKeyValue, command, foreignKey);
847846
}
848847
}
849848
}
@@ -873,6 +872,14 @@ private static bool CanCreateDependency(IForeignKey foreignKey, IReadOnlyModific
873872
{
874873
if (command.Table != null)
875874
{
875+
// JSON-owned entities are stored inline in their owner's column and never have separate
876+
// modification commands, so they cannot participate in inter-command dependency ordering.
877+
var otherEntityType = principal ? foreignKey.DeclaringEntityType : foreignKey.PrincipalEntityType;
878+
if (otherEntityType.IsMappedToJson())
879+
{
880+
return false;
881+
}
882+
876883
if (foreignKey.IsRowInternal(StoreObjectIdentifier.Table(command.TableName, command.Schema))
877884
|| (foreignKey.PrincipalEntityType.IsAssignableFrom(foreignKey.DeclaringEntityType)
878885
&& foreignKey.PrincipalKey.Properties.SequenceEqual(foreignKey.Properties)))

test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1328,4 +1328,90 @@ private class AnotherFakeEntity
13281328
public int Id { get; set; }
13291329
public int? AnotherId { get; set; }
13301330
}
1331+
1332+
[ConditionalFact]
1333+
public void BatchCommands_sorts_added_entities_with_TPC_abstract_principal()
1334+
{
1335+
var configuration = CreateContextServices(CreateTpcFKModel());
1336+
var stateManager = configuration.GetRequiredService<IStateManager>();
1337+
1338+
var principalEntry = stateManager.GetOrCreateEntry(
1339+
new ConcretePrincipal { Id = 1 });
1340+
principalEntry.SetEntityState(EntityState.Added);
1341+
1342+
var dependentEntry = stateManager.GetOrCreateEntry(
1343+
new TpcDependent { Id = 1, PrincipalId = 1 });
1344+
dependentEntry.SetEntityState(EntityState.Added);
1345+
1346+
var modelData = new UpdateAdapter(stateManager);
1347+
1348+
var batches = CreateBatches([dependentEntry, principalEntry], modelData);
1349+
var batch = Assert.Single(batches);
1350+
1351+
Assert.Equal(
1352+
[principalEntry, dependentEntry],
1353+
batch.ModificationCommands.Select(c => c.Entries.Single()));
1354+
}
1355+
1356+
[ConditionalFact]
1357+
public void BatchCommands_sorts_deleted_entities_with_TPC_abstract_principal()
1358+
{
1359+
var configuration = CreateContextServices(CreateTpcFKModel());
1360+
var stateManager = configuration.GetRequiredService<IStateManager>();
1361+
1362+
var principalEntry = stateManager.GetOrCreateEntry(
1363+
new ConcretePrincipal { Id = 1 });
1364+
principalEntry.SetEntityState(EntityState.Deleted);
1365+
1366+
var dependentEntry = stateManager.GetOrCreateEntry(
1367+
new TpcDependent { Id = 1, PrincipalId = 1 });
1368+
dependentEntry.SetEntityState(EntityState.Deleted);
1369+
1370+
var modelData = new UpdateAdapter(stateManager);
1371+
1372+
var batches = CreateBatches([principalEntry, dependentEntry], modelData);
1373+
var batch = Assert.Single(batches);
1374+
1375+
Assert.Equal(
1376+
[dependentEntry, principalEntry],
1377+
batch.ModificationCommands.Select(c => c.Entries.Single()));
1378+
}
1379+
1380+
private static IModel CreateTpcFKModel()
1381+
{
1382+
var modelBuilder = FakeRelationalTestHelpers.Instance.CreateConventionBuilder();
1383+
1384+
modelBuilder.Entity<AbstractPrincipal>()
1385+
.UseTpcMappingStrategy()
1386+
.ToTable((string)null)
1387+
.Property(e => e.Id)
1388+
.ValueGeneratedNever();
1389+
1390+
modelBuilder.Entity<ConcretePrincipal>()
1391+
.ToTable(nameof(ConcretePrincipal));
1392+
1393+
modelBuilder.Entity<TpcDependent>(b =>
1394+
{
1395+
b.HasOne<AbstractPrincipal>()
1396+
.WithMany()
1397+
.HasForeignKey(c => c.PrincipalId);
1398+
});
1399+
1400+
return modelBuilder.Model.FinalizeModel();
1401+
}
1402+
1403+
private abstract class AbstractPrincipal
1404+
{
1405+
public int Id { get; set; }
1406+
}
1407+
1408+
private class ConcretePrincipal : AbstractPrincipal
1409+
{
1410+
}
1411+
1412+
private class TpcDependent
1413+
{
1414+
public int Id { get; set; }
1415+
public int PrincipalId { get; set; }
1416+
}
13311417
}

0 commit comments

Comments
 (0)