diff --git a/src/Shared/Multigraph.cs b/src/Shared/Multigraph.cs index 8c0783c09e7..8f0d59ab767 100644 --- a/src/Shared/Multigraph.cs +++ b/src/Shared/Multigraph.cs @@ -218,21 +218,7 @@ private IReadOnlyList> TopologicalSortCore( if (predecessorCounts[successor] == 0) { nextRootsQueue.Add(successor); - - // Detect batch boundary (if batching is enabled). - // If the successor has any predecessor where the edge requires a batching boundary, and that predecessor is - // already in the current batch, then the next batch will have to be executed in a separate batch. - // TODO: Optimization: Instead of currentBatchSet, store a batch counter on each vertex, and check if later - // vertexes have a boundary-requiring dependency on a vertex with the same batch counter. - if (withBatching - && _predecessorMap[successor].Any( - kv => - (kv.Value is Edge { RequiresBatchingBoundary: true } - || kv.Value is IEnumerable edges && edges.Any(e => e.RequiresBatchingBoundary)) - && currentBatchSet.Contains(kv.Key))) - { - batchBoundaryRequired = true; - } + CheckBatchingBoundary(successor); } } } @@ -278,6 +264,7 @@ private IReadOnlyList> TopologicalSortCore( if (predecessorCounts[candidateVertex] == 0) { currentRootsQueue.Add(candidateVertex); + CheckBatchingBoundary(candidateVertex); broken = true; } @@ -334,6 +321,24 @@ private IReadOnlyList> TopologicalSortCore( } return result; + + // Detect batch boundary (if batching is enabled). + // If the successor has any predecessor where the edge requires a batching boundary, and that predecessor is + // already in the current batch, then the next batch will have to be executed in a separate batch. + // TODO: Optimization: Instead of currentBatchSet, store a batch counter on each vertex, and check if later + // vertexes have a boundary-requiring dependency on a vertex with the same batch counter. + void CheckBatchingBoundary(TVertex vertex) + { + if (withBatching + && _predecessorMap[vertex].Any( + kv => + (kv.Value is Edge { RequiresBatchingBoundary: true } + || kv.Value is IEnumerable edges && edges.Any(e => e.RequiresBatchingBoundary)) + && currentBatchSet.Contains(kv.Key))) + { + batchBoundaryRequired = true; + } + } } private void ThrowCycle( diff --git a/test/EFCore.Relational.Specification.Tests/Update/NonSharedModelUpdatesTestBase.cs b/test/EFCore.Relational.Specification.Tests/Update/NonSharedModelUpdatesTestBase.cs new file mode 100644 index 00000000000..83cf8585629 --- /dev/null +++ b/test/EFCore.Relational.Specification.Tests/Update/NonSharedModelUpdatesTestBase.cs @@ -0,0 +1,137 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// ReSharper disable MethodHasAsyncOverload +namespace Microsoft.EntityFrameworkCore.Update; + +#nullable enable + +public abstract class NonSharedModelUpdatesTestBase : NonSharedModelTestBase +{ + protected override string StoreName + => "NonSharedModelUpdatesTestBase"; + + [ConditionalTheory] // Issue #29356 + [InlineData(false)] + [InlineData(true)] + public virtual async Task Principal_and_dependent_roundtrips_with_cycle_breaking(bool async) + { + var contextFactory = await InitializeAsync( + onModelCreating: mb => + { + mb.Entity( + b => + { + b.HasOne(a => a.AuthorsClub) + .WithMany() + .HasForeignKey(a => a.AuthorsClubId); + }); + + mb.Entity( + b => + { + b.HasOne(book => book.Author) + .WithMany() + .HasForeignKey(book => book.AuthorId); + }); + }); + + await ExecuteWithStrategyInTransactionAsync( + contextFactory, + async context => + { + context.Add( + new Book + { + Author = new Author + { + Name = "Alice", + AuthorsClub = new AuthorsClub + { + Name = "AC South" + } + } + }); + + await context.SaveChangesAsync(); + }, + async context => + { + AuthorsClub authorsClubNorth = new() + { + Name = "AC North" + }; + Author authorOfTheYear2023 = new() + { + Name = "Author of the year 2023", + AuthorsClub = authorsClubNorth + }; + context.Add(authorsClubNorth); + context.Add(authorOfTheYear2023); + + var book = await context + .Set() + .Include(b => b.Author) + .SingleAsync(); + var authorOfTheYear2022 = book.Author!; + book.Author = authorOfTheYear2023; + + context.Remove(authorOfTheYear2022); + + if (async) + { + await context.SaveChangesAsync(); + } + else + { + context.SaveChanges(); + } + }); + } + + private class AuthorsClub + { + public int Id { get; set; } + public string? Name { get; set; } + } + + private class Author + { + public int Id { get; set; } + public string? Name { get; set; } + public int AuthorsClubId { get; set; } + public AuthorsClub? AuthorsClub { get; set; } + } + + private class Book + { + public int Id { get; set; } + public string? Title { get; set; } + public int AuthorId { get; set; } + public Author? Author { get; set; } + } + + protected virtual void ExecuteWithStrategyInTransaction( + ContextFactory contextFactory, + Action testOperation, + Action? nestedTestOperation1 = null, + Action? nestedTestOperation2 = null, + Action? nestedTestOperation3 = null) + => TestHelpers.ExecuteWithStrategyInTransaction( + contextFactory.CreateContext, UseTransaction, testOperation, nestedTestOperation1, nestedTestOperation2, nestedTestOperation3); + + protected virtual Task ExecuteWithStrategyInTransactionAsync( + ContextFactory contextFactory, + Func testOperation, + Func? nestedTestOperation1 = null, + Func? nestedTestOperation2 = null, + Func? nestedTestOperation3 = null) + => TestHelpers.ExecuteWithStrategyInTransactionAsync( + contextFactory.CreateContext, UseTransaction, testOperation, nestedTestOperation1, nestedTestOperation2, nestedTestOperation3); + + public void UseTransaction(DatabaseFacade facade, IDbContextTransaction transaction) + => facade.UseTransaction(transaction.GetDbTransaction()); + + protected TestSqlLoggerFactory TestSqlLoggerFactory + => (TestSqlLoggerFactory)ListLoggerFactory; +} diff --git a/test/EFCore.SqlServer.FunctionalTests/Update/NonSharedModelUpdatesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Update/NonSharedModelUpdatesSqlServerTest.cs new file mode 100644 index 00000000000..5df73ad30fa --- /dev/null +++ b/test/EFCore.SqlServer.FunctionalTests/Update/NonSharedModelUpdatesSqlServerTest.cs @@ -0,0 +1,91 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.EntityFrameworkCore.Update; + +public class NonSharedModelUpdatesSqlServerTest : NonSharedModelUpdatesTestBase +{ + public override async Task Principal_and_dependent_roundtrips_with_cycle_breaking(bool async) + { + await base.Principal_and_dependent_roundtrips_with_cycle_breaking(async); + + AssertSql( +""" +@p0='AC South' (Size = 4000) + +SET IMPLICIT_TRANSACTIONS OFF; +SET NOCOUNT ON; +INSERT INTO [AuthorsClub] ([Name]) +OUTPUT INSERTED.[Id] +VALUES (@p0); +""", + // +""" +@p1='1' +@p2='Alice' (Size = 4000) + +SET IMPLICIT_TRANSACTIONS OFF; +SET NOCOUNT ON; +INSERT INTO [Author] ([AuthorsClubId], [Name]) +OUTPUT INSERTED.[Id] +VALUES (@p1, @p2); +""", + // +""" +@p3='1' +@p4=NULL (Size = 4000) + +SET IMPLICIT_TRANSACTIONS OFF; +SET NOCOUNT ON; +INSERT INTO [Book] ([AuthorId], [Title]) +OUTPUT INSERTED.[Id] +VALUES (@p3, @p4); +""", + // +""" +SELECT TOP(2) [b].[Id], [b].[AuthorId], [b].[Title], [a].[Id], [a].[AuthorsClubId], [a].[Name] +FROM [Book] AS [b] +INNER JOIN [Author] AS [a] ON [b].[AuthorId] = [a].[Id] +""", + // +""" +@p0='AC North' (Size = 4000) + +SET IMPLICIT_TRANSACTIONS OFF; +SET NOCOUNT ON; +INSERT INTO [AuthorsClub] ([Name]) +OUTPUT INSERTED.[Id] +VALUES (@p0); +""", + // +""" +@p1='2' +@p2='Author of the year 2023' (Size = 4000) + +SET IMPLICIT_TRANSACTIONS OFF; +SET NOCOUNT ON; +INSERT INTO [Author] ([AuthorsClubId], [Name]) +OUTPUT INSERTED.[Id] +VALUES (@p1, @p2); +""", + // +""" +@p4='1' +@p3='2' +@p5='1' + +SET NOCOUNT ON; +UPDATE [Book] SET [AuthorId] = @p3 +OUTPUT 1 +WHERE [Id] = @p4; +DELETE FROM [Author] +OUTPUT 1 +WHERE [Id] = @p5; +"""); + } + + private void AssertSql(params string[] expected) + => TestSqlLoggerFactory.AssertBaseline(expected); + + protected override ITestStoreFactory TestStoreFactory => SqlServerTestStoreFactory.Instance; +} diff --git a/test/EFCore.Sqlite.FunctionalTests/Update/NonSharedModelUpdatesSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Update/NonSharedModelUpdatesSqliteTest.cs new file mode 100644 index 00000000000..75b836707a1 --- /dev/null +++ b/test/EFCore.Sqlite.FunctionalTests/Update/NonSharedModelUpdatesSqliteTest.cs @@ -0,0 +1,85 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.EntityFrameworkCore.Update; + +public class NonSharedModelUpdatesSqlServerTest : NonSharedModelUpdatesTestBase +{ + public override async Task Principal_and_dependent_roundtrips_with_cycle_breaking(bool async) + { + await base.Principal_and_dependent_roundtrips_with_cycle_breaking(async); + + AssertSql( +""" +@p0='AC South' (Size = 8) + +INSERT INTO "AuthorsClub" ("Name") +VALUES (@p0) +RETURNING "Id"; +""", + // +""" +@p1='1' +@p2='Alice' (Size = 5) + +INSERT INTO "Author" ("AuthorsClubId", "Name") +VALUES (@p1, @p2) +RETURNING "Id"; +""", + // +""" +@p3='1' +@p4=NULL + +INSERT INTO "Book" ("AuthorId", "Title") +VALUES (@p3, @p4) +RETURNING "Id"; +""", + // +""" +SELECT "b"."Id", "b"."AuthorId", "b"."Title", "a"."Id", "a"."AuthorsClubId", "a"."Name" +FROM "Book" AS "b" +INNER JOIN "Author" AS "a" ON "b"."AuthorId" = "a"."Id" +LIMIT 2 +""", + // +""" +@p0='AC North' (Size = 8) + +INSERT INTO "AuthorsClub" ("Name") +VALUES (@p0) +RETURNING "Id"; +""", + // +""" +@p1='2' +@p2='Author of the year 2023' (Size = 23) + +INSERT INTO "Author" ("AuthorsClubId", "Name") +VALUES (@p1, @p2) +RETURNING "Id"; +""", + // +""" +@p4='1' +@p3='2' + +UPDATE "Book" SET "AuthorId" = @p3 +WHERE "Id" = @p4 +RETURNING 1; +""", + // +""" +@p0='1' + +DELETE FROM "Author" +WHERE "Id" = @p0 +RETURNING 1; +"""); + } + + private void AssertSql(params string[] expected) + => TestSqlLoggerFactory.AssertBaseline(expected); + + protected override ITestStoreFactory TestStoreFactory => SqliteTestStoreFactory.Instance; +}