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

Sort inserts to preserve tracking order in more cases. #25874

Merged
merged 1 commit into from
Sep 7, 2021
+156 −25
Merged

Sort inserts to preserve tracking order in more cases. #25874

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
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,6 @@ protected virtual IEnumerable<IReadOnlyModificationCommand> CreateModificationCo
foreach (var mapping in mappings)
{
var table = mapping.Table;
var tableKey = (table.Name, table.Schema);

IModificationCommand command;
var isMainEntry = true;
Expand All @@ -189,6 +188,7 @@ protected virtual IEnumerable<IReadOnlyModificationCommand> CreateModificationCo
sharedTablesCommandsMap = new Dictionary<(string, string?), SharedTableEntryMap<IModificationCommand>>();
}

var tableKey = (table.Name, table.Schema);
if (!sharedTablesCommandsMap.TryGetValue(tableKey, out var sharedCommandsMap))
{
sharedCommandsMap = new SharedTableEntryMap<IModificationCommand>(table, updateAdapter);
Expand Down
21 changes: 9 additions & 12 deletions src/EFCore.Relational/Update/Internal/ModificationCommandComparer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ public virtual int Compare(IReadOnlyModificationCommand? x, IReadOnlyModificatio
return 1;
}

result = StringComparer.Ordinal.Compare(x.Schema, y.Schema);
result = StringComparer.Ordinal.Compare(x.TableName, y.TableName);
if (result != 0)
{
return result;
}

result = StringComparer.Ordinal.Compare(x.TableName, y.TableName);
result = StringComparer.Ordinal.Compare(x.Schema, y.Schema);
if (result != 0)
{
return result;
Expand Down Expand Up @@ -82,18 +82,15 @@ public virtual int Compare(IReadOnlyModificationCommand? x, IReadOnlyModificatio
}
}

if (xState != EntityState.Added)
var xKey = xEntry.EntityType.FindPrimaryKey()!;
for (var i = 0; i < xKey.Properties.Count; i++)
{
var xKey = xEntry.EntityType.FindPrimaryKey()!;
for (var i = 0; i < xKey.Properties.Count; i++)
{
var xKeyProperty = xKey.Properties[i];
var xKeyProperty = xKey.Properties[i];

result = xKeyProperty.GetCurrentValueComparer().Compare(xEntry, yEntry);
if (result != 0)
{
return result;
}
result = xKeyProperty.GetCurrentValueComparer().Compare(xEntry, yEntry);
if (result != 0)
{
return result;
}
}
}
Expand Down
18 changes: 9 additions & 9 deletions test/EFCore.Relational.Specification.Tests/TransactionTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public virtual async Task SaveChanges_can_be_used_with_no_transaction(bool async
context.Database.AutoTransactionsEnabled = false;

context.Add(
new TransactionCustomer { Id = 77, Name = "Bobble" });
new TransactionCustomer { Id = -77, Name = "Bobble" });

context.Entry(context.Set<TransactionCustomer>().OrderBy(c => c.Id).Last()).State = EntityState.Added;

Expand All @@ -69,9 +69,9 @@ public virtual async Task SaveChanges_can_be_used_with_no_transaction(bool async
Assert.Equal(
new List<int>
{
-77,
1,
2,
77
},
context.Set<TransactionCustomer>().OrderBy(c => c.Id).Select(e => e.Id).ToList());
}
Expand Down Expand Up @@ -119,7 +119,7 @@ public virtual async Task SaveChanges_uses_enlisted_transaction(bool async, bool
context.Database.AutoTransactionsEnabled = autoTransactionsEnabled;

context.Add(
new TransactionCustomer { Id = 77, Name = "Bobble" });
new TransactionCustomer { Id = -77, Name = "Bobble" });

context.Entry(context.Set<TransactionCustomer>().OrderBy(c => c.Id).Last()).State = EntityState.Added;

Expand Down Expand Up @@ -151,7 +151,7 @@ public virtual async Task SaveChanges_uses_enlisted_transaction(bool async, bool
if (!autoTransactionsEnabled)
{
using var context = CreateContext();
context.Entry(context.Set<TransactionCustomer>().Single(c => c.Id == 77)).State = EntityState.Deleted;
context.Entry(context.Set<TransactionCustomer>().Single(c => c.Id == -77)).State = EntityState.Deleted;

if (async)
{
Expand Down Expand Up @@ -275,7 +275,7 @@ public virtual async Task SaveChanges_uses_ambient_transaction(bool async, bool
context.Database.AutoTransactionsEnabled = autoTransactionsEnabled;

context.Add(
new TransactionCustomer { Id = 77, Name = "Bobble" });
new TransactionCustomer { Id = -77, Name = "Bobble" });

context.Entry(context.Set<TransactionCustomer>().OrderBy(c => c.Id).Last()).State = EntityState.Added;

Expand Down Expand Up @@ -305,7 +305,7 @@ public virtual async Task SaveChanges_uses_ambient_transaction(bool async, bool
Fixture.ListLoggerFactory.Log.Skip(2).First().Message);

using var context = CreateContext();
context.Entry(context.Set<TransactionCustomer>().Single(c => c.Id == 77)).State = EntityState.Deleted;
context.Entry(context.Set<TransactionCustomer>().Single(c => c.Id == -77)).State = EntityState.Deleted;

if (async)
{
Expand Down Expand Up @@ -1268,7 +1268,7 @@ public virtual async Task SaveChanges_can_be_used_with_no_savepoint(bool async)
? await context.Database.BeginTransactionAsync()
: context.Database.BeginTransaction();

context.Add(new TransactionCustomer { Id = 77, Name = "Bobble" });
context.Add(new TransactionCustomer { Id = -77, Name = "Bobble" });

if (async)
{
Expand All @@ -1279,7 +1279,7 @@ public virtual async Task SaveChanges_can_be_used_with_no_savepoint(bool async)
context.SaveChanges();
}

context.Add(new TransactionCustomer { Id = 78, Name = "Hobble" });
context.Add(new TransactionCustomer { Id = -78, Name = "Hobble" });
context.Add(new TransactionCustomer { Id = 1, Name = "Gobble" }); // Cause SaveChanges failure

if (async)
Expand All @@ -1298,7 +1298,7 @@ public virtual async Task SaveChanges_can_be_used_with_no_savepoint(bool async)

using (var context = CreateContext())
{
Assert.Equal(78, context.Set<TransactionCustomer>().Max(c => c.Id));
Assert.Equal(-78, context.Set<TransactionCustomer>().Min(c => c.Id));
}
}

Expand Down
140 changes: 137 additions & 3 deletions test/EFCore.SqlServer.FunctionalTests/BatchingTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore.Diagnostics.Internal;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.SqlServer.Diagnostics.Internal;
using Microsoft.EntityFrameworkCore.SqlServer.Infrastructure.Internal;
using Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.TestUtilities;
using Xunit;
Expand Down Expand Up @@ -125,6 +128,93 @@ public void Inserts_and_updates_are_batched_correctly()
context => AssertDatabaseState(context, true, expectedBlogs));
}

[ConditionalTheory]
[InlineData(1)]
[InlineData(3)]
[InlineData(4)]
[InlineData(100)]
public void Insertion_order_is_preserved(int maxBatchSize)
{
var blogId = new Guid();

TestHelpers.ExecuteWithStrategyInTransaction(
() => (BloggingContext)Fixture.CreateContext(maxBatchSize: maxBatchSize),
UseTransaction,
context =>
{
var owner = new Owner();
var blog = new Blog
{
Owner = owner
};
for (var i = 0; i < 20; i++)
{
context.Add(new Post { Order = i, Blog = blog });
}
context.SaveChanges();
blogId = blog.Id;
},
context =>
{
var posts = context.Set<Post>().Where(p => p.BlogId == blogId).OrderBy(p => p.Order);
var lastId = 0;
foreach (var post in posts)
{
Assert.True(post.PostId > lastId, $"Last ID: {lastId}, current ID: {post.PostId}");
lastId = post.PostId;
}
});
}

[ConditionalFact]
public void Deadlock_on_deletes_with_dependents_is_handled_correctly()
{
var owners = new[] { new Owner { Name = "0" }, new Owner { Name = "1" } };
using (var context = CreateContext())
{
context.Owners.AddRange(owners);

for (var h = 0; h <= 40; h++)
{
var owner = owners[h % 2];
var blog = new Blog
{
Id = Guid.NewGuid(),
Owner = owner,
Order = h
};

for (var i = 0; i <= 40; i++)
{
blog.Posts.Add(new Post { Comments = { new Comment() } });
}

context.Add(blog);
}

context.SaveChanges();
}

Parallel.ForEach(owners, owner =>
{
using var context = (BloggingContext)Fixture.CreateContext(useConnectionString: true);
context.RemoveRange(context.Blogs.Where(b => b.OwnerId == owner.Id));
context.SaveChanges();
});

using (var context = CreateContext())
{
Assert.Empty(context.Blogs);
}

Fixture.Reseed();
}

[ConditionalFact]
public void Inserts_when_database_type_is_different()
{
Expand Down Expand Up @@ -251,6 +341,7 @@ private class Blog
public string OwnerId { get; set; }
public Owner Owner { get; set; }
public byte[] Version { get; set; }
public ICollection<Post> Posts { get; } = new HashSet<Post>();
}

private class Owner
Expand All @@ -260,6 +351,22 @@ private class Owner
public byte[] Version { get; set; }
}

private class Post
{
public int PostId { get; set; }
public int? Order { get; set; }
public Guid BlogId { get; set; }
public Blog Blog { get; set; }
public ICollection<Comment> Comments { get; } = new HashSet<Comment>();
}

private class Comment
{
public int CommentId { get; set; }
public int PostId { get; set; }
public Post Post { get; set; }
}

public class BatchingTestFixture : SharedStoreFixtureBase<PoolableDbContext>
{
protected override string StoreName { get; } = "BatchingTest";
Expand All @@ -284,10 +391,37 @@ ALTER TABLE dbo.Owners
ALTER COLUMN Name nvarchar(MAX);");
}

public DbContext CreateContext(int minBatchSize)
public DbContext CreateContext(
int? minBatchSize = null,
int? maxBatchSize = null,
bool useConnectionString = false,
bool disableConnectionResiliency = false)
{
var optionsBuilder = new DbContextOptionsBuilder(CreateOptions());
new SqlServerDbContextOptionsBuilder(optionsBuilder).MinBatchSize(minBatchSize);
var options = CreateOptions();
var optionsBuilder = new DbContextOptionsBuilder(options);
if (useConnectionString)
{
RelationalOptionsExtension extension = options.FindExtension<SqlServerOptionsExtension>()
?? new SqlServerOptionsExtension();

extension = extension.WithConnection(null).WithConnectionString(((SqlServerTestStore)TestStore).ConnectionString);
((IDbContextOptionsBuilderInfrastructure)optionsBuilder).AddOrUpdateExtension(extension);
}

if (minBatchSize.HasValue)
{
new SqlServerDbContextOptionsBuilder(optionsBuilder).MinBatchSize(minBatchSize.Value);
}

if (maxBatchSize.HasValue)
{
new SqlServerDbContextOptionsBuilder(optionsBuilder).MinBatchSize(maxBatchSize.Value);
}

if (disableConnectionResiliency)
{
new SqlServerDbContextOptionsBuilder(optionsBuilder).ExecutionStrategy(d => new SqlServerExecutionStrategy(d));
}
return new BloggingContext(optionsBuilder.Options);
}
}
Expand Down