diff --git a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs index 6e679019454..a70dedb1688 100644 --- a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs +++ b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs @@ -178,7 +178,6 @@ protected virtual IEnumerable CreateModificationCo foreach (var mapping in mappings) { var table = mapping.Table; - var tableKey = (table.Name, table.Schema); IModificationCommand command; var isMainEntry = true; @@ -189,6 +188,7 @@ protected virtual IEnumerable CreateModificationCo sharedTablesCommandsMap = new Dictionary<(string, string?), SharedTableEntryMap>(); } + var tableKey = (table.Name, table.Schema); if (!sharedTablesCommandsMap.TryGetValue(tableKey, out var sharedCommandsMap)) { sharedCommandsMap = new SharedTableEntryMap(table, updateAdapter); diff --git a/src/EFCore.Relational/Update/Internal/ModificationCommandComparer.cs b/src/EFCore.Relational/Update/Internal/ModificationCommandComparer.cs index 914d7be6332..7f1fefe0a31 100644 --- a/src/EFCore.Relational/Update/Internal/ModificationCommandComparer.cs +++ b/src/EFCore.Relational/Update/Internal/ModificationCommandComparer.cs @@ -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; @@ -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; } } } diff --git a/test/EFCore.Relational.Specification.Tests/TransactionTestBase.cs b/test/EFCore.Relational.Specification.Tests/TransactionTestBase.cs index f4fe9ed5d68..e1c43e6f11e 100644 --- a/test/EFCore.Relational.Specification.Tests/TransactionTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/TransactionTestBase.cs @@ -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().OrderBy(c => c.Id).Last()).State = EntityState.Added; @@ -69,9 +69,9 @@ public virtual async Task SaveChanges_can_be_used_with_no_transaction(bool async Assert.Equal( new List { + -77, 1, 2, - 77 }, context.Set().OrderBy(c => c.Id).Select(e => e.Id).ToList()); } @@ -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().OrderBy(c => c.Id).Last()).State = EntityState.Added; @@ -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().Single(c => c.Id == 77)).State = EntityState.Deleted; + context.Entry(context.Set().Single(c => c.Id == -77)).State = EntityState.Deleted; if (async) { @@ -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().OrderBy(c => c.Id).Last()).State = EntityState.Added; @@ -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().Single(c => c.Id == 77)).State = EntityState.Deleted; + context.Entry(context.Set().Single(c => c.Id == -77)).State = EntityState.Deleted; if (async) { @@ -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) { @@ -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) @@ -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().Max(c => c.Id)); + Assert.Equal(-78, context.Set().Min(c => c.Id)); } } diff --git a/test/EFCore.SqlServer.FunctionalTests/BatchingTest.cs b/test/EFCore.SqlServer.FunctionalTests/BatchingTest.cs index 0e61b60e550..67ca340baa6 100644 --- a/test/EFCore.SqlServer.FunctionalTests/BatchingTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/BatchingTest.cs @@ -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; @@ -125,6 +128,100 @@ 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().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 => + { + RemoveBlogs(owner); + }); + + using (var context = CreateContext()) + { + context.Owners.AddRange(owners); + + Assert.Empty(context.Blogs); + } + + Fixture.Reseed(); + + void RemoveBlogs(Owner owner) + { + using var context = (BloggingContext)Fixture.CreateContext(useConnectionString: true); + + context.RemoveRange(context.Blogs.Where(b => b.OwnerId == owner.Id)); + + context.SaveChanges(); + } + } + [ConditionalFact] public void Inserts_when_database_type_is_different() { @@ -251,6 +348,7 @@ private class Blog public string OwnerId { get; set; } public Owner Owner { get; set; } public byte[] Version { get; set; } + public ICollection Posts { get; } = new HashSet(); } private class Owner @@ -260,6 +358,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 Comments { get; } = new HashSet(); + } + + private class Comment + { + public int CommentId { get; set; } + public int PostId { get; set; } + public Post Post { get; set; } + } + public class BatchingTestFixture : SharedStoreFixtureBase { protected override string StoreName { get; } = "BatchingTest"; @@ -284,10 +398,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() + ?? 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); } }