You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This is primarily seen in libraries like EF core that take the C# translation as canonical (and often don't recognize F# translations). However this case warrants a bit of extra investigation in my opinion, it's a very verbose translation that probably doesn't need to be like this.
The LINQ expression 'where (copyOfStruct => (copyOfStruct != null).Invoke([x].creationDate) AndAlso (copyOfStruct => Convert(copyOfStruct, DateTimeOffset).Invoke([x].creationDate) < 30/11/2018 11:01:57 +00:00))' could not be translated and will be evaluated locally.
Original expression:
// the lambda here is the expression pointed to in the error
context.Blogs.Where(fun x -> x.creationDate.HasValue && x.creationDate.Value < now)
We need to establish if this is by design from the F# side and if it is not implement a fix on this side (I can take this) otherwise we (I) should focus on fixing EF core for 6.0.
From the EF Core side, we can definitely add support for a specific expression tree shape around this if needed. However, as @NinoFloris wrote above, it would be good to get confirmation whether this tree shape is by-design, or if you'd consider changing it. It especially seems problematic given the apparent discrepancy in tree shapes when class vs. record is used, as per dotnet/efcore#14013.
Some months ago I had a chat with @baronfel about this, where he linked the following piece of code in Query.fs, specifically Always eliminate these ... let copyOfStruct.
EF Core doesn't really work well with the query CE for a variety of reasons and regardless I prefer method syntax over the query CE, the same goes for C# linq syntax.
However the rewriting to remove these copyOfStruct bindings just happens for the query CE and not generally for all linq expression trees, meaning these bindings show up in trees passed to EF Core methods, this is the heart of the matter.
Now I know the compiler emits LeafExpressionConverter.QuotationToLambdaExpression when the argument type matches Expression<Func<...>>, would your suggestion be to lift this rewriting out of Query into LeafExpressionConverter?
Now I know the compiler emits LeafExpressionConverter.QuotationToLambdaExpression when the argument type matches Expression<Func<...>>, would your suggestion be to lift this rewriting out of Query into LeafExpressionConverter?
Apologies for the late reply @NinoFloris - yes, that would be the right approach
The exact predicate from this issue still produces two copyOfStruct blocks for a class property on FSharp.Core 10.1.402 and 11.0.200-rc2.26529.102, now as Expression.Block rather than the original Lambda.Invoke. I'd like to remove them in LeafExpressionConverter, where dsyme agreed above that this rewriting belongs, and I have three questions before writing a PR.
The record-field version and the class version inside query { } have no such blocks. The class-property version outside a query produces the two blocks shown here:
typeBlog()=member valcreationDate= Nullable<DateTimeOffset>()with get, set
blogs.Where(fun x -> x.creationDate.HasValue && x.creationDate.Value < now)// x => ({var copyOfStruct; ... } AndAlso ({var copyOfStruct; ... } < ...))
I'd remove a binding only where evaluation count, order and struct-copy behaviour are preserved, and keep the block otherwise. Each temporary in this predicate has one immediate use, as the receiver of HasValue or Value, so both blocks can go without repeating or skipping a getter call. The query rule can't move as it is: it matches copyOfStruct by name and value type, including in user-authored quotations. With f : unit -> int, <@ let copyOfStruct = f () in copyOfStruct + copyOfStruct @> calls f once through LeafExpressionConverter today. Query translation already calls f twice here, and moving its rule unchanged would bring that into LeafExpressionConverter. I'd check both the tree shape and the compiled behaviour in ComponentTests, as T-Gro suggested on #20649.
Is this still the preferred direction, with those constraints?
Should these two cases wait for separate changes?
Cnt(Key = k, Count = c) produces a returnVal block outside queries and MemberInit inside, but the query runs the setters in reverse order, Count then Key.
The tested struct-union match, match e.Kind with Handle -> true | _ -> false, keeps a matchValue block both inside and outside queries.
@pcshrosbree thanks for the careful questions. My fixes are pushed: #20894 adds the tree-shape validator and scenarios from #20649, and #20895 handles let in LeafExpressionConverter, which is the part that concerns this issue.
Yes, same direction and the same constraints. In Replace lets by their values in lambdas handed to a LINQ provider #20895 the value of a let takes the place of the variable only where that computes the same: the value merely reads (a constant, a member of the lambda parameter, or a member or method of a string, number or date on those), or the variable is read once, as the first thing the rest of the lambda evaluates. Every other let keeps its Block and is evaluated once where it is written, and so does every let of a lambda that contains an assignment or another statement. copyOfStruct is the one mutable name that is eligible, and only under the same conditions, so your <@ let copyOfStruct = f () in copyOfStruct + copyOfStruct @> still calls f once. The class-property predicate from this issue comes out without blocks in method syntax, and a getter is called once per use, as in the source. The query rule in Query.fs stays as it is. The differences that remain are documented in PENDING_BREAKING_CHANGES.md: record fields written out of order are evaluated in declaration order, and a let that only reads is computed where it is used.
I would keep both out of Replace lets by their values in lambdas handed to a LINQ provider #20895. An object initializer counts as a statement there, so a lambda that contains one keeps all its blocks. I have not looked at the setter order or at the matchValue block of the struct-union match. Separate changes are welcome.
They overlap, so please build on the validator rather than adding a second one. A new shape is one line in tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/Query*Scenarios.fs, and Scenario.violating pins what its tree still contains today, so a fix removes exactly the entries it makes untrue. Compiled behavior (evaluation count and order) is checked in ExpressionLambdaConversionTests.fs of Replace lets by their values in lambdas handed to a LINQ provider #20895, in the same project.
@pcshrosbree could you review the rest of the stack as well? All of them are stacked on #20894 and carry its commits until it merges; #20896 also sits on #20895. Each PR has a "Covered cases" comment with what was run.
tests/SQLProviderTests/Queries.fs has 59 queries in the style of SQLProvider users: joins, leftOuterJoin, groupJoin, (!!) navigation, groupBy with aggregates, |=|, method syntax. They run against a SQLite database and the rows are compared with the expected ones. It would help to know whether these are the idioms you care about, and which ones are missing.
With the pull requests of this stack the 59 queries pass. With the draft #20903 (left outer join) three of them return wrong rows, because LEFT OUTER JOIN becomes INNER JOIN.
@xperiandri I've reviewed the stack; #20895 removes the blocks from this issue's predicate without changing the results in the getter cases I tried. I'll address the setter order and the matchValue block in separate follow-up changes on top of your validator.
I left reviews on #20894, #20895, #20897, #20898, #20899 and #20900. The nested-lambda results I checked on #20896 matched plain F#. On #20901, LINQ to Objects results match #20894 in the cases I checked, now including the reference-identity cases Copilot raised. The EF Core probes show that exists and sumBy translate after a join.
I'll go through FSharp.Linq.ProviderTests next and answer here about the idioms and gaps.
@xperiandri I've reviewed the provider tests without running them. The SQLProvider cases cover joins and grouping, but some checks can pass on a wrong result and several query idioms are missing.
Missing from Queries.fs:
nullable predicates beyond .IsSome, especially a presence check followed by .Value < now with a captured timestamp, as in this issue
captured paging variables and List.contains over a local list
StartsWith, ToUpper, =%, exactlyOneOrDefault, nth and if/else in select
executeQueryAsync and struct tuples
On the checks:
exists, all, contains and Any all expect true. False companions would catch an implementation that always returns true.
render prints 1, 1L and 1.0 identically, so the conversion checks miss numeric type errors with the same rendered value.
Baseline update mode writes observations marked wrong and returns success. Those entries need review before a refreshed baseline is accepted.
The schema has no nullable join-key columns or nullable string columns, so nullable source keys and string predicates over NULL source values are not covered.
For #20903, SQLProvider could also recognize LEFT joins when DefaultIfEmpty appears in the SelectMany collection selector after GroupJoin. SQLProvider currently only unwraps DefaultIfEmpty there. This provider-side change is worth testing against the reported SQLProvider regression. The EF Core case that reads the GroupJoin group again would remain open.
Activity
roji commentedon Mar 31, 2021
From the EF Core side, we can definitely add support for a specific expression tree shape around this if needed. However, as @NinoFloris wrote above, it would be good to get confirmation whether this tree shape is by-design, or if you'd consider changing it. It especially seems problematic given the apparent discrepancy in tree shapes when class vs. record is used, as per dotnet/efcore#14013.
[-]Expression translation is off[/-][+]Linq Expression translation is off[/+]NinoFloris commentedon Aug 26, 2021
@dsyme I'm exploring a fix for this.
Some months ago I had a chat with @baronfel about this, where he linked the following piece of code in Query.fs, specifically
Always eliminate these ... let copyOfStruct.fsharp/src/fsharp/FSharp.Core/Query.fs
Lines 1018 to 1027 in b23b2a0
EF Core doesn't really work well with the query CE for a variety of reasons and regardless I prefer method syntax over the query CE, the same goes for C# linq syntax.
However the rewriting to remove these copyOfStruct bindings just happens for the query CE and not generally for all linq expression trees, meaning these bindings show up in trees passed to EF Core methods, this is the heart of the matter.
Now I know the compiler emits
LeafExpressionConverter.QuotationToLambdaExpressionwhen the argument type matchesExpression<Func<...>>, would your suggestion be to lift this rewriting out of Query into LeafExpressionConverter?dsyme commentedon Mar 30, 2022
Apologies for the late reply @NinoFloris - yes, that would be the right approach
pcshrosbree commentedon Nov 1, 2026
The exact predicate from this issue still produces two
copyOfStructblocks for a class property on FSharp.Core 10.1.402 and 11.0.200-rc2.26529.102, now asExpression.Blockrather than the originalLambda.Invoke. I'd like to remove them inLeafExpressionConverter, where dsyme agreed above that this rewriting belongs, and I have three questions before writing a PR.The record-field version and the class version inside
query { }have no such blocks. The class-property version outside a query produces the two blocks shown here:I'd remove a binding only where evaluation count, order and struct-copy behaviour are preserved, and keep the block otherwise. Each temporary in this predicate has one immediate use, as the receiver of
HasValueorValue, so both blocks can go without repeating or skipping a getter call. The query rule can't move as it is: it matchescopyOfStructby name and value type, including in user-authored quotations. Withf : unit -> int,<@ let copyOfStruct = f () in copyOfStruct + copyOfStruct @>callsfonce throughLeafExpressionConvertertoday. Query translation already callsftwice here, and moving its rule unchanged would bring that intoLeafExpressionConverter. I'd check both the tree shape and the compiled behaviour in ComponentTests, as T-Gro suggested on #20649.Cnt(Key = k, Count = c)produces areturnValblock outside queries andMemberInitinside, but the query runs the setters in reverse order,CountthenKey.match e.Kind with Handle -> true | _ -> false, keeps amatchValueblock both inside and outside queries.xperiandri commentedon Nov 1, 2026
Most probably yes, let me
push my fixes so you can check
xperiandri commentedon Nov 1, 2026
@pcshrosbree please review PR and its tests
xperiandri commentedon Nov 1, 2026
@pcshrosbree thanks for the careful questions. My fixes are pushed: #20894 adds the tree-shape validator and scenarios from #20649, and #20895 handles
letinLeafExpressionConverter, which is the part that concerns this issue.Yes, same direction and the same constraints. In Replace lets by their values in lambdas handed to a LINQ provider #20895 the value of a
lettakes the place of the variable only where that computes the same: the value merely reads (a constant, a member of the lambda parameter, or a member or method of a string, number or date on those), or the variable is read once, as the first thing the rest of the lambda evaluates. Every otherletkeeps itsBlockand is evaluated once where it is written, and so does everyletof a lambda that contains an assignment or another statement.copyOfStructis the onemutablename that is eligible, and only under the same conditions, so your<@ let copyOfStruct = f () in copyOfStruct + copyOfStruct @>still callsfonce. The class-property predicate from this issue comes out without blocks in method syntax, and a getter is called once per use, as in the source. The query rule inQuery.fsstays as it is. The differences that remain are documented inPENDING_BREAKING_CHANGES.md: record fields written out of order are evaluated in declaration order, and aletthat only reads is computed where it is used.I would keep both out of Replace lets by their values in lambdas handed to a LINQ provider #20895. An object initializer counts as a statement there, so a lambda that contains one keeps all its blocks. I have not looked at the setter order or at the
matchValueblock of the struct-union match. Separate changes are welcome.They overlap, so please build on the validator rather than adding a second one. A new shape is one line in
tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/Query*Scenarios.fs, andScenario.violatingpins what its tree still contains today, so a fix removes exactly the entries it makes untrue. Compiled behavior (evaluation count and order) is checked inExpressionLambdaConversionTests.fsof Replace lets by their values in lambdas handed to a LINQ provider #20895, in the same project.xperiandri commentedon Nov 1, 2026
@pcshrosbree could you review the rest of the stack as well? All of them are stacked on #20894 and carry its commits until it merges; #20896 also sits on #20895. Each PR has a "Covered cases" comment with what was run.
ValueTuple] #15522)letin lambdas handed to a provider (Linq Expression translation is off #11332, Different property order of a LINQ select expression cannot be translated #11131, Inconsistent anonymous record instantiation on IQueryable #15648)Selectfst,snd,isNull,boxand the math functions as C# would express themexists,all,count,sumByand the like on the elements the translation produces#20897 and #20899 change the trees EF Core receives the most, so a look from the EF Core side would help there.
xperiandri commentedon Nov 2, 2026
@pcshrosbree, if you have time, could you also look at the tests? I put the SQLProvider side into a test repository that the regression pipeline of dotnet/fsharp can run: https://github.com/fsprojects/FSharp.Linq.ProviderTests
tests/SQLProviderTests/Queries.fshas 59 queries in the style of SQLProvider users: joins,leftOuterJoin,groupJoin,(!!)navigation,groupBywith aggregates,|=|, method syntax. They run against a SQLite database and the rows are compared with the expected ones. It would help to know whether these are the idioms you care about, and which ones are missing.With the pull requests of this stack the 59 queries pass. With the draft #20903 (left outer join) three of them return wrong rows, because
LEFT OUTER JOINbecomesINNER JOIN.pcshrosbree commentedon Nov 2, 2026
@xperiandri I've reviewed the stack; #20895 removes the blocks from this issue's predicate without changing the results in the getter cases I tried. I'll address the setter order and the
matchValueblock in separate follow-up changes on top of your validator.I left reviews on #20894, #20895, #20897, #20898, #20899 and #20900. The nested-lambda results I checked on #20896 matched plain F#. On #20901, LINQ to Objects results match #20894 in the cases I checked, now including the reference-identity cases Copilot raised. The EF Core probes show that
existsandsumBytranslate after ajoin.I'll go through FSharp.Linq.ProviderTests next and answer here about the idioms and gaps.
pcshrosbree commentedon Nov 2, 2026
@xperiandri I've reviewed the provider tests without running them. The SQLProvider cases cover joins and grouping, but some checks can pass on a wrong result and several query idioms are missing.
Missing from
Queries.fs:.IsSome, especially a presence check followed by.Value < nowwith a captured timestamp, as in this issueList.containsover a local listStartsWith,ToUpper,=%,exactlyOneOrDefault,nthandif/elseinselectexecuteQueryAsyncand struct tuplesOn the checks:
exists,all,containsandAnyall expecttrue. False companions would catch an implementation that always returnstrue.renderprints1,1Land1.0identically, so the conversion checks miss numeric type errors with the same rendered value.wrongand returns success. Those entries need review before a refreshed baseline is accepted.For #20903, SQLProvider could also recognize LEFT joins when
DefaultIfEmptyappears in theSelectManycollection selector afterGroupJoin. SQLProvider currently only unwrapsDefaultIfEmptythere. This provider-side change is worth testing against the reported SQLProvider regression. The EF Core case that reads theGroupJoingroup again would remain open.Replace lets by their values in lambdas handed to a LINQ provider