[benedict-odonovan]

Fixes #28002
Part of #36510

• I've read the guidelines for contributing and seen the walkthrough
• I've posted a comment on an issue with a detailed description of how I am planning to contribute and got approval from a member of the team
• The code builds and tests pass locally (also verified by our automated build checks)
• Commit messages follow this format
• Tests for the changes have been added (for bug fixes / features)
• Code follows the same patterns and style as existing code in this repo

Summary

Every SelectMany over a grouping currently fails in navigation expansion, before any provider sees it, including the plain SelectMany(g => g). This translates the shapes that map directly to SQL, the main one being top-N per group:

context.Orders
    .GroupBy(o => o.CustomerID)
    .SelectMany(g => g.OrderBy(o => o.OrderID).Skip(skip).Take(take))

SELECT [o0].[OrderID], [o0].[CustomerID], [o0].[EmployeeID], [o0].[OrderDate]
FROM (
    SELECT [o].[OrderID], [o].[CustomerID], [o].[EmployeeID], [o].[OrderDate], ROW_NUMBER() OVER(PARTITION BY [o].[CustomerID] ORDER BY [o].[OrderID]) AS [row]
    FROM [Orders] AS [o]
) AS [o0]
WHERE [o0].[row] > @skip AND ([o0].[row] - @skip <= @take OR [o0].[row] <= @take)

The elements of all groups are the source rows, so the GROUP BY isn't needed. SelectMany(g => g) becomes a plain read of the source, and a Skip or Take over the grouping filters on ROW_NUMBER() partitioned by the grouping key, the same shape as the group element reads from #39173.

The bounds subtract from ROW_NUMBER(), which is a bigint, rather than adding the counts, so Take(int.MaxValue) doesn't overflow. The second comparison keeps a negative Skip skipping nothing, as in LINQ. With constants the bounds are folded instead, e.g. Skip(1).Take(2) gives [row] > 1 AND [row] <= 3.

This is also the first step towards #36510. What's left there is translating aggregates over g in the element selector as window functions, using the support added in #39240.

The change

• Navigation expansion: SelectMany accepts a grouping as its collection and always produces the overload without a result selector. A result selector can only see the grouping and an element, so it's composed over the collection as a Select, and nothing downstream has to carry a grouping.
• Members of a projected element: navigation expansion now follows a member of an anonymous type, or of a DTO initialized through its members, that has become the current element, such as x.o after SelectMany(g => g, (g, o) => new { g.Key, o }). Without this, x.o.Customer.City read null and x.o.OrderDetails came back empty, with no join and no warning. SnapshotExpression also keeps a MemberInitExpression instead of reducing it to a default, and the include expansion rebuilds such a DTO so that Include applies to the entities in it, as it already did for anonymous types.
• This also fixes existing queries outside GroupBy. The same silent nulls happen on main after Distinct, Union, Concat, Except, Join and a plain SelectMany, e.g. Select(o => new { o }).Distinct().Select(x => x.o.Customer.City) returns null for every row. These now get a LEFT JOIN. New tests in NorthwindSetOperationsQueryRelationalTestBase cover Distinct and Union with anonymous and DTO members.
• Set operations: since the result's members can now be followed, an entity in a set operation's result is marked optional when it's optional in either operand. Otherwise a Concat whose second operand has a null entity from a left join read a required navigation with an INNER JOIN and dropped those rows. This also fixes the same query with the bare entity, which drops the rows on main. DTO operands are now checked for compatible includes and query roots, as anonymous ones are; when their members differ, the DTO stays opaque as before.
• Relational translation: TranslateSelectManyOverGrouping matches g [OrderBy/ThenBy] [Skip and/or Take] [Select]. It keeps a Take(5).Skip(2) and a Skip(2).Take(5) distinct: the first ends at the group's 5th row, the second at its 7th. The counts must be constants or parameters, so g.Take(g.Count()) fails translation rather than in the database.
• SelectExpression.ApplyRowsPerGroup: removes the grouping, restoring the pre-grouping identifier, and numbers and filters the rows when there's a limit. It shares RemoveGrouping with ApplyFirstRowPerGroup.
• InMemory: declines with a translation failure. Without this, the new shape reached its correlated path and threw an internal error about ShapedQueryExpression.VisitChildren.

Since the result is rows rather than groups, anything composed afterwards works normally, including Where, OrderBy, Take, aggregates, navigations and Include. This avoids the root-operator restriction on the lifted grouping projections.

What stays out

These keep failing translation, each pinned by a test:

• g.Where(...) inside the selector
• an OrderBy over a Take inside the selector
• a collection in the element selector
• a filter or a Take on the groups before the SelectMany
• a Skip or Take whose count depends on the group, such as g.Key or g.Count()
• an ordering on an aggregate of the group, such as g.OrderBy(o => g.Count())
• SelectMany over a grouping in a projected subquery, such as Select(c => c.Orders.GroupBy(...).SelectMany(g => g).ToList()), or in a predicate subquery, such as Where(o => orders.GroupBy(...).SelectMany(g => g.Take(1)).Select(x => x.OrderID).Contains(o.OrderID))

A SelectMany over a grouping in the collection selector of a correlated SelectMany does translate, with CROSS APPLY, and is tested. SQLite asserts the APPLY error.

The rewrite suggested earlier on #28002, .Select(g => g.Take(n)).SelectMany(x => x), doesn't reach this translation and isn't covered here.

Worth reviewing closely

Positional records and constructor DTOs aren't followed. For new RecO(o, k), nothing links x.O to the constructor argument, so x.O.Customer.City still reads null, as it does on main for Select(o => new RecO(o, 1)).Select(x => x.O.Customer.City). This is an existing limitation, not specific to this PR, but it now also applies to the new SelectMany shapes. Making it fail translation instead would change existing queries, so I've left it.

The _subquery check in TranslateSelectManyOverGrouping is conservative. The subquery tests fail translation even without it, so it's kept as a guard rather than pinned by a test.

Ordering within groups. When the groups are ordered by their key, the element ordering is appended to it, so GroupBy(...).OrderBy(g => g.Key).SelectMany(g => g.OrderByDescending(o => o.OrderID)) gives ORDER BY [o].[CustomerID], [o].[OrderID] DESC, as in LINQ to Objects. When the groups aren't ordered, the element ordering only decides which rows a limit keeps, and is otherwise dropped, as for SelectMany over any other subquery. Ordering the result by the grouping terms in that case would also match LINQ, but it would add an ORDER BY to every such query, so this is a choice I'd like checked.

An ordering before the GroupBy doesn't choose the rows. OrderByDescending(o => o.OrderID).GroupBy(...).SelectMany(g => g.Take(1)) numbers each group by its identifier and logs the row-limiting-without-OrderBy warning, as g.First() already does. The ordering has to go inside the group.

g.Key under a case-insensitive collation is read from each row, so one group can return ABC and abc as its key where GROUP BY would report one value.

The navigation expansion change is in core. It runs for every provider. Cosmos doesn't translate GroupBy, so it should still fail there, and I haven't run the Cosmos tests. A third-party non-relational provider that translates GroupBy would now see this shape in TranslateSelectMany.

Testing

The tests for the new behavior are in NorthwindGroupByQueryRelationalTestBase, with baselines on SQL Server. The four existing tests which pinned the failure moved there from NorthwindGroupByQueryTestBase, and ProjectedType became protected for that. Select_GroupBy_SelectMany also translates now, so its comment about entity equality is gone. OwnedQueryRelationalTestBase gains a test with owned navigations. NorthwindGroupByQueryInMemoryTest gains one pinning the translation failure.

The new tests cover the review cases: navigations through a projected element, Take(0), Take(a).Skip(b) past the end, int.MaxValue and negative counts as constants and parameters, two ThenBys, ordered groups with and without a limit, counts and orderings that depend on the group, and a correlated SelectMany.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.