Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Include key values in the update topological sort #21556

Merged
1 commit merged into from
Jul 10, 2020
Merged

Include key values in the update topological sort #21556

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 75 additions & 23 deletions src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@
using System.Linq;
using System.Text;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Utilities;
using Microsoft.Extensions.DependencyInjection;
Expand Down Expand Up @@ -290,9 +292,8 @@ private void AddUnchangedSharingEntries(
// 2. Commands deleting rows or modifying the foreign key values must precede
// commands deleting rows or modifying the candidate key values (when supported) of rows
// that are currently being referenced by the former
// 3. Commands deleting rows or modifying the foreign key values must precede
// commands adding or modifying the foreign key values to the same values
// if foreign key is unique
// 3. Commands deleting rows or modifying unique constraint values must precede
// commands adding or modifying unique constraint values to the same values
/// <summary>
/// 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
Expand Down Expand Up @@ -570,6 +571,12 @@ private void AddForeignKeyEdges(
{
foreach (var command in commandGraph.Vertices)
{
if (command.Predecessor != null)
{
// This is usually an implicit relationship between TPT rows for the same entity
commandGraph.AddEdge(command.Predecessor, command, null);
}

switch (command.EntityState)
{
case EntityState.Modified:
Expand Down Expand Up @@ -638,30 +645,26 @@ private static void AddMatchingPredecessorEdge(
IKeyValueIndex dependentKeyValue,
Multigraph<ModificationCommand, IAnnotatable> commandGraph,
ModificationCommand command,
IForeignKey foreignKey)
IAnnotatable edge)
{
if (predecessorsMap.TryGetValue(dependentKeyValue, out var predecessorCommands))
{
foreach (var predecessor in predecessorCommands)
{
if (predecessor != command)
{
commandGraph.AddEdge(predecessor, command, foreignKey);
commandGraph.AddEdge(predecessor, command, edge);
}
}
}
}

private void AddUniqueValueEdges(Multigraph<ModificationCommand, IAnnotatable> commandGraph)
{
Dictionary<IIndex, Dictionary<object[], ModificationCommand>> predecessorsMap = null;
Dictionary<IIndex, Dictionary<object[], ModificationCommand>> indexPredecessorsMap = null;
var keyPredecessorsMap = new Dictionary<IKeyValueIndex, List<ModificationCommand>>();
foreach (var command in commandGraph.Vertices)
{
if (command.Predecessor != null)
{
commandGraph.AddEdge(command.Predecessor, command, null);
}

if (command.EntityState != EntityState.Modified
&& command.EntityState != EntityState.Deleted)
{
Expand All @@ -682,11 +685,11 @@ private void AddUniqueValueEdges(Multigraph<ModificationCommand, IAnnotatable> c
var valueFactory = index.GetNullableValueFactory<object[]>();
if (valueFactory.TryCreateFromOriginalValues(entry, out var indexValue))
{
predecessorsMap ??= new Dictionary<IIndex, Dictionary<object[], ModificationCommand>>();
if (!predecessorsMap.TryGetValue(index, out var predecessorCommands))
indexPredecessorsMap ??= new Dictionary<IIndex, Dictionary<object[], ModificationCommand>>();
if (!indexPredecessorsMap.TryGetValue(index, out var predecessorCommands))
{
predecessorCommands = new Dictionary<object[], ModificationCommand>(valueFactory.EqualityComparer);
predecessorsMap.Add(index, predecessorCommands);
indexPredecessorsMap.Add(index, predecessorCommands);
}

if (!predecessorCommands.ContainsKey(indexValue))
Expand All @@ -695,19 +698,41 @@ private void AddUniqueValueEdges(Multigraph<ModificationCommand, IAnnotatable> c
}
}
}
}
}

if (predecessorsMap == null)
{
return;
if (command.EntityState != EntityState.Deleted)
{
continue;
}

foreach (var key in entry.EntityType.GetKeys().Where(k => k.GetMappedConstraints().Any()))
{
var principalKeyValue = _keyValueIndexFactorySource
.GetKeyValueIndexFactory(key)
.CreatePrincipalKeyValue(entry, null);

if (principalKeyValue != null)
{
if (!keyPredecessorsMap.TryGetValue(principalKeyValue, out var predecessorCommands))
{
predecessorCommands = new List<ModificationCommand>();
keyPredecessorsMap.Add(principalKeyValue, predecessorCommands);
}

predecessorCommands.Add(command);
}
}
}
}

foreach (var command in commandGraph.Vertices)
if (indexPredecessorsMap != null)
{
if (command.EntityState == EntityState.Modified
|| command.EntityState == EntityState.Added)
foreach (var command in commandGraph.Vertices)
{
if (command.EntityState == EntityState.Deleted)
{
continue;
}

foreach (var entry in command.Entries)
{
foreach (var index in entry.EntityType.GetIndexes().Where(i => i.IsUnique && i.GetMappedTableIndexes().Any()))
Expand All @@ -720,7 +745,7 @@ private void AddUniqueValueEdges(Multigraph<ModificationCommand, IAnnotatable> c

var valueFactory = index.GetNullableValueFactory<object[]>();
if (valueFactory.TryCreateFromCurrentValues(entry, out var indexValue)
&& predecessorsMap.TryGetValue(index, out var predecessorCommands)
&& indexPredecessorsMap.TryGetValue(index, out var predecessorCommands)
&& predecessorCommands.TryGetValue(indexValue, out var predecessor)
&& predecessor != command)
{
Expand All @@ -730,6 +755,33 @@ private void AddUniqueValueEdges(Multigraph<ModificationCommand, IAnnotatable> c
}
}
}

if (keyPredecessorsMap != null)
{
foreach (var command in commandGraph.Vertices)
{
if (command.EntityState != EntityState.Added)
{
continue;
}

foreach (var entry in command.Entries)
{
foreach (var key in entry.EntityType.GetKeys().Where(k => k.GetMappedConstraints().Any()))
{
var principalKeyValue = _keyValueIndexFactorySource
.GetKeyValueIndexFactory(key)
.CreatePrincipalKeyValue(entry, null);

if (principalKeyValue != null)
{
AddMatchingPredecessorEdge(
keyPredecessorsMap, principalKeyValue, commandGraph, command, key);
}
}
}
}
}
}
}
}
4 changes: 2 additions & 2 deletions src/EFCore.Relational/Update/Internal/IKeyValueIndexFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ public interface IKeyValueIndexFactory
/// 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.
/// </summary>
IKeyValueIndex CreatePrincipalKeyValue([NotNull] IUpdateEntry entry, [NotNull] IForeignKey foreignKey);
IKeyValueIndex CreatePrincipalKeyValue([NotNull] IUpdateEntry entry, [CanBeNull] IForeignKey foreignKey);

/// <summary>
/// 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.
/// </summary>
IKeyValueIndex CreatePrincipalKeyValueFromOriginalValues([NotNull] IUpdateEntry entry, [NotNull] IForeignKey foreignKey);
IKeyValueIndex CreatePrincipalKeyValueFromOriginalValues([NotNull] IUpdateEntry entry, [CanBeNull] IForeignKey foreignKey);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Update/Internal/KeyValueIndex.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public sealed class KeyValueIndex<TKey> : IKeyValueIndex
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public KeyValueIndex(
[NotNull] IForeignKey foreignKey,
[CanBeNull] IForeignKey foreignKey,
[NotNull] TKey keyValue,
[NotNull] IEqualityComparer<TKey> keyComparer,
bool fromOriginalValues)
Expand Down
9 changes: 7 additions & 2 deletions src/EFCore/ChangeTracking/Internal/IdentityMap.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,8 @@ private void Add(TKey key, InternalEntityEntry entry, bool updateDuplicate)
}
}

if (!bothStatesEquivalent)
if (!bothStatesEquivalent
&& Key.IsPrimaryKey())
{
entry.SharedIdentityEntry = existingEntry;
existingEntry.SharedIdentityEntry = entry;
Expand Down Expand Up @@ -413,7 +414,11 @@ protected virtual void Remove([NotNull] TKey key, [NotNull] InternalEntityEntry

if (otherEntry == null)
{
_identityMap.Remove(key);
if (_identityMap.TryGetValue(key, out var existingEntry)
&& existingEntry == entry)
{
_identityMap.Remove(key);
}
}
else
{
Expand Down
7 changes: 7 additions & 0 deletions test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBaseOneToManyAk.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,13 @@ public virtual void Save_required_many_to_one_dependents_with_alternate_key(
new1d.Parent = null;
new1dd.Parent = null;

var old1 = root.RequiredChildrenAk.OrderBy(c => c.Id).Last();

// Test replacing entity with the same AK, but different PK
new1.AlternateId = old1.AlternateId;
context.Remove(old1);
context.RemoveRange(old1.Children);
context.RemoveRange(old1.CompositeChildren);
context.AddRange(new1, new1d, new1dd, new2a, new2d, new2dd, new2b, new2ca, new2cb);
}

Expand Down