Skip to content

Commit b39d274

Browse files
rojiroji
authored
[release/7.0] Fix batching boundary with cycle breaking (#29388)
1 parent be9e79d commit b39d274

File tree

4 files changed

+333
-15
lines changed

4 files changed

+333
-15
lines changed

‎src/Shared/Multigraph.cs‎

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -218,21 +218,7 @@ private IReadOnlyList<List<TVertex>> TopologicalSortCore(
218218
if (predecessorCounts[successor] == 0)
219219
{
220220
nextRootsQueue.Add(successor);
221-
222-
// Detect batch boundary (if batching is enabled).
223-
// If the successor has any predecessor where the edge requires a batching boundary, and that predecessor is
224-
// already in the current batch, then the next batch will have to be executed in a separate batch.
225-
// TODO: Optimization: Instead of currentBatchSet, store a batch counter on each vertex, and check if later
226-
// vertexes have a boundary-requiring dependency on a vertex with the same batch counter.
227-
if (withBatching
228-
&& _predecessorMap[successor].Any(
229-
kv =>
230-
(kv.Value is Edge { RequiresBatchingBoundary: true }
231-
|| kv.Value is IEnumerable<Edge> edges && edges.Any(e => e.RequiresBatchingBoundary))
232-
&& currentBatchSet.Contains(kv.Key)))
233-
{
234-
batchBoundaryRequired = true;
235-
}
221+
CheckBatchingBoundary(successor);
236222
}
237223
}
238224
}
@@ -278,6 +264,7 @@ private IReadOnlyList<List<TVertex>> TopologicalSortCore(
278264
if (predecessorCounts[candidateVertex] == 0)
279265
{
280266
currentRootsQueue.Add(candidateVertex);
267+
CheckBatchingBoundary(candidateVertex);
281268
broken = true;
282269
}
283270

@@ -334,6 +321,24 @@ private IReadOnlyList<List<TVertex>> TopologicalSortCore(
334321
}
335322

336323
return result;
324+
325+
// Detect batch boundary (if batching is enabled).
326+
// If the successor has any predecessor where the edge requires a batching boundary, and that predecessor is
327+
// already in the current batch, then the next batch will have to be executed in a separate batch.
328+
// TODO: Optimization: Instead of currentBatchSet, store a batch counter on each vertex, and check if later
329+
// vertexes have a boundary-requiring dependency on a vertex with the same batch counter.
330+
void CheckBatchingBoundary(TVertex vertex)
331+
{
332+
if (withBatching
333+
&& _predecessorMap[vertex].Any(
334+
kv =>
335+
(kv.Value is Edge { RequiresBatchingBoundary: true }
336+
|| kv.Value is IEnumerable<Edge> edges && edges.Any(e => e.RequiresBatchingBoundary))
337+
&& currentBatchSet.Contains(kv.Key)))
338+
{
339+
batchBoundaryRequired = true;
340+
}
341+
}
337342
}
338343

339344
private void ThrowCycle(

‎test/EFCore.Relational.Specification.Tests/Update/NonSharedModelUpdatesTestBase.cs‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
// ReSharper disable MethodHasAsyncOverload
5+
namespace Microsoft.EntityFrameworkCore.Update;
6+
7+
#nullable enable
8+
9+
public abstract class NonSharedModelUpdatesTestBase : NonSharedModelTestBase
10+
{
11+
protected override string StoreName
12+
=> "NonSharedModelUpdatesTestBase";
13+
14+
[ConditionalTheory] // Issue #29356
15+
[InlineData(false)]
16+
[InlineData(true)]
17+
public virtual async Task Principal_and_dependent_roundtrips_with_cycle_breaking(bool async)
18+
{
19+
var contextFactory = await InitializeAsync<DbContext>(
20+
onModelCreating: mb =>
21+
{
22+
mb.Entity<Author>(
23+
b =>
24+
{
25+
b.HasOne(a => a.AuthorsClub)
26+
.WithMany()
27+
.HasForeignKey(a => a.AuthorsClubId);
28+
});
29+
30+
mb.Entity<Book>(
31+
b =>
32+
{
33+
b.HasOne(book => book.Author)
34+
.WithMany()
35+
.HasForeignKey(book => book.AuthorId);
36+
});
37+
});
38+
39+
await ExecuteWithStrategyInTransactionAsync(
40+
contextFactory,
41+
async context =>
42+
{
43+
context.Add(
44+
new Book
45+
{
46+
Author = new Author
47+
{
48+
Name = "Alice",
49+
AuthorsClub = new AuthorsClub
50+
{
51+
Name = "AC South"
52+
}
53+
}
54+
});
55+
56+
await context.SaveChangesAsync();
57+
},
58+
async context =>
59+
{
60+
AuthorsClub authorsClubNorth = new()
61+
{
62+
Name = "AC North"
63+
};
64+
Author authorOfTheYear2023 = new()
65+
{
66+
Name = "Author of the year 2023",
67+
AuthorsClub = authorsClubNorth
68+
};
69+
context.Add(authorsClubNorth);
70+
context.Add(authorOfTheYear2023);
71+
72+
var book = await context
73+
.Set<Book>()
74+
.Include(b => b.Author)
75+
.SingleAsync();
76+
var authorOfTheYear2022 = book.Author!;
77+
book.Author = authorOfTheYear2023;
78+
79+
context.Remove(authorOfTheYear2022);
80+
81+
if (async)
82+
{
83+
await context.SaveChangesAsync();
84+
}
85+
else
86+
{
87+
context.SaveChanges();
88+
}
89+
});
90+
}
91+
92+
private class AuthorsClub
93+
{
94+
public int Id { get; set; }
95+
public string? Name { get; set; }
96+
}
97+
98+
private class Author
99+
{
100+
public int Id { get; set; }
101+
public string? Name { get; set; }
102+
public int AuthorsClubId { get; set; }
103+
public AuthorsClub? AuthorsClub { get; set; }
104+
}
105+
106+
private class Book
107+
{
108+
public int Id { get; set; }
109+
public string? Title { get; set; }
110+
public int AuthorId { get; set; }
111+
public Author? Author { get; set; }
112+
}
113+
114+
protected virtual void ExecuteWithStrategyInTransaction(
115+
ContextFactory<DbContext> contextFactory,
116+
Action<DbContext> testOperation,
117+
Action<DbContext>? nestedTestOperation1 = null,
118+
Action<DbContext>? nestedTestOperation2 = null,
119+
Action<DbContext>? nestedTestOperation3 = null)
120+
=> TestHelpers.ExecuteWithStrategyInTransaction(
121+
contextFactory.CreateContext, UseTransaction, testOperation, nestedTestOperation1, nestedTestOperation2, nestedTestOperation3);
122+
123+
protected virtual Task ExecuteWithStrategyInTransactionAsync(
124+
ContextFactory<DbContext> contextFactory,
125+
Func<DbContext, Task> testOperation,
126+
Func<DbContext, Task>? nestedTestOperation1 = null,
127+
Func<DbContext, Task>? nestedTestOperation2 = null,
128+
Func<DbContext, Task>? nestedTestOperation3 = null)
129+
=> TestHelpers.ExecuteWithStrategyInTransactionAsync(
130+
contextFactory.CreateContext, UseTransaction, testOperation, nestedTestOperation1, nestedTestOperation2, nestedTestOperation3);
131+
132+
public void UseTransaction(DatabaseFacade facade, IDbContextTransaction transaction)
133+
=> facade.UseTransaction(transaction.GetDbTransaction());
134+
135+
protected TestSqlLoggerFactory TestSqlLoggerFactory
136+
=> (TestSqlLoggerFactory)ListLoggerFactory;
137+
}

‎test/EFCore.SqlServer.FunctionalTests/Update/NonSharedModelUpdatesSqlServerTest.cs‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
namespace Microsoft.EntityFrameworkCore.Update;
5+
6+
public class NonSharedModelUpdatesSqlServerTest : NonSharedModelUpdatesTestBase
7+
{
8+
public override async Task Principal_and_dependent_roundtrips_with_cycle_breaking(bool async)
9+
{
10+
await base.Principal_and_dependent_roundtrips_with_cycle_breaking(async);
11+
12+
AssertSql(
13+
"""
14+
@p0='AC South' (Size = 4000)
15+
16+
SET IMPLICIT_TRANSACTIONS OFF;
17+
SET NOCOUNT ON;
18+
INSERT INTO [AuthorsClub] ([Name])
19+
OUTPUT INSERTED.[Id]
20+
VALUES (@p0);
21+
""",
22+
//
23+
"""
24+
@p1='1'
25+
@p2='Alice' (Size = 4000)
26+
27+
SET IMPLICIT_TRANSACTIONS OFF;
28+
SET NOCOUNT ON;
29+
INSERT INTO [Author] ([AuthorsClubId], [Name])
30+
OUTPUT INSERTED.[Id]
31+
VALUES (@p1, @p2);
32+
""",
33+
//
34+
"""
35+
@p3='1'
36+
@p4=NULL (Size = 4000)
37+
38+
SET IMPLICIT_TRANSACTIONS OFF;
39+
SET NOCOUNT ON;
40+
INSERT INTO [Book] ([AuthorId], [Title])
41+
OUTPUT INSERTED.[Id]
42+
VALUES (@p3, @p4);
43+
""",
44+
//
45+
"""
46+
SELECT TOP(2) [b].[Id], [b].[AuthorId], [b].[Title], [a].[Id], [a].[AuthorsClubId], [a].[Name]
47+
FROM [Book] AS [b]
48+
INNER JOIN [Author] AS [a] ON [b].[AuthorId] = [a].[Id]
49+
""",
50+
//
51+
"""
52+
@p0='AC North' (Size = 4000)
53+
54+
SET IMPLICIT_TRANSACTIONS OFF;
55+
SET NOCOUNT ON;
56+
INSERT INTO [AuthorsClub] ([Name])
57+
OUTPUT INSERTED.[Id]
58+
VALUES (@p0);
59+
""",
60+
//
61+
"""
62+
@p1='2'
63+
@p2='Author of the year 2023' (Size = 4000)
64+
65+
SET IMPLICIT_TRANSACTIONS OFF;
66+
SET NOCOUNT ON;
67+
INSERT INTO [Author] ([AuthorsClubId], [Name])
68+
OUTPUT INSERTED.[Id]
69+
VALUES (@p1, @p2);
70+
""",
71+
//
72+
"""
73+
@p4='1'
74+
@p3='2'
75+
@p5='1'
76+
77+
SET NOCOUNT ON;
78+
UPDATE [Book] SET [AuthorId] = @p3
79+
OUTPUT 1
80+
WHERE [Id] = @p4;
81+
DELETE FROM [Author]
82+
OUTPUT 1
83+
WHERE [Id] = @p5;
84+
""");
85+
}
86+
87+
private void AssertSql(params string[] expected)
88+
=> TestSqlLoggerFactory.AssertBaseline(expected);
89+
90+
protected override ITestStoreFactory TestStoreFactory => SqlServerTestStoreFactory.Instance;
91+
}

‎test/EFCore.Sqlite.FunctionalTests/Update/NonSharedModelUpdatesSqliteTest.cs‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
namespace Microsoft.EntityFrameworkCore.Update;
5+
6+
public class NonSharedModelUpdatesSqlServerTest : NonSharedModelUpdatesTestBase
7+
{
8+
public override async Task Principal_and_dependent_roundtrips_with_cycle_breaking(bool async)
9+
{
10+
await base.Principal_and_dependent_roundtrips_with_cycle_breaking(async);
11+
12+
AssertSql(
13+
"""
14+
@p0='AC South' (Size = 8)
15+
16+
INSERT INTO "AuthorsClub" ("Name")
17+
VALUES (@p0)
18+
RETURNING "Id";
19+
""",
20+
//
21+
"""
22+
@p1='1'
23+
@p2='Alice' (Size = 5)
24+
25+
INSERT INTO "Author" ("AuthorsClubId", "Name")
26+
VALUES (@p1, @p2)
27+
RETURNING "Id";
28+
""",
29+
//
30+
"""
31+
@p3='1'
32+
@p4=NULL
33+
34+
INSERT INTO "Book" ("AuthorId", "Title")
35+
VALUES (@p3, @p4)
36+
RETURNING "Id";
37+
""",
38+
//
39+
"""
40+
SELECT "b"."Id", "b"."AuthorId", "b"."Title", "a"."Id", "a"."AuthorsClubId", "a"."Name"
41+
FROM "Book" AS "b"
42+
INNER JOIN "Author" AS "a" ON "b"."AuthorId" = "a"."Id"
43+
LIMIT 2
44+
""",
45+
//
46+
"""
47+
@p0='AC North' (Size = 8)
48+
49+
INSERT INTO "AuthorsClub" ("Name")
50+
VALUES (@p0)
51+
RETURNING "Id";
52+
""",
53+
//
54+
"""
55+
@p1='2'
56+
@p2='Author of the year 2023' (Size = 23)
57+
58+
INSERT INTO "Author" ("AuthorsClubId", "Name")
59+
VALUES (@p1, @p2)
60+
RETURNING "Id";
61+
""",
62+
//
63+
"""
64+
@p4='1'
65+
@p3='2'
66+
67+
UPDATE "Book" SET "AuthorId" = @p3
68+
WHERE "Id" = @p4
69+
RETURNING 1;
70+
""",
71+
//
72+
"""
73+
@p0='1'
74+
75+
DELETE FROM "Author"
76+
WHERE "Id" = @p0
77+
RETURNING 1;
78+
""");
79+
}
80+
81+
private void AssertSql(params string[] expected)
82+
=> TestSqlLoggerFactory.AssertBaseline(expected);
83+
84+
protected override ITestStoreFactory TestStoreFactory => SqliteTestStoreFactory.Instance;
85+
}

0 commit comments

Comments
 (0)