[xperiandri]

Stacked on #20894

A groupBy or groupValBy over tuples or records, and any groupBy after a join or a second for, no longer rebuilds every group in the middle of the query, which EF Core failed on with "Value cannot be null. (Parameter 'key')".

Details

• The yield of the element itself that the compiler adds after each join, for and custom operator was translated as a projection rebuilding the element. The elements now pass through unchanged.
• A read of a group's key, and Count, Any, All, Sum, Average, Min, Max, Select or SelectMany over a group, reads the provider's grouping. EF Core turns these into COUNT, SUM, AVG and MAX.
• Only the translation's own rebuilding is moved, and only into a lambda that reads the members of its element. A Select the query itself wrote stays where it is.
• A group used as a value is built with new Grouping(key, elements) at each place where the query mentions it, so groupValBy (c.Name, c.Age) c.City into g; let n = g.Count(); select (g, n), also after a join, after two fors and with a record as the element, runs on EF Core and linq2db as on Check query trees for LINQ providers and translate struct tuples #20894. Two mentions of a group are two objects: FSharp.Core 10.1 built the group once, in the Select right after the GroupBy, so over an in-memory IQueryable a query that compares a group with itself now gives other rows. PENDING_BREAKING_CHANGES.md lists them.
• Where the final conversion reads the elements of a join, its parameter is named tupledArg, the name the compiler gave the lambda that read them before. SQLProvider recognizes joined elements by that name.

🤖 Generated with Claude Code

[github-actions]

✅ No release notes required

[xperiandri]

Covered cases

EF Core SQLite translates 102 of 145 scenarios, 93 without this change. Now translated: groupValBy a tuple of values, alone and with a sum, an average, a max, a select-then-sum and a count with a predicate over the group; groupBy after a join with the key and a count, and with a sum over the joined side; groupBy after two fors.

SQLProvider 1.5.27 on SQLite, 301 queries: none that works today is lost, and count, exists, sumBy, minBy, maxBy and averageBy after a join now run as one SQL aggregate.

[Copilot]

🟡 Changes recommended

Projection fusion can break nested tuple and record reference-identity semantics, while gaps in shape validation allow invalid expression trees to pass tests.

4 open issues

• Preserve Select when rebuilt values require reference identity · New
• Validate unnamed-member reads throughout the expression tree · New
• Preserve unnamed-member state across identity Select · New
• Document linq2db breakage for fused tuple group values · New

What changed in this PR

Updates FSharp.Core query translation so grouping aggregates and joined elements remain in forms LINQ providers can translate. Builds on #20894’s struct-tuple support and provider expression-tree tests.

Changes:

• Removes redundant projections after joins and additional for clauses.
• Reads grouping keys and aggregates directly from provider groupings, while preserving group identity when used as values.
• Adds query-shape scenarios, updates expression baselines, and documents translation changes.

File	Description
tests/​fsharp/​core/​queriesOverIQueryable/​test.fsx	Updates join expression baselines.
tests/​FSharp.Core.UnitTests/​FSharp.Core/​Microsoft.FSharp.Linq/​QueryStructureScenarios.fs	Covers loops, subqueries, and left joins.
tests/​FSharp.Core.UnitTests/​FSharp.Core/​Microsoft.FSharp.Linq/​QueryShapeTests.fs	Runs parameterized query scenarios.
tests/​FSharp.Core.UnitTests/​FSharp.Core/​Microsoft.FSharp.Linq/​QueryScenarioDomain.fs	Supplies shared query entities and data.
tests/​FSharp.Core.UnitTests/​FSharp.Core/​Microsoft.FSharp.Linq/​QueryScenario.fs	Compares results and expected tree violations.
tests/​FSharp.Core.UnitTests/​FSharp.Core/​Microsoft.FSharp.Linq/​QueryProviderShape.fs	Captures provider expressions and checks their structure.
tests/​FSharp.Core.UnitTests/​FSharp.Core/​Microsoft.FSharp.Linq/​QueryProjectionScenarios.fs	Covers tuple, record, and entity projections.
tests/​FSharp.Core.UnitTests/​FSharp.Core/​Microsoft.FSharp.Linq/​QueryOperatorScenarios.fs	Covers filtering, ordering, joins, and aggregates.
tests/​FSharp.Core.UnitTests/​FSharp.Core/​Microsoft.FSharp.Linq/​QueryMethodSyntaxScenarios.fs	Covers direct LINQ method calls.
tests/​FSharp.Core.UnitTests/​FSharp.Core/​Microsoft.FSharp.Linq/​QueryKeyScenarios.fs	Covers composite keys, grouped aggregates, and identity.
tests/​FSharp.Core.UnitTests/​FSharp.Core.UnitTests.fsproj	Registers the scenario infrastructure and tests.
src/​FSharp.Core/​QueryExtensions.fs	Simplifies grouping consumers and caches rebuilt groups.
src/​FSharp.Core/​Query.fs	Preserves converted elements and controls final reconstruction.
src/​FSharp.Core/​Linq.fs	Reads struct-tuple elements through fields.
docs/​release-notes/​PENDING_BREAKING_CHANGES.md	Illustrates changed provider expression shapes.
docs/​release-notes/​.FSharp.Core/​11.0.200.md	Records struct-tuple and grouping translation fixes.

_{Review effort: Balanced}

[pcshrosbree]

On EF Core 10.0.14 with SQLite, grouping queries that failed on #20894 now translate and return the same rows as LINQ to Objects, but four that ran on #20894 now fail. Those four should keep working alongside the new ones; each returns a group of tuples or records alongside another value:

query { for c in db.Customers do
        groupValBy (c.Name, c.Age) c.City into g
        let n = g.Count()
        select (g, n) }

On #20894 this runs. Here it throws "Translation of 'Select' which contains grouping parameter without composition is not supported", because RebuiltGroupings.Of(source: _arg2, ..) passes EF the provider's grouping itself. The same happens after a join, after two fors, and with a record element. Each of these four queries rebuilds the group only once in its projection. Group identity can still be observed across operators and on repeated enumeration, so counting uses within one lambda is not enough to decide when Of is needed.

• The new Impact paragraph in PENDING_BREAKING_CHANGES covers linq2db. These four EF Core queries belong there too.
• This PR also fixes a nested find that throws Type mismatch when building 'args' on main: where (c.Orders.Count > 1); select (c.Name, query { for o in c.Orders do for o2 in c.Orders do find (o.Id < o2.Id) }). The existing find scenario passes without this PR's changes, so no test covers this fix.
• A scenario for a group beside a count would guard the case above.
• A nested query { for (_, a) in g do sumBy a } over a group of tuples still doesn't translate, while g.Sum(fun (_, a) -> a) does.

[T-Gro]

🤖🕵️ AI review - verify independently.

[xperiandri]

Groups used as values: keep RebuiltGroupings.Of, or build the group where the query uses it

Thanks @pcshrosbree, I reproduced the four queries. They run on #20894. On this head EF Core fails them with "Translation of 'Select' which contains grouping parameter without composition is not supported", and linq2db with LinqToDBException:

query { for c in db.Customers do
        groupValBy (c.Name, c.Age) c.City into g
        let n = g.Count()
        select (g, n) }

The same happens after a join, after two fors, and with a record as the element. They are scenarios now, groupValBy a tuple of values, the group beside its count and its after a join, after two for and a record variants.

The cause is RebuiltGroupings.Of(source: g, ..). It hands the provider's grouping to a method, which no provider translates, so that a group stays one object in every operator that mentions it. Counting the mentions in one lambda cannot say when that is needed, as you point out. There are two ways to go.

1. Keep Of (this head). A group used as a value is one object in every operator that mentions it, also on repeated enumeration. The cost is the four queries above on EF Core and on linq2db, and the group as a sequence is still the group on linq2db. PENDING_BREAKING_CHANGES.md lists them.

2. Build the group where the query uses it. The group is built with new Grouping(key, elements) as before this PR, and a group that a lambda uses as a value counts as an object (makesObject), so the Select that builds it is not fused into the consumer. What it gives up is the one object that Of keeps across several mentions: where the query builds the group inside a lambda that mentions it twice, g.Equals g and obj.ReferenceEquals(g, g) are false, and so is finding a group among the groups it is in. They are true on #20894. Three scenarios of this PR go with it: the group compared with itself, groups of groups, a group found among the groups it is in and groupBy a group, the group found among the elements it is the key of. The change removes RebuiltGroupings, rebuildsOneObject and the ofResult variants of the conversions: 24 lines added, 113 removed.

The scenarios of the test repository (248, the FSharp.Core of the SDK as the baseline), counted as the ones that translate and return the right rows:

FSharp.Core	EF Core SQLite	EF Core SQL Server and PostgreSQL	linq2db
baseline	106	111	154
1. keep Of (0677fc4)	116, 4 regressions	121, 4 regressions	168, 5 regressions
2. build where used (8cee6ec)	121, none	126, none	174, none

I go with the second: the four queries are plain F# that people write, and only code that tests a group for identity sees the other side. The suggestions below implement it against this head, and the same change is the branch draft/query-groupings-no-of of my fork. The tests of this repository pass with it, and the test repository gives the table above. If you agree, I apply them as one commit; if you prefer the first way, this head stays as it is.

The other points of the review are in this PR: the nested find has a line in the release note and the scenario find in a nested query over two for, and the four queries above are scenarios. A nested query { for (_, a) in g do sumBy a } over a group of tuples still does not translate while g.Sum(fun (_, a) -> a) does; I leave that as a follow-up.

[xperiandri]

Suggestions that make the second way, against this head. They belong together: apply all of them or none.

[xperiandri]

The cache of rebuilt groupings goes: the second way builds a group where the query uses it.

Suggested change

	/// The groupings rebuilt so far, by the grouping each was rebuilt from. A query that mentions a group in several
	/// places, or goes through a group of groups twice, gets one and the same object for a group every time.
	type internal RebuiltGroupings<'K, 'T> private () =
	static let rebuilt =
	System.Runtime.CompilerServices.ConditionalWeakTable<obj, Grouping<'K, 'T>>()
	static member Of(source: obj, key: 'K, values: seq<'T>) : IGrouping<'K, 'T> =
	match rebuilt.TryGetValue source with
	| true, grouping -> grouping
	| _ -> rebuilt.GetValue(source, fun _ -> Grouping<'K, 'T>(key, values))

[xperiandri]

RebuiltGrouping matches the construction of a Grouping again.

Suggested change

	/// A grouping rebuilt from the provider's grouping of converted elements, where a query mentions the group.
	[<return: Struct>]
	let rec (|RebuiltGrouping|_|) (expr: Expr) =
	match expr with
	| Patterns.Coerce(RebuiltGrouping grouping, _) -> ValueSome grouping
	| Patterns.Call(None, rebuild, [ _; key; elements ]) ->
	match rebuild.DeclaringType with
	| null -> ValueNone
	| owner when
	owner.IsGenericType
	&& owner.GetGenericTypeDefinition() = typedefof<RebuiltGroupings<_, _>>
	->
	ValueSome struct (key, elements)
	/// A grouping built from the provider's grouping of converted elements, where a query mentions the group.
	[<return: Struct>]
	let rec (|RebuiltGrouping|_|) (expr: Expr) =
	match expr with
	| Patterns.Coerce(RebuiltGrouping grouping, _) -> ValueSome grouping
	| Patterns.NewObject(constructor, [ key; elements ]) ->
	match constructor.DeclaringType with
	| null -> ValueNone
	| owner when owner.IsGenericType && owner.GetGenericTypeDefinition() = typedefof<Grouping<_, _>> -> ValueSome struct (key, elements)

[xperiandri]

A group is no longer rebuilt into one object by itself, so this goes. A group that a lambda uses whole is an object through makesObject.

Suggested change

	/// A group is rebuilt into the same object each time
	let rebuildsOneObject (projection: Expr) =
	match projection with
	| RebuiltGrouping _ -> true
	| _ -> false

[xperiandri]

The fusion is decided by usesWhole alone.

Suggested change

	&& (rebuildsOneObject projection || not (usesWhole consumed projection body))
	&& (not (usesWhole consumed projection body))

[xperiandri]

The conversion back to immutable tuples and records is the one of #20894 again, without ofResult.

Suggested change

	let rec ConvMutableToImmutable conv mutExpr =
	match conv with
	| TupleConv (isStruct, convs) ->
	let elements = convs |> List.mapi (fun i conv -> ConvMutableToImmutable conv (AnonymousObjectGet (mutExpr, i)))
	if isStruct then Expr.NewStructTuple elements else Expr.NewTuple elements
	| RecordConv (typ, convs) ->
	Expr.NewRecord (typ, convs |> List.mapi (fun i conv -> ConvMutableToImmutable conv (AnonymousObjectGet (mutExpr, i))))
	| SeqConv conv ->
	// At this point, we know the input is either an IQueryable or an IEnumerable.
	// If it is an IQueryable, we must return an IQueryable.
	let isIQ = IsIQueryableTy mutExpr.Type
	assert (IsIEnumerableTy mutExpr.Type || IsIQueryableTy mutExpr.Type)
	let mutElemTy = mutExpr.Type.GetGenericArguments().[0]
	// Generate "source.Select(fun v -> ...)" (remembering that Select is an extension member, i.e. static)
	let mutVar = new Var("v", mutElemTy)
	let mutToImmutConvExpr = ConvMutableToImmutable conv (Expr.Var mutVar)
	// Use Queryable.Select when source is IQueryable, Enumerable.Select otherwise
	// This preserves the IQueryable type, enabling query composition and EF Core async ops
	let immutExpr = MakeSelect (CanEliminate.Yes, isIQ, mutExpr, mutVar, mutToImmutConvExpr)
	immutExpr
	| GroupingConv (immutKeyTy, immutElemTy, conv) ->
	assert (mutExpr.Type.GetGenericTypeDefinition() = typedefof<IGrouping<_, _>>)
	let mutElemTy = mutExpr.Type.GetGenericArguments().[1]
	let immutIGroupingTy = typedefof<IGrouping<_, _>>.MakeGenericType [| immutKeyTy; immutElemTy |]
	let immutGroupingTy = typedefof<Grouping<_, _>>.MakeGenericType [| immutKeyTy; immutElemTy |]
	// Generate "source.Select(fun v -> ...)" (remembering that Select is an extension member, i.e. static)
	let var = new Var("v", mutElemTy)
	let convExpr = ConvMutableToImmutable conv (Expr.Var var)
	// Construct an IGrouping
	let args =
	[ Expr.PropertyGet (mutExpr, mutExpr.Type.GetProperty "Key")
	MakeSelect(CanEliminate.Yes, false, mutExpr, var, convExpr) ]
	Expr.Coerce (Expr.NewObject (immutGroupingTy.GetConstructors().[0], args), immutIGroupingTy)
	| NoConv ->
	mutExpr
	/// Given the expressions for a function (fun immutConsumingVar -> immutConsumingExpr) operating over immutable tuple and record
	/// types, build the expressions for an equivalent function (fun mutConsumingVar -> mutConsumingExpr) which will operate over mutable data, where the given conversion
	/// data says how immutable types have been replaced by mutable types in the type of the input variable.
	///
	/// For example, if 'conv' is NoConv, then the input function will be returned unchanged.
	///
	/// If 'conv' is a TupleConv, then the input function will accept immutable tuples, and the output
	/// function will accept mutable tuples. In this case, the function is implemented by replacing
	/// uses of the immutConsumingVar in the body of immutConsumingExpr with a tuple expression built
	/// from the elements of mutConsumingVar, and then simplifying the overall result.
	let ConvertImmutableConsumerToMutableConsumer conv (immutConsumingVar: Var, immutConsumingExpr: Expr) : Var * Expr =
	match conv with
	| NoConv -> (immutConsumingVar, immutConsumingExpr)
	| _ ->
	let mutConsumingVarType = ConvImmutableTypeToMutableType conv immutConsumingVar.Type
	let mutConsumingVar = Var (immutConsumingVar.Name, mutConsumingVarType)
	let immutConsumingVarReplacementExpr = ConvMutableToImmutable conv (Expr.Var mutConsumingVar)
	let mutConsumingExprBeforeSimplification = immutConsumingExpr.Substitute (fun v -> if v = immutConsumingVar then Some immutConsumingVarReplacementExpr else None)
	let mutConsumingExpr = SimplifyConsumingExpr mutConsumingExprBeforeSimplification
	mutConsumingVar, mutConsumingExpr

[xperiandri]

Pinned with the constructor.

Suggested change

	|> Scenario.violating rebuiltGroup
	|> Scenario.violating newGroup

[xperiandri]

Pinned with the constructor.

Suggested change

	|> Scenario.violating rebuiltGroup
	|> Scenario.violating newGroup

[xperiandri]

Pinned with the constructor.

Suggested change

	|> Scenario.violating rebuiltGroup
	|> Scenario.violating newGroup

[xperiandri]

Pinned with the constructor.

Suggested change

	|> Scenario.violating rebuiltGroup
	|> Scenario.violating newGroup

[xperiandri]

The paragraph of the impact says what the second way does.

Suggested change

**Impact:** A group of tuples or records that a query uses as a value in the middle of a query, for example one that is passed on whole to a later `select` or `where`, is rebuilt from the provider's grouping by `RebuiltGroupings.Of`, so that it stays one object in every operator that mentions it. linq2db, which built such a group on the client, cannot be handed the grouping itself and no longer translates that call: such a query no longer translates with linq2db. EF Core cannot translate the call either where the group goes into a `select` beside another value: `groupValBy (c.Name, c.Age) c.City into g; let n = g.Count(); select (g, n)` ran with the groups rebuilt in the middle of the query and now fails with "Translation of 'Select' which contains grouping parameter without composition is not supported". The same holds after a `join`, after two `for`s, and with a record as the element. Reading the key of a group, or counting, summing, averaging or searching it, is not affected: those read the provider's grouping.

**Impact:** A group of tuples or records that a query uses as a value in the middle of a query, for example one that is passed on whole to a later `select` or `where`, is built where the query uses it, as before, so a lambda that mentions the group twice has two objects: `obj.ReferenceEquals(g, g)` is false for it. Reading the key of a group, or counting, summing, averaging or searching it, is not affected: those read the provider's grouping.

[pcshrosbree]

The second way looks right to me. The four queries are plain F#, and your results show that neither EF Core nor linq2db translates them with Of, so I'd apply the suggestions as one commit.

One correction for PENDING_BREAKING_CHANGES.md: the new Impact line says the group is built "as before", but identity is not as before. With FSharp.Core 10.1.402 and with main, this query over three customers in two cities, through AsQueryable(), gives [("Oslo", true); ("Rome", true)]:

query { for c in customers.AsQueryable() do
        groupValBy (c.Name, c.Age) c.City into g
        where (g.Equals g)
        select (g.Key, obj.ReferenceEquals(g, g)) }

You say the second way makes both g.Equals g and obj.ReferenceEquals(g, g) false. The where clause would then filter out every group, leaving an empty result. That would change released behavior for in-memory IQueryable sources, so the note should say that. I'd also keep the three identity scenarios, pinned to their option-2 results, so the change stays visible in the tests.

[xperiandri]

Thanks @pcshrosbree, you are right: identity is not as before. I ran your query with FSharp.Core 10.1.402 and on #20894. Both build the group once, in the Select right after the GroupBy, and give [("Oslo", true); ("Rome", true)]. With the second way the group is built at each mention, and the query gives no rows.

d8a4360 applies the second way as one commit, with your two points:

• The Impact paragraph of PENDING_BREAKING_CHANGES.md now says that FSharp.Core 10.1 built the group once and that two mentions are two objects now. It shows your query: every city with true before, no rows now, and every city with false without the where. It also names the other two cases, Contains of a group among the groups it is in and a group as the key of groupBy, and says that EF Core, linq2db and the Cosmos DB provider translate none of these queries, before or after. In the test repository every provider fails these scenarios on 10.1.
• The three identity scenarios stay, pinned to the rows the second way gives (no rows, (length, false, false) and (city, false)) and to the number of new Grouping they build. I checked that the pins fail when they are changed.

The commit differs from the suggestions in these two points and in formatting (Fantomas splits the owner when line), so I did not apply the suggestions from the review. The PR description says the same now.