Repository navigation
Translate F# operators in LINQ trees as C# would express them - #20900
xperiandri wants to merge 12 commits into
Conversation
✅ Release notes checked
|
Covered casesEF Core SQLite translates 115 of 153 scenarios, 94 without this change, in queries and in Cosmos translates 31 scenarios, up from 18. Not translated: |
There was a problem hiding this comment.
🟡 Changes recommended
Tuple reductions can discard user-created tuple-element evaluations and suppress expected exceptions, while join analysis misses unnamed tuples in inner sources.
2 open issues
What changed in this PR
This PR makes FSharp.Core’s LINQ translation emit standard expression nodes and System.Math calls that providers can recognize. It builds on #20894’s struct-tuple support and query-shape tests.
Changes:
- Translate tuple access, null checks, boxing, and primitive math operators into provider-friendly expressions.
- Add query scenarios and expression tests covering tree structure and compiled behavior.
- Document expression-tree changes, including floating-point
min/maxbehavior and provider-specific rounding.
| File | Description |
|---|---|
tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs |
Cover loops, subqueries, and left joins. |
tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs |
Register scenario-based query tests. |
tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenarioDomain.fs |
Supply shared query entities and data. |
tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs |
Compare query results and expected tree violations. |
tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs |
Capture queries and detect unsupported tree shapes. |
tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs |
Cover tuple, record, and object projections. |
tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryOperatorScenarios.fs |
Cover filtering, sorting, joins, and aggregates. |
tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs |
Exercise LINQ method-syntax expressions. |
tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryKeyScenarios.fs |
Cover composite join and grouping keys. |
tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryFSharpOperatorScenarios.fs |
Exercise F# operators in provider queries. |
tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/ExpressionFSharpOperatorTests.fs |
Check translated nodes and compiled operator results. |
tests/FSharp.Core.UnitTests/FSharp.Core.UnitTests.fsproj |
Include the new test files. |
src/FSharp.Core/QueryExtensions.fs |
Adapt struct tuples while preserving tuple kind. |
src/FSharp.Core/Query.fs |
Expand fst/snd and reconstruct struct tuples. |
src/FSharp.Core/Linq.fs |
Translate operators and struct-tuple field reads. |
docs/release-notes/PENDING_BREAKING_CHANGES.md |
Explain tree-shape and behavioral compatibility changes. |
docs/release-notes/.FSharp.Core/11.0.200.md |
Add struct-tuple and operator translation release notes. |
Review effort: Balanced
69af300 to
157fc5b
Compare
pcshrosbree
left a comment
There was a problem hiding this comment.
At the current head, the tree results for the changed functions match compiled F# in every edge case I checked, including four min and max cases where main's tree does not. Please document the incorrect SQLite results for decimal min and max in PENDING_BREAKING_CHANGES.md and add max 1.0 nan to the test.
The edge values were abs of MinValue, sign nan, midpoint rounding on float and decimal, NaN and signed zero in min and max, isNull on a type with a custom ==, and box of a struct and of null. The four min and max cases are max 1.0 nan, min nan 1.0, max 1.0f nanf and max -0.0 0.0. Main's tree gives 1, 1, 1f and -0 for them; compiled F# and this PR give NaN, NaN, NaN and 0.
- Decimal
minandmaxnow reach EF Core 10.0.2 in this probe, and on SQLite, which stores decimals as text, they return wrong rows. With rows2.5mand-1.25m,Where(fun x -> max x.D 10m = 10m)used to fail to translate. It now returns one row instead of two, fromWHERE max("i"."D", '10.0') = '10.0'. C#Math.Maxgets the same SQL, so this is EF Core's behaviour, but the note warns only aboutMath.Round. - The NaN test asserts
max nan 0.0, which already passes on #20894. Please also assertmax 1.0 nan, which fails there. - The "Was" line shows
Call(Operators.Abs, x). Forabs,roundand the other functions with member constraints, the old tree called the witness overload,Operators.Abs$W(FuncConvert.ToFSharpFunc(..), x).
157fc5b to
0f10296
Compare
Dismissed: SQLite decimal min/max behavior, warning documentation, NaN/abs coverage, and the workflow gate remain unresolved.
|
Thanks @pcshrosbree. Changes since 157fc5b:
|
0f10296 to
68f8fa6
Compare
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>
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>
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>
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>
fst and snd, isNull, box, and the math functions - abs, sqrt, floor, ceil, round, truncate, sign, exp, log, log10, the trigonometric functions, min, max and ** - reached LINQ providers as calls into FSharp.Core (Operators.Fst, Operators.IsNull, Operators.Abs$W with its witness, ...). No provider knows those methods, so a query or an Expression<Func<..>> lambda using any of them could not be translated. They now become what a C# lambda has for the same thing: a read of Item1/Item2, a comparison with a null constant, a conversion to object, and a call of the System.Math method the F# function itself calls for that primitive type (through float for float32, as the F# functions do). EF Core turns these into abs, sqrt, floor, IS NULL and so on. Math functions over other types keep their F# implementation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
fst and snd were read as the item of the tuple, which let the query translation replace a tuple of the query by its item and drop the evaluation of the others: fst (x, 1 / (x - x)) did not divide. Only a tuple the translation rebuilds, a variable or a tuple taken from one, is read directly now. Any other tuple keeps its items, and the converter reads the item of the tuple it builds. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The tree answered max 1.0 nan, min nan 1.0 and max 1.0f nanf with a number before this change, where the F# functions answer NaN. The test asserts all three next to the one it had. The note on breaking changes says what the old tree called for abs, and what min and max of decimals do on SQLite. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
68f8fa6 to
305f4a3
Compare


Stacked on #20894
fst,snd,isNull,boxand the math functions on primitive types now reach LINQ providers as tuple reads, null comparisons, conversions andSystem.Mathcalls instead of calls into FSharp.Core that no provider knows.Details
abs,sqrt,floor,ceil,round,truncate,sign,exp,log,log10, the trigonometric and hyperbolic functions,min,max,atan2and**. Other types keep the F# implementation.minandmaxonfloatandfloat32that differs from before when an argument is NaN or the arguments are zeros of different sign.Math.Roundrounds midpoints its own way; SQLite rounds them away from zero.🤖 Generated with Claude Code