Repository navigation
Consume the elements a query translation produces - #20901
Open
xperiandri wants to merge 14 commits into
Open
xperiandri wants to merge 14 commits into
xperiandri wants to merge 14 commits into
Conversation
Every query test so far runs over AsQueryable() and compares rows, so a tree that only LINQ to Objects can run passes. These scenarios also check the tree: the query must stay on its provider, and must not contain what a C# expression lambda never does - a block, a parameter nothing binds, a call into FSharp.Core, a join nested in a collection selector, a member read from a tuple built without naming its members. A scenario names what is wrong with its tree today. The name has to be removed by the change that makes it untrue, because a scenario fails when its tree is cleaner than it says. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Selecting a struct tuple failed with "unreachable": the tuple was replaced by an anonymous object, but only reference tuples had an anonymous object type to be replaced by, and the way back always built a reference tuple. Both kinds now share the anonymous object of their arity, and the way back builds the kind the query asked for. Taking a struct tuple apart in a lambda failed with a tuple index out of range, because a struct tuple has fields where a reference tuple has properties. Fixes dotnet#15522 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Contributor
✅ Release notes checked
|
Contributor
Author
Covered casesEF Core SQLite translates 100 of 137 scenarios, 92 without this change. Now translated: |
There was a problem hiding this comment.
🟡 Changes recommended
The consumer rewrite reconstructs a selected record separately at each use, causing obj.ReferenceEquals(p, p) to return false and breaking reference-identity semantics on LINQ to Objects.
3 open issues
What changed in this PR
This PR updates FSharp.Core query translation so terminal predicates and aggregates consume translated elements without first rebuilding tuples or records. It builds on #20894’s struct-tuple support and provider-shape tests.
Changes:
- Rewrite terminal consumers and
countto use translated element types. - Support struct-tuple construction and field access in query expression trees.
- Add result and expression-shape scenarios, test registration, and release notes.
| File | Description |
|---|---|
| tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs | Covers loops, joins, subqueries, and terminal consumers. |
| tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs | Runs scenario groups as parameterized tests. |
| tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenarioDomain.fs | Supplies shared entities and sample data. |
| tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs | Checks results, provider retention, and expression shapes. |
| tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs | Captures provider expressions and detects shape violations. |
| tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs | Covers tuple, record, and other projections. |
| tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryOperatorScenarios.fs | Covers query operators and aggregates. |
| tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs | Covers LINQ method-syntax expression trees. |
| tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryKeyScenarios.fs | Covers join and grouping keys. |
| tests/FSharp.Core.UnitTests/FSharp.Core.UnitTests.fsproj | Registers the new test files. |
| src/FSharp.Core/QueryExtensions.fs | Tracks struct-tuple conversions through anonymous objects. |
| src/FSharp.Core/Query.fs | Rewrites terminal consumers and restores tuple kinds. |
| src/FSharp.Core/Linq.fs | Translates struct-tuple element access into field reads. |
| docs/release-notes/.FSharp.Core/11.0.200.md | Documents struct-tuple and terminal-consumer changes. |
Review effort: Balanced
The shape tests followed only the first source of each operator. Each lambda parameter is now tied to the sequence it stands for, so the check also covers the inner input of a join, the second parameter of a join result selector, a collection selector of SelectMany, an identity Select, a chain inside the lambda of another operator and a query captured from earlier. A value scenario now has to end in a Queryable operator that gives the value: a count made over the rows of the provider leaves it. The validator itself is tested on trees built by hand. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
xperiandri
force-pushed
the
fix/query-consume-translated
branch
from
November 1, 2026 22:52
462d2b6 to
67a2c46
Compare
T-Gro
reviewed
Nov 2, 2026
T-Gro
left a comment
T-Gro
left a comment
Member
There was a problem hiding this comment.
🤖 🕵️ AI review — verify independently.
| /// is asked for | ||
| let IsRebuiltObject (ty: Type) = | ||
| not ty.IsValueType | ||
| && (Microsoft.FSharp.Reflection.FSharpType.IsTuple ty || isPartiallyImmutableRecord ty) |
Member
There was a problem hiding this comment.
🤖 🕵️ Repeated uses of the same nested query get different wrapper objects — result changes from true on the base to false here; reconstructed sequences also need whole-element preservation.
open System.Linq
let result =
query {
for n in [1; 2].AsQueryable() do
let s =
query {
for m in [n].AsQueryable() do
select (m, m)
}
exists (obj.ReferenceEquals(s, s))
}
Contributor
Author
There was a problem hiding this comment.
Fixed in 96ae948. A sequence of tuples or records is selected anew at each mention, as a tuple or record is built, so it counts as an object of its own too, and the function gets the elements as they are. The scenario is exists with a nested query, the nested query compared with itself. The tree is the one main builds, with the unnamed read of the tuple that it has; that is pinned.
Member
There was a problem hiding this comment.
🤖🕵️ A nested query that returns groupings still gets two wrappers: true on the base, false at 96ae948c4d.
open System.Linq
let result =
query {
for n in [1; 2].AsQueryable() do
let groups =
query {
for m in [n].AsQueryable() do
groupValBy (m, m) m
}
exists (obj.ReferenceEquals(groups, groups))
}IsRebuiltObject does not recognize the IGrouping<_, _> element, although GroupingConv rebuilds it.
Contributor
Author
There was a problem hiding this comment.
Fixed in 998b564. IsRebuiltObject now asks whether the elements of a sequence are converted at all (IsConvertedElement): tuples and records of a class or a struct, groups of such elements, and sequences of those. A nested query of groups, or of struct tuples, that a function mentions twice is one sequence again.
Scenarios: exists with a nested query of groups, the nested query compared with itself (the example above) and the same for struct tuples. Both fail without the change.
T-Gro
self-requested a review
November 2, 2026 12:51
A value scenario was accepted when the last execution at the provider was an operator that gives a value. A count made by the client over rows taken from the provider passed whenever a function it ran asked the provider for a count of its own, because that count came last. Now every execution has to be such an operator, and so does the enumeration of a source taken as it is, which is recorded as an execution too. The violations of a tree are compared with every occurrence: a tree with one block was equal to a tree with two. The scenarios that have several of a kind are pinned with their number. The validator tests build a tree with an invoked lambda and one with an unbound parameter, which no scenario has, and the two client counts. The release note names the error a struct tuple gave on main. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…y what they hold The shape tests did not see an unnamed element that passes through a result selector giving one of its parameters back, in SelectMany, Join and Zip, and they skipped the element parameter of Aggregate together with the accumulated value. Both are checked now. A function with a seed takes the accumulated value and an element, and one without a seed takes two elements. A scenario compared an entity by the text it makes of itself, which names its key only: a result with the right key and the wrong name or age passed. An object of a class of the scenarios is now compared by every property that gives a plain value. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
xperiandri
force-pushed
the
fix/query-consume-translated
branch
from
November 2, 2026 18:57
67a2c46 to
96ae948
Compare
Contributor
Build Failure AnalysisSummary - All 16 primary Windows build binlogs fail while compiling Root cause:
|
T-Gro
reviewed
Nov 3, 2026
| else | ||
| Microsoft.FSharp.Reflection.FSharpType.IsTuple ty | ||
| || isPartiallyImmutableRecord ty | ||
| || ((IsIEnumerableTy ty || IsIQueryableTy ty) && IsRebuiltObject (ty.GetGenericArguments().[0])) |
Member
There was a problem hiding this comment.
🤖 🕵️ Stack overflow before even an empty query can run over a recursive struct.
open System.Linq
[<Struct>]
type Node = { Children: seq<Node> }
let result =
query {
for n in Seq.empty<Node>.AsQueryable() do
exists (Seq.isEmpty n.Children)
}
Contributor
Author
There was a problem hiding this comment.
Fixed in 998b564. The check no longer goes from a sequence into the fields of its element: it follows only the type arguments of sequences and groups, so a struct that holds a sequence of itself ends the search.
Test: exists over a struct that holds a sequence of itself (an empty source gives false, one element with an empty Children gives true). Without the change the test host dies with a stack overflow.
String.Contains(string, StringComparison) does not exist on .NET Framework, so the test project failed to compile for net472 and all the Windows legs of the CI went red. IndexOf with an ordinal comparison does the same on both. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Reflection promises no order of the properties of a class. On .NET Framework one scenario listed the properties of the first order it rendered in another order than those of the three after it, so the expected and the actual rows of the same orders differed as text. The names decide the order now. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
exists, all, sumBy, averageBy, minBy, maxBy and count after two fors, a join, or a tuple or record select took the translated query, turned every element back into an F# tuple or record with a final Select, and ran the consumer over the rebuilt value. A LINQ provider then saw a tuple built at the end of the query and a lambda reading an unnamed member of it, which EF Core evaluates on the client or rejects. The consumers now read the anonymous objects the translation already uses, the same way the next operator in a chain does, and count counts them without the conversion. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The consumer rewrite put a separate tuple or record in the place of every mention of the element, so a function that used the element, or a tuple or record in it, as a value of its own got a different object for each mention: ReferenceEquals(p, p) was false. A function like that is evaluated over the elements as the query produces them, as before. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ject, as a use of the whole A function over the elements of a query is rewritten over the elements as the query translation produces them, unless it uses an element, or a tuple or record in it, as a value of its own. Two reads were taken for a member of the element without being one: - A member of a record that is no field, like member x.Identity = x, is a function of the whole record: the rewrite gave each read a record of its own, and ReferenceEquals(p.Identity, p.Identity) was false. Only the fields of a record, and the items of a tuple, are members of the element now. - A struct that holds a tuple or a record of a class was not counted, although the tuple in it is built anew each time the struct is. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A function that consumes the elements of a query was rewritten over the elements the translation produces whenever it read only parts of an element. A part that is a nested query was then selected anew at every mention, so a function that mentioned it twice got two sequences where the element held one. A sequence of tuples or records now counts as an object of its own, like a tuple or a record, and the function gets the elements as they are. The scenarios add that nested query compared with itself, and a struct passed twice whose record is changed through one copy and read through the other. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…erts IsRebuiltObject asked of the element of a sequence whether it is itself a rebuilt object, which a group is not, and a struct tuple or record without a rebuilt part is not either. A nested query that returns groups, or struct tuples, was taken for a sequence the translation leaves alone, so a function that mentioned it twice got two sequences, where ReferenceEquals gave true on the code before. The sequence is rebuilt when its elements are converted at all: tuples and records of a class or a struct, groups of such elements, and sequences of those. Only the type arguments of the sequences and the groups are followed, which also ends the search for a struct that holds a sequence of itself. That search ran without end and overflowed the stack on the first query over such a struct. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
xperiandri
force-pushed
the
fix/query-consume-translated
branch
from
November 4, 2026 19:54
96ae948 to
998b564
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #20894
exists,all,count,sumBy,averageBy,minBy,maxByand theirNullablevariants after twofors, a join, or a tuple or recordselectnow run on the query's own elements instead of on tuples and records rebuilt by a finalSelect, which EF Core evaluated on the client or rejected.Details
🤖 Generated with Claude Code