From eb6fe8ed6d3dad30a15510a2163faeb2ce06e4c3 Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Fri, 17 Apr 2026 14:10:04 -0700 Subject: [PATCH 1/2] Refactor CommandBatchPreparer --- .../Update/Internal/CommandBatchPreparer.cs | 360 +++++++----------- 1 file changed, 139 insertions(+), 221 deletions(-) diff --git a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs index 6b2e45614fc..eca48c0efe0 100644 --- a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs +++ b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs @@ -650,254 +650,172 @@ private void AddForeignKeyEdges() { if (command.EntityState is EntityState.Modified or EntityState.Added) { - if (command.Table != null) - { - foreach (var foreignKey in command.Table.ReferencingForeignKeyConstraints) - { - if (!IsModified(foreignKey.PrincipalUniqueConstraint.Columns, command)) - { - continue; - } - - var principalKeyValue = ((ForeignKeyConstraint)foreignKey).GetRowForeignKeyValueFactory() - .CreatePrincipalEquatableKeyValue(command); - Check.DebugAssert(principalKeyValue != null, "null principalKeyValue"); - - if (!predecessorsMap.TryGetValue(principalKeyValue, out var predecessorCommands)) - { - predecessorCommands = []; - predecessorsMap.Add(principalKeyValue, predecessorCommands); - } - - predecessorCommands.Add(command); - } - } - - for (var i = 0; i < command.Entries.Count; i++) - { - var entry = command.Entries[i]; - foreach (var foreignKey in entry.EntityType.GetReferencingForeignKeys()) - { - if (!CanCreateDependency(foreignKey, command, principal: true) - || !IsModified(foreignKey.PrincipalKey.Properties, entry) - || (command.Table != null - && !IsStoreGenerated(entry, foreignKey.PrincipalKey) - && foreignKey.GetMappedConstraints().Any())) - { - continue; - } - - var principalKeyValue = foreignKey.GetDependentKeyValueFactory() - .CreatePrincipalEquatableKey(entry); - Check.DebugAssert(principalKeyValue != null, "null principalKeyValue"); - - if (!predecessorsMap.TryGetValue(principalKeyValue, out var predecessorCommands)) - { - predecessorCommands = []; - predecessorsMap.Add(principalKeyValue, predecessorCommands); - } - - predecessorCommands.Add(command); - } - } + AddForeignKeyPredecessors(command, predecessorsMap, principal: true); } if (command.EntityState is EntityState.Modified or EntityState.Deleted) { - if (command.Table != null) - { - foreach (var foreignKey in command.Table!.ForeignKeyConstraints) - { - if (!IsModified(foreignKey.Columns, command)) - { - continue; - } - - var dependentKeyValue = ((ForeignKeyConstraint)foreignKey).GetRowForeignKeyValueFactory() - .CreateDependentEquatableKeyValue(command, fromOriginalValues: true); - if (dependentKeyValue != null) - { - if (!originalPredecessorsMap.TryGetValue(dependentKeyValue, out var predecessorCommands)) - { - predecessorCommands = []; - originalPredecessorsMap.Add(dependentKeyValue, predecessorCommands); - } - - predecessorCommands.Add(command); - } - } + AddForeignKeyPredecessors(command, originalPredecessorsMap, principal: false); + } + } - // Also handle FKs with no mapped constraints (e.g. TPC with abstract principal mapped to no table) - foreach (var entry in command.Entries) - { - foreach (var foreignKey in entry.EntityType.GetForeignKeys()) - { - if (!CanCreateDependency(foreignKey, command, principal: false) - || !IsModified(foreignKey.Properties, entry) - || foreignKey.GetMappedConstraints().Any()) - { - continue; - } + foreach (var command in _modificationCommandGraph.Vertices) + { + if (command.EntityState is EntityState.Modified or EntityState.Added) + { + AddForeignKeyEdges(command, predecessorsMap, principal: false); + } - var dependentKeyValue = foreignKey.GetDependentKeyValueFactory() - ?.CreateDependentEquatableKey(entry, fromOriginalValues: true); + if (command.EntityState is EntityState.Modified or EntityState.Deleted) + { + AddForeignKeyEdges(command, originalPredecessorsMap, principal: true); + } + } + } - if (dependentKeyValue != null) - { - if (!originalPredecessorsMap.TryGetValue(dependentKeyValue, out var predecessorCommands)) - { - predecessorCommands = []; - originalPredecessorsMap.Add(dependentKeyValue, predecessorCommands); - } + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + private static void AddForeignKeyPredecessors( + IReadOnlyModificationCommand command, + Dictionary> predecessorsMap, + bool principal) + { + // This call doesn't need the IsStoreGenerated check because the constraint-level factory + // (IRowForeignKeyValueFactory) works from column modifications and always produces a usable key value, + // even for store-generated keys. The model-level factory below works from IUpdateEntry, which may hold + // temporary sentinel values for store-generated keys, so it needs to register those separately to ensure + // the dependent side can find the principal via either factory. + foreach (var (_, keyValue) in GetForeignKeyConstraintValues(command, principal, fromOriginalValues: !principal)) + { + AddPredecessor(predecessorsMap, keyValue, command); + } - predecessorCommands.Add(command); - } - } - } - } - else - { - foreach (var entry in command.Entries) - { - foreach (var foreignKey in entry.EntityType.GetForeignKeys()) - { - if (!CanCreateDependency(foreignKey, command, principal: false) - || !IsModified(foreignKey.Properties, entry)) - { - continue; - } + // Handle the cases where there is no relational model and where there is no mapped foreign key constraint (TPC) + foreach (var (_, keyValue) in GetForeignKeyValues( + command, principal, fromOriginalValues: !principal, checkStoreGenerated: principal)) + { + AddPredecessor(predecessorsMap, keyValue, command); + } + } - var dependentKeyValue = foreignKey.GetDependentKeyValueFactory() - ?.CreateDependentEquatableKey(entry, fromOriginalValues: true); + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + private void AddForeignKeyEdges( + IReadOnlyModificationCommand command, + Dictionary> predecessorsMap, + bool principal) + { + foreach (var (constraint, keyValue) in GetForeignKeyConstraintValues(command, principal, fromOriginalValues: principal)) + { + AddMatchingPredecessorEdge(predecessorsMap, keyValue, command, constraint, checkStoreGenerated: !principal); + } - if (dependentKeyValue != null) - { - if (!originalPredecessorsMap.TryGetValue(dependentKeyValue, out var predecessorCommands)) - { - predecessorCommands = []; - originalPredecessorsMap.Add(dependentKeyValue, predecessorCommands); - } + // Handle the cases where there is no relational model and where there is no mapped foreign key constraint (TPC) + foreach (var (foreignKey, keyValue) in GetForeignKeyValues( + command, principal, fromOriginalValues: principal, checkStoreGenerated: false)) + { + AddMatchingPredecessorEdge(predecessorsMap, keyValue, command, foreignKey, checkStoreGenerated: !principal); + } + } - predecessorCommands.Add(command); - } - } - } - } - } + private static List<(IForeignKeyConstraint Constraint, object KeyValue)> GetForeignKeyConstraintValues( + IReadOnlyModificationCommand command, + bool principal, + bool fromOriginalValues) + { + var result = new List<(IForeignKeyConstraint, object)>(); + if (command.Table == null) + { + return result; } - foreach (var command in _modificationCommandGraph.Vertices) + var constraints = principal ? command.Table.ReferencingForeignKeyConstraints : command.Table.ForeignKeyConstraints; + foreach (var constraint in constraints) { - if (command.EntityState is EntityState.Modified or EntityState.Added) + var columns = principal ? constraint.PrincipalUniqueConstraint.Columns : constraint.Columns; + if (!IsModified(columns, command)) { - if (command.Table != null) - { - foreach (var foreignKey in command.Table.ForeignKeyConstraints) - { - if (!IsModified(foreignKey.Columns, command)) - { - continue; - } - - var dependentKeyValue = ((ForeignKeyConstraint)foreignKey).GetRowForeignKeyValueFactory() - .CreateDependentEquatableKeyValue(command); - if (dependentKeyValue is null) - { - continue; - } + continue; + } - AddMatchingPredecessorEdge( - predecessorsMap, dependentKeyValue, command, foreignKey, checkStoreGenerated: true); - } - } + var foreignKeyValueFactory = ((ForeignKeyConstraint)constraint).GetRowForeignKeyValueFactory(); + var keyValue = principal + ? foreignKeyValueFactory.CreatePrincipalEquatableKeyValue(command, fromOriginalValues) + : foreignKeyValueFactory.CreateDependentEquatableKeyValue(command, fromOriginalValues); - // ReSharper disable once ForCanBeConvertedToForeach - for (var entryIndex = 0; entryIndex < command.Entries.Count; entryIndex++) - { - var entry = command.Entries[entryIndex]; - foreach (var foreignKey in entry.EntityType.GetForeignKeys()) - { - if (!CanCreateDependency(foreignKey, command, principal: false) - || !IsModified(foreignKey.Properties, entry)) - { - continue; - } + if (keyValue is null) + { + Check.DebugAssert(!principal, "null principal keyValue"); + continue; + } - var dependentKeyValue = foreignKey.GetDependentKeyValueFactory() - ?.CreateDependentEquatableKey(entry); - if (dependentKeyValue == null) - { - continue; - } + result.Add((constraint, keyValue)); + } - AddMatchingPredecessorEdge( - predecessorsMap, dependentKeyValue, command, foreignKey, checkStoreGenerated: true); - } - } - } + return result; + } - if (command.EntityState is EntityState.Modified or EntityState.Deleted) + private static List<(IForeignKey ForeignKey, object KeyValue)> GetForeignKeyValues( + IReadOnlyModificationCommand command, + bool principal, + bool fromOriginalValues, + bool checkStoreGenerated) + { + var result = new List<(IForeignKey, object)>(); + // ReSharper disable once ForCanBeConvertedToForeach + for (var i = 0; i < command.Entries.Count; i++) + { + var entry = command.Entries[i]; + var foreignKeys = principal + ? entry.EntityType.GetReferencingForeignKeys() + : entry.EntityType.GetForeignKeys(); + foreach (var foreignKey in foreignKeys) { - if (command.Table != null) + var properties = principal ? foreignKey.PrincipalKey.Properties : foreignKey.Properties; + if (!CanCreateDependency(foreignKey, command, principal) + || !IsModified(properties, entry) + || (command.Table != null + && (!checkStoreGenerated || !IsStoreGenerated(entry, foreignKey.PrincipalKey)) + && foreignKey.GetMappedConstraints().Any())) { - foreach (var foreignKey in command.Table.ReferencingForeignKeyConstraints) - { - if (!IsModified(foreignKey.PrincipalUniqueConstraint.Columns, command)) - { - continue; - } + continue; + } - var principalKeyValue = ((ForeignKeyConstraint)foreignKey).GetRowForeignKeyValueFactory() - .CreatePrincipalEquatableKeyValue(command, fromOriginalValues: true); - Check.DebugAssert(principalKeyValue != null, "null principalKeyValue"); - AddMatchingPredecessorEdge( - originalPredecessorsMap, principalKeyValue, command, foreignKey); - } + var foreignKeyValueFactory = foreignKey.GetDependentKeyValueFactory(); + var keyValue = principal + ? foreignKeyValueFactory.CreatePrincipalEquatableKey(entry, fromOriginalValues) + : foreignKeyValueFactory.CreateDependentEquatableKey(entry, fromOriginalValues); - // 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++) - { - var entry = command.Entries[entryIndex]; - foreach (var foreignKey in entry.EntityType.GetReferencingForeignKeys()) - { - 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 + if (keyValue != null) { - // 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); - } - } + result.Add((foreignKey, keyValue)); } } } + + return result; + } + + private static void AddPredecessor( + Dictionary> predecessorsMap, + object keyValue, + IReadOnlyModificationCommand command) + { + if (!predecessorsMap.TryGetValue(keyValue, out var predecessorCommands)) + { + predecessorCommands = []; + predecessorsMap.Add(keyValue, predecessorCommands); + } + + predecessorCommands.Add(command); } private static bool IsStoreGenerated(IUpdateEntry entry, IKey key) @@ -1149,7 +1067,7 @@ private void AddMatchingPredecessorEdge( T keyValue, IReadOnlyModificationCommand command, IForeignKeyConstraint foreignKey, - bool checkStoreGenerated = false) + bool checkStoreGenerated) where T : notnull { if (predecessorsMap.TryGetValue(keyValue, out var predecessorCommands)) From c73fec12f131ed72b474eac9510acbe066d74458 Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Fri, 17 Apr 2026 14:22:24 -0700 Subject: [PATCH 2/2] Use iterators --- .../Update/Internal/CommandBatchPreparer.cs | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs index eca48c0efe0..15515fd683e 100644 --- a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs +++ b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs @@ -726,15 +726,14 @@ private void AddForeignKeyEdges( } } - private static List<(IForeignKeyConstraint Constraint, object KeyValue)> GetForeignKeyConstraintValues( + private static IEnumerable<(IForeignKeyConstraint Constraint, object KeyValue)> GetForeignKeyConstraintValues( IReadOnlyModificationCommand command, bool principal, bool fromOriginalValues) { - var result = new List<(IForeignKeyConstraint, object)>(); if (command.Table == null) { - return result; + yield break; } var constraints = principal ? command.Table.ReferencingForeignKeyConstraints : command.Table.ForeignKeyConstraints; @@ -757,19 +756,16 @@ private void AddForeignKeyEdges( continue; } - result.Add((constraint, keyValue)); + yield return (constraint, keyValue); } - - return result; } - private static List<(IForeignKey ForeignKey, object KeyValue)> GetForeignKeyValues( + private static IEnumerable<(IForeignKey ForeignKey, object KeyValue)> GetForeignKeyValues( IReadOnlyModificationCommand command, bool principal, bool fromOriginalValues, bool checkStoreGenerated) { - var result = new List<(IForeignKey, object)>(); // ReSharper disable once ForCanBeConvertedToForeach for (var i = 0; i < command.Entries.Count; i++) { @@ -796,12 +792,10 @@ private void AddForeignKeyEdges( if (keyValue != null) { - result.Add((foreignKey, keyValue)); + yield return (foreignKey, keyValue); } } } - - return result; } private static void AddPredecessor(