Handle out-of-order split include rows during concurrent inserts by AndriySvyryd · Pull Request #38676 · dotnet/efcore

AndriySvyryd · 2026-07-20T07:45:49Z

Problem

When a query uses AsSplitQuery() with collection Includes, EF executes one SQL statement per collection level. Between those statements the database can change. Issue #33826 showed that a completely unrelated concurrent insert could make EF silently drop the child collections of untouched parents:

var blogs = await db.Blogs
    .Include(b => b.Posts)
    .AsSplitQuery()
    .OrderByDescending(b => b.Id)
    .ToListAsync();
// Blog 1's posts come back empty — even though Blog 1 and its posts were never modified —
// simply because an unrelated Blog 2 was inserted between the two split queries.

Root cause

The parent query and each child query both carry the same ORDER BY <parent key>, so a parent's child rows arrive as a contiguous, correctly-ordered block. The old stitching loop streamed child rows and treated the first row whose parent key didn't match the current parent as the boundary of the next parent — then stopped. When a concurrent insert put an unrelated child row in the stream (e.g. an out-of-order/orphan parent key), that row was misread as "the current parent has no more children," so the remaining real children were abandoned and the whole collection was lost. The bug is purely in client-side correlation — the correct rows were fetched from the database and then thrown away.

Why this approach

I considered several designs; the key constraints (from reviewing the earlier attempt in this PR and the issue discussion) were:

No client-side ordering comparison. The obvious fix — "if the non-matching row sorts after the current parent it's the next parent, otherwise skip it as an orphan" — requires comparing key magnitudes on the client. That's wrong because .NET ordering doesn't always match the database's ordering (GUIDs on SQL Server, culture/collation-sensitive strings, etc.). Any magnitude comparison risks new, subtler correctness bugs. We must rely only on key equality, which EF already trusts.
Avoid buffering the whole result set. The previous iteration of this PR (Handle out-of-order split-include rows to prevent child collection loss under concurrent inserts #38590) fell back to buffering every child row into a dictionary whenever ordering wasn't "trusted," which defeats the streaming purpose of split queries and regressed memory for ordinary queries.
Fail loudly rather than silently corrupt. Where a scenario genuinely can't be disambiguated, throwing is far better than returning wrong data.

The chosen design satisfies all three by giving the materializer the one piece of information that removes the ambiguity: how many children each parent is supposed to have.

The fix: project a per-parent child count, correlate by count + equality

Producer. For every split collection, the outer (parent) query now projects a correlated COUNT(*) of that collection as an extra scalar column:

SELECT [c].[CustomerID], /* … */, (
    SELECT COUNT(*)
    FROM [Orders] AS [o]
    WHERE [c].[CustomerID] = [o].[CustomerID])
FROM [Customers] AS [c]
ORDER BY [c].[CustomerID]

The count is derived from a clone of the collection subquery taken before the split decorrelates it, so it automatically respects the collection's own predicate/filters, and Take/Skip/Distinct/GroupBy (via pushdown) so it equals the number of rows the child query actually yields.

Consumer. The materializer reads child rows as contiguous blocks (a maximal run sharing one parent key) and correlates each parent using key equality only, driven by the expected count:

The parent's block is normally at the head of the stream → assign it. (A count mismatch here — some of that parent's own children concurrently added/removed — is benign; we assign the actual rows.)
If the head block belongs to a different parent, exactly one concurrent "block-position" anomaly can be reconciled with a single buffered block:
- Peek the key of the next block. If it's this parent's, the buffered block was an orphan (concurrent unrelated insert) → discard it and assign this parent's real block.
- Otherwise this parent was concurrently made childless → keep the buffered block for a later parent to claim.
A parent with count 0 is skipped without touching the reader, so genuinely childless parents never consume rows.

At most one block is ever held in memory, and no ordering magnitudes are compared. When a second, colliding concurrent change would be needed to explain the stream, EF throws the new DbQueryConcurrencyException instead of guessing.

Best-effort detection, by design

The guarantee is deliberately scoped: correct for any single concurrent change (which may touch many rows of one parent), and never silently wrong. Multiple concurrent changes either still produce correct results (e.g. rows added/removed within several parents' own blocks — pure count changes that keep each block in place) or raise DbQueryConcurrencyException. Callers handle it the same way as optimistic-concurrency conflicts on save:

The results of a split query could not be correlated because the data was modified concurrently while the query was executing. Re-execute the query, or execute it within a serializable or snapshot transaction to prevent concurrent modifications.

A dedicated exception type (rather than a bare InvalidOperationException) is introduced precisely so this is catchable and retryable.

Changes

New public API: DbQueryConcurrencyException (relational).
Producer: SelectExpression projects a correlated COUNT(*) per split collection; carried on RelationalSplitCollectionShaperExpression.ChildCount and SplitCollectionInfo.
Materializer: rewrote PopulateSplitIncludeCollection/PopulateSplitCollection (sync + async) to the block/count algorithm; SplitQueryCollectionContext.ChildCount and a bounded one-block buffer (BufferedBlock*, AnomalyUsed) on SplitQueryDataReaderContext.
End-of-stream check: SplitQueryResultCoordinator.VerifyNoLeftoverBufferedBlocks() (called when the outer enumerator completes) throws if a buffered "childless" block was never claimed.
Tests: #33826 regression tests in AdHocQuerySplittingQueryTestBase covering a concurrent unrelated insert and a concurrent parent-made-childless, sync + async (SQL Server; no-ops on SQLite, whose per-connection snapshot can't reproduce cross-statement visibility).

Trade-offs

Every split-collection query gains one correlated COUNT(*) column on the parent query (extra aggregate cost on wide parent sets), in exchange for correctness under concurrency without buffering. Split-query SQL baselines are updated accordingly.
More than one simultaneous structural change can surface as DbQueryConcurrencyException rather than a (possibly wrong) result — the same failure model as DbUpdateConcurrencyException, and strictly better than the previous silent data loss.

Copilot

Pull request overview

This PR fixes a split-query (AsSplitQuery) materialization bug where concurrent, unrelated modifications can cause EF Core to silently drop included child collections by making split-include child rows appear out-of-order. The change adds a per-parent child-count projection to the outer query and updates split-collection stitching to tolerate a single out-of-order “block” (or throw a dedicated exception when correlation becomes ambiguous).

Changes:

Project a correlated COUNT(*) per split collection onto the parent query (plumbed via RelationalSplitCollectionShaperExpression.ChildCount).
Rewrite split-include/split-collection materialization to use a one-block buffer and throw DbQueryConcurrencyException when correlation cannot be safely determined.
Add DbQueryConcurrencyException public API + message resource, and add regression coverage for #33826 (SQL Server; SQLite no-op).

Reviewed changes

Copilot reviewed 38 out of 40 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test/EFCore.SqlServer.FunctionalTests/Query/TemporalOwnedQuerySqlServerTest.cs	Updates SQL baselines to include correlated `COUNT(*)` columns.
test/EFCore.SqlServer.FunctionalTests/Query/OwnedQuerySqlServerTest.cs	Updates SQL baselines to include correlated `COUNT(*)` columns.
test/EFCore.SqlServer.FunctionalTests/Query/OwnedEntityQuerySqlServerTest.cs	Updates SQL baselines to include correlated `COUNT(*)` columns.
test/EFCore.SqlServer.FunctionalTests/Query/NorthwindQueryTaggingQuerySqlServerTest.cs	Updates SQL baselines to include correlated `COUNT(*)` columns.
test/EFCore.SqlServer.FunctionalTests/Query/NorthwindMiscellaneousQuerySqlServerTest.cs	Updates SQL baselines to include correlated `COUNT(*)` columns.
test/EFCore.SqlServer.FunctionalTests/Query/InheritanceRelationshipsQuerySqlServerTest.cs	Updates SQL baselines to include correlated `COUNT(*)` columns.
test/EFCore.SqlServer.FunctionalTests/Query/Inheritance/TPTRelationshipsQuerySqlServerTest.cs	Updates SQL baselines to include correlated `COUNT(*)` columns.
test/EFCore.SqlServer.FunctionalTests/Query/Inheritance/TPCRelationshipsQuerySqlServerTest.cs	Updates SQL baselines to include correlated `COUNT(*)` columns.
test/EFCore.SqlServer.FunctionalTests/Query/Inheritance/TPCManyToManyNoTrackingQuerySqlServerTest.cs	Updates SQL baselines to include correlated `COUNT(*)` columns.
test/EFCore.SqlServer.FunctionalTests/Query/FromSqlQuerySqlServerTest.cs	Updates SQL baselines to include correlated `COUNT(*)` columns.
test/EFCore.SqlServer.FunctionalTests/Query/CompositeKeysSplitQuerySqlServerTest.cs	Updates SQL baselines to include correlated `COUNT(*)` columns.
test/EFCore.SqlServer.FunctionalTests/Query/AdHocQuerySplittingQuerySqlServerTest.cs	Overrides new regression tests to skip nondeterministic SQL baselines.
test/EFCore.SqlServer.FunctionalTests/Query/AdHocNavigationsQuerySqlServerTest.cs	Updates SQL baselines to include correlated `COUNT(*)` columns.
test/EFCore.SqlServer.FunctionalTests/Query/AdHocAdvancedMappingsQuerySqlServerTest.cs	Updates SQL baselines to include correlated `COUNT(*)` columns.
test/EFCore.Sqlite.FunctionalTests/Query/AdHocQuerySplittingQuerySqliteTest.cs	No-ops the new concurrency regression tests (SQLite snapshot behavior).
test/EFCore.Relational.Specification.Tests/Query/AdHocQuerySplittingQueryTestBase.cs	Adds #33826 regression tests for concurrent insert/child-deletion scenarios.
src/EFCore.Relational/Query/SqlTreePruner.cs	Ensures `ChildCount` is preserved during pruning.
src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs	Plumbs `ChildCount` through split-collection shaper construction.
src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs	Projects correlated `COUNT(*)` for split collections; adds precompilation fallback.
src/EFCore.Relational/Query/RelationalSplitCollectionShaperExpression.cs	Adds `ChildCount` to the split-collection shaper expression.
src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs	Compiles and passes `childCount` lambda into split include/collection init.
src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.ClientMethods.cs	Implements block/buffer correlation logic and throws `DbQueryConcurrencyException` on ambiguity.
src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessor.cs	Passes `IsPrecompiling` into projection application.
src/EFCore.Relational/Query/Internal/SplitQueryResultCoordinator.cs	Adds end-of-stream verification for leftover buffered blocks.
src/EFCore.Relational/Query/Internal/SplitQueryingEnumerable.cs	Calls `VerifyNoLeftoverBufferedBlocks()` when outer enumeration completes.
src/EFCore.Relational/Query/Internal/SplitQueryDataReaderContext.cs	Stores one buffered block + “anomaly used” marker.
src/EFCore.Relational/Query/Internal/SplitQueryCollectionContext.cs	Stores `ChildCount` per parent/collection context.
src/EFCore.Relational/Query/Internal/SelectExpressionProjectionApplyingExpressionVisitor.cs	Threads `isPrecompiling` into `SelectExpression.ApplyProjection`.
src/EFCore.Relational/Properties/RelationalStrings.resx	Adds `SplitQueryConcurrentModification` resource text.
src/EFCore.Relational/Properties/RelationalStrings.Designer.cs	Adds strongly-typed accessor for the new resource string.
src/EFCore.Relational/EFCore.Relational.baseline.json	Updates public API baseline (new exception, new shaper signature, new ApplyProjection overload).
src/EFCore.Relational/DbQueryConcurrencyException.cs	Introduces new public exception type for split-query correlation failures.

Files not reviewed (1)

src/EFCore.Relational/Properties/RelationalStrings.Designer.cs: Generated file

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 42 out of 44 changed files in this pull request and generated no new comments.

Files not reviewed (1)

src/EFCore.Relational/Properties/RelationalStrings.Designer.cs: Generated file

Comments suppressed due to low confidence (1)

src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs:1269

The correlated COUNT() subquery is created as a SqlFunctionExpression typed as int and then only type-mapped. For providers where COUNT() returns a larger integer type at the ADO.NET level (e.g. bigint/Int64), this can cause an invalid cast when EF reads the projection as int. Consider emitting an explicit SQL CAST by typing COUNT() as long and then converting to int via SqlExpressionFactory.Convert(), so the SQL generator produces CAST(COUNT() AS int) and the reader shape matches the declared type.

                                childCountSelectExpression.ClearOrdering();
                                var countExpression = new SqlFunctionExpression(
                                    "COUNT",
                                    new SqlExpression[] { new SqlFragmentExpression("*") },
                                    nullable: false,
                                    argumentsPropagateNullability: new[] { false },
                                    typeof(int),
                                    typeMapping: null);

AndriySvyryd · 2026-07-20T23:09:28Z

+        ISqlExpressionFactory? sqlExpressionFactory,
+        IAggregateMethodCallTranslatorProvider? aggregateMethodCallTranslatorProvider,
+        QueryCompilationContext? queryCompilationContext)


I don't love this, but the alternative is to assume "COUNT" works for all relational providers

Even if we would assume COUNT works for all relational providers, the type mapping is definitely going to differ.

We would have to relocate the COUNT construction earlier in translation phase. But that's bigger change than this.

Copilot

Pull request overview

Copilot reviewed 44 out of 46 changed files in this pull request and generated 2 comments.

Files not reviewed (1)

src/EFCore.Relational/Properties/RelationalStrings.Designer.cs: Generated file

cincuranet · 2026-07-21T09:13:17Z

+
+            // The child count could not be projected for this collection's shape (grouping-element projection, etc.).
+            // Fall back to streaming correlation: read this parent's contiguous block and stop at the first non-matching row.
+            if (splitQueryCollectionContext.ChildCount < 0)


Given we hardcode -1 it would make sense to me to compare specifically with -1 here.

cincuranet · 2026-07-21T09:15:13Z

+
+            // The child count could not be projected for this collection's shape (grouping-element projection, etc.).
+            // Fall back to streaming correlation: read this parent's contiguous block and stop at the first non-matching row.
+            if (splitQueryCollectionContext.ChildCount < 0)


Same -1 comment as above.

cincuranet · 2026-07-21T09:15:41Z

+
+            // The child count could not be projected for this collection's shape (grouping-element projection, etc.).
+            // Fall back to streaming correlation: read this parent's contiguous block and stop at the first non-matching row.
+            if (splitQueryCollectionContext.ChildCount < 0)


Same -1 comment as above.

cincuranet · 2026-07-21T09:17:44Z

-                if (relatedDataLoaders != null)
+            // The child count could not be projected for this collection's shape (grouping-element projection, etc.).
+            // Fall back to streaming correlation: read this parent's contiguous block and stop at the first non-matching row.
+            if (splitQueryCollectionContext.ChildCount < 0)


Same -1 comment as above.

cincuranet · 2026-07-21T11:04:36Z

+        ISqlExpressionFactory? sqlExpressionFactory,
+        IAggregateMethodCallTranslatorProvider? aggregateMethodCallTranslatorProvider,
+        QueryCompilationContext? queryCompilationContext)


Even if we would assume COUNT works for all relational providers, the type mapping is definitely going to differ.

We would have to relocate the COUNT construction earlier in translation phase. But that's bigger change than this.

cincuranet · 2026-07-21T11:16:37Z

Every split-collection now carries a correlated COUNT(*) per collection. That's not cheap. Quite the opposite. Especially on wide parent sets or unindexed FKs. And all of this even for cases with zero concurrency - which I would say is majority of cases. Together with works for "any single concurrent change" I'm not convinced this a good change.

What makes more sense to me is doing "detect and throw" only. The cascading silent loss can be detected purely client-side (buffered non-matching row still unclaimed at end of the parent stream) and turned into DbQueryConcurrencyException with no COUNT(*) added. That kills the silent corruption at zero steady-state cost, and tells users exactly what the docs already say: re-run under serializable/snapshot.

AndriySvyryd requested a review from Copilot July 20, 2026 07:45

AndriySvyryd requested a review from a team as a code owner July 20, 2026 07:45

Copilot started reviewing on behalf of AndriySvyryd July 20, 2026 07:46 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

Comment thread src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs Outdated

Handle out-of-order split include rows during concurrent inserts

b7868e4

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>

Copilot AI review requested due to automatic review settings July 20, 2026 18:27

AndriySvyryd force-pushed the Issue33826 branch from 344040c to b7868e4 Compare July 20, 2026 18:27

Copilot started reviewing on behalf of AndriySvyryd July 20, 2026 18:28 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

Translate Count

ed7d083

Copilot AI review requested due to automatic review settings July 20, 2026 23:07

Copilot started reviewing on behalf of AndriySvyryd July 20, 2026 23:08 View session

AndriySvyryd commented Jul 20, 2026

View reviewed changes

Copilot AI reviewed Jul 20, 2026

View reviewed changes

Comment thread src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs

Comment thread test/EFCore.Relational.Specification.Tests/Query/AdHocQuerySplittingQueryTestBase.cs

AndriySvyryd requested a review from cincuranet July 20, 2026 23:30

AndriySvyryd added the tell-mode label Jul 21, 2026

cincuranet reviewed Jul 21, 2026

View reviewed changes

Uh oh!

Conversation

AndriySvyryd commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Root cause

Why this approach

The fix: project a per-parent child count, correlate by count + equality

Best-effort detection, by design

Changes

Trade-offs

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

AndriySvyryd Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

cincuranet Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

cincuranet Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

cincuranet Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

cincuranet Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

cincuranet Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

cincuranet Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

cincuranet commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AndriySvyryd commented Jul 20, 2026 •

edited

Loading