From d1be7038c13d9f5adccbf7fbe65ef975b0e447ce Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 27 Sep 2026 04:27:15 +0200 Subject: [PATCH 01/14] Test what queries hand to a LINQ provider 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 --- .../FSharp.Core.UnitTests.fsproj | 9 + .../QueryKeyScenarios.fs | 170 +++++++++ .../QueryMethodSyntaxScenarios.fs | 116 ++++++ .../QueryOperatorScenarios.fs | 197 ++++++++++ .../QueryProjectionScenarios.fs | 130 +++++++ .../QueryProviderShape.fs | 355 ++++++++++++++++++ .../Microsoft.FSharp.Linq/QueryScenario.fs | 189 ++++++++++ .../QueryScenarioDomain.fs | 109 ++++++ .../Microsoft.FSharp.Linq/QueryShapeTests.fs | 31 ++ .../QueryStructureScenarios.fs | 268 +++++++++++++ 10 files changed, 1574 insertions(+) create mode 100644 tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryKeyScenarios.fs create mode 100644 tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs create mode 100644 tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryOperatorScenarios.fs create mode 100644 tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs create mode 100644 tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs create mode 100644 tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs create mode 100644 tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenarioDomain.fs create mode 100644 tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs create mode 100644 tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core.UnitTests.fsproj b/tests/FSharp.Core.UnitTests/FSharp.Core.UnitTests.fsproj index 494509d0906..3a20b60f7e4 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core.UnitTests.fsproj +++ b/tests/FSharp.Core.UnitTests/FSharp.Core.UnitTests.fsproj @@ -92,6 +92,15 @@ + + + + + + + + + diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryKeyScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryKeyScenarios.fs new file mode 100644 index 00000000000..03011a4d99f --- /dev/null +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryKeyScenarios.fs @@ -0,0 +1,170 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Core.UnitTests.Linq.QueryKeyScenarios + +open System.Linq +open System.Linq.Expressions +open Microsoft.FSharp.Linq.RuntimeHelpers + +let private block = [ Violation.Statement ExpressionType.Block ] + +let private customers (data: Data) = Seq.ofList data.Customers + +/// Orders paired with the lines that share both the order number and, as quantity, the customer number. +let private matchingLines (data: Data) = + seq { + for o in data.Orders do + for l in data.Lines do + if l.OrderId = o.Id && l.Quantity = o.CustomerId then + yield o.Id, l.Id + } + +let private matchingLinesOrNone (data: Data) = + seq { + for o in data.Orders do + match matchingLines data |> Seq.filter (fun (order, _) -> order = o.Id) |> List.ofSeq with + | [] -> yield o.Id, -1 + | matches -> yield! matches + } + +let private joins () = + [ Scenario.rows + "join on a scalar" + (fun db -> query { for c in db.Customers do join o in db.Orders on (c.Id = o.CustomerId); select (c.Name, o.Id) }) + (fun data -> seq { for c in data.Customers do for o in c.Orders -> c.Name, o.Id }) + + Scenario.rows + "join on a tuple" + (fun db -> + query { for o in db.Orders do join l in db.Lines on ((o.Id, o.CustomerId) = (l.OrderId, l.Quantity)); select (o.Id, l.Id) }) + matchingLines + + Scenario.rows + "join on a struct tuple" + (fun db -> + query { + for o in db.Orders do + join l in db.Lines on (struct (o.Id, o.CustomerId) = struct (l.OrderId, l.Quantity)) + select (o.Id, l.Id) + }) + matchingLines + + Scenario.rows + "join on a record, fields in declaration order" + (fun db -> + query { + for o in db.Orders do + join l in db.Lines on ({ OrderNumber = o.Id; Units = o.CustomerId } = { OrderNumber = l.OrderId; Units = l.Quantity }) + select (o.Id, l.Id) + }) + matchingLines + + Scenario.rows + "join on a record, fields out of declaration order" + (fun db -> + query { + for o in db.Orders do + join l in db.Lines on ({ Units = o.CustomerId; OrderNumber = o.Id } = { Units = l.Quantity; OrderNumber = l.OrderId }) + select (o.Id, l.Id) + }) + matchingLines + |> Scenario.violating block + + Scenario.rows + "join on an anonymous record, fields in alphabetical order" + (fun db -> + query { + for o in db.Orders do + join l in db.Lines on ({| Order = o.Id; Units = o.CustomerId |} = {| Order = l.OrderId; Units = l.Quantity |}) + select (o.Id, l.Id) + }) + matchingLines + + Scenario.rows + "join on an anonymous record, fields out of alphabetical order" + (fun db -> + query { + for o in db.Orders do + join l in db.Lines on ({| Units = o.CustomerId; Order = o.Id |} = {| Units = l.Quantity; Order = l.OrderId |}) + select (o.Id, l.Id) + }) + matchingLines + |> Scenario.violating block + + Scenario.rows + "groupJoin on a tuple" + (fun db -> + query { + for o in db.Orders do + groupJoin l in db.Lines on ((o.Id, o.CustomerId) = (l.OrderId, l.Quantity)) into lines + select (o.Id, lines.Count()) + }) + (fun data -> seq { for o in data.Orders -> o.Id, matchingLines data |> Seq.filter (fun (order, _) -> order = o.Id) |> Seq.length }) + + Scenario.rows + "leftOuterJoin on a tuple" + (fun db -> + query { + for o in db.Orders do + leftOuterJoin l in db.Lines on ((o.Id, o.CustomerId) = (l.OrderId, l.Quantity)) into lines + for l in lines do + select (o.Id, (if obj.ReferenceEquals(l, null) then -1 else l.Id)) + }) + matchingLinesOrNone ] + +let private groupings () = + [ Scenario.rows + "groupBy a scalar" + (fun db -> query { for c in db.Customers do groupBy c.City into g; select (g.Key, g.Count()) }) + (customers >> Seq.countBy _.City) + + Scenario.rows + "groupBy a scalar, groups themselves" + (fun db -> query { for c in db.Customers do groupBy c.City }) + (customers >> Seq.groupBy _.City >> Seq.map (fun (city, group) -> Grouping(city, group) :> IGrouping<_, _>)) + + Scenario.rows + "groupBy a tuple" + (fun db -> query { for c in db.Customers do groupBy (c.City, c.Age > 28) into g; select (g.Key, g.Count()) }) + (customers >> Seq.countBy (fun c -> c.City, c.Age > 28)) + + Scenario.rows + "groupBy a tuple, element of the key" + (fun db -> query { for c in db.Customers do groupBy (c.City, c.Age > 28) into g; select (fst g.Key, g.Count()) }) + (customers >> Seq.countBy (fun c -> c.City, c.Age > 28) >> Seq.map (fun ((city, _), count) -> city, count)) + |> Scenario.violating [ Violation.RuntimeHelperCall "Operators.Fst" ] + + Scenario.rows + "groupBy a struct tuple" + (fun db -> query { for c in db.Customers do groupBy (struct (c.City, c.Age > 28)) into g; select (g.Key, g.Count()) }) + (customers >> Seq.countBy (fun c -> struct (c.City, c.Age > 28))) + + Scenario.rows + "groupBy a record, fields in declaration order" + (fun db -> query { for c in db.Customers do groupBy { KeyCity = c.City; KeyAge = c.Age / 10 } into g; select (g.Key, g.Count()) }) + (customers >> Seq.countBy (fun c -> { KeyCity = c.City; KeyAge = c.Age / 10 })) + + Scenario.rows + "groupBy a record, fields out of declaration order" + (fun db -> query { for c in db.Customers do groupBy { KeyAge = c.Age / 10; KeyCity = c.City } into g; select (g.Key, g.Count()) }) + (customers >> Seq.countBy (fun c -> { KeyCity = c.City; KeyAge = c.Age / 10 })) + |> Scenario.violating block + + Scenario.rows + "groupBy an anonymous record, fields in alphabetical order" + (fun db -> query { for c in db.Customers do groupBy {| City = c.City; Decade = c.Age / 10 |} into g; select (g.Key, g.Count()) }) + (customers >> Seq.countBy (fun c -> {| City = c.City; Decade = c.Age / 10 |})) + + Scenario.rows + "groupBy an anonymous record, fields out of alphabetical order" + (fun db -> query { for c in db.Customers do groupBy {| Decade = c.Age / 10; City = c.City |} into g; select (g.Key, g.Count()) }) + (customers >> Seq.countBy (fun c -> {| City = c.City; Decade = c.Age / 10 |})) + |> Scenario.violating block + + Scenario.rows + "groupValBy a tuple of values" + (fun db -> query { for c in db.Customers do groupValBy (c.Name, c.Age) c.City into g; select (g.Key, g.Count()) }) + (customers >> Seq.countBy _.City) + |> Scenario.violating [ Violation.RuntimeHelperCall "new Grouping`2" ] ] + +let cases () = joins () @ groupings () diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs new file mode 100644 index 00000000000..b8a704a846e --- /dev/null +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs @@ -0,0 +1,116 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Core.UnitTests.Linq.QueryMethodSyntaxScenarios + +open System.Linq +open System.Linq.Expressions + +let private block = [ Violation.Statement ExpressionType.Block ] + +let private reading typeName = + [ Violation.ReadsUnnamedMember typeName ] + +let private customers (data: Data) = Seq.ofList data.Customers + +let cases () = + [ Scenario.rows "Select a tuple" (fun db -> db.Customers.Select(fun c -> (c.Id, c.Name))) (customers >> Seq.map (fun c -> c.Id, c.Name)) + + Scenario.rows + "Select a struct tuple" + (fun db -> db.Customers.Select(fun c -> struct (c.Id, c.Name))) + (customers >> Seq.map (fun c -> struct (c.Id, c.Name))) + + Scenario.rows + "Where over a query built earlier" + (fun db -> + let withLargeOrders = db.Orders.Where(fun o -> o.Total > 4.0).Select(fun o -> o.CustomerId) + db.Customers.Where(fun c -> withLargeOrders.Contains c.Id).Select(fun c -> c.Name)) + (customers >> Seq.filter (fun c -> c.Orders |> Seq.exists (fun o -> o.Total > 4.0)) >> Seq.map _.Name) + + Scenario.rows + "Where over a query built earlier with a let" + (fun db -> + let withLargeOrders = db.Orders.Where(fun o -> let total = o.Total in total > 4.0).Select(fun o -> o.CustomerId) + db.Customers.Where(fun c -> withLargeOrders.Contains c.Id).Select(fun c -> c.Name)) + (customers >> Seq.filter (fun c -> c.Orders |> Seq.exists (fun o -> o.Total > 4.0)) >> Seq.map _.Name) + |> Scenario.violating block + + Scenario.rows + "Select a record, fields in declaration order" + (fun db -> db.Customers.Select(fun c -> { PersonName = c.Name; PersonAge = c.Age })) + (customers >> Seq.map (fun c -> { PersonName = c.Name; PersonAge = c.Age })) + + Scenario.rows + "Select a record, fields out of declaration order" + (fun db -> db.Customers.Select(fun c -> { PersonAge = c.Age; PersonName = c.Name })) + (customers >> Seq.map (fun c -> { PersonName = c.Name; PersonAge = c.Age })) + |> Scenario.violating block + + Scenario.rows + "Select an anonymous record, fields in alphabetical order" + (fun db -> db.Customers.Select(fun c -> {| Age = c.Age; Name = c.Name |})) + (customers >> Seq.map (fun c -> {| Age = c.Age; Name = c.Name |})) + + Scenario.rows + "Select an anonymous record, fields out of alphabetical order" + (fun db -> db.Customers.Select(fun c -> {| Name = c.Name; Age = c.Age |})) + (customers >> Seq.map (fun c -> {| Age = c.Age; Name = c.Name |})) + |> Scenario.violating block + + Scenario.rows + "Select with a let used twice" + (fun db -> db.Customers.Select(fun c -> let upper = c.Name.ToUpper() in upper + upper)) + (customers >> Seq.map (fun c -> c.Name.ToUpper() + c.Name.ToUpper())) + |> Scenario.violating block + + Scenario.rows + "Where taking a tuple apart" + (fun db -> db.Customers.Select(fun c -> (c.Id, c.Name)).Where(fun (id, _) -> id > 1)) + (customers >> Seq.map (fun c -> c.Id, c.Name) >> Seq.filter (fun (id, _) -> id > 1)) + |> Scenario.violating (reading "Tuple`2" @ block) + + Scenario.rows + "Where taking the last of nine apart" + (fun db -> + db.Customers + .Select(fun c -> (c.Id + 100, c.Name, c.City, c.Age, c.Id * 7, c.Name.Length, c.City.Length, c.Age * 2, c.Id)) + .Where(fun (_, _, _, _, _, _, _, _, last) -> last > 1) + .Select(fun (_, name, _, _, _, _, _, _, _) -> name)) + (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) + |> Scenario.violating (reading "Tuple`8" @ block) + + Scenario.rows + "Where over a query that selects a tuple" + (fun db -> (query { for c in db.Customers do select (c.Id, c.Name) }).Where(fun (id, _) -> id > 1)) + (customers >> Seq.map (fun c -> c.Id, c.Name) >> Seq.filter (fun (id, _) -> id > 1)) + |> Scenario.violating (reading "Tuple`2" @ block) + + Scenario.rows + "Where on a nullable property" + (fun db -> db.Customers.Where(fun c -> c.Registered.HasValue && c.Registered.Value.Year > 2020)) + (customers >> Seq.filter (fun c -> c.Registered.HasValue && c.Registered.Value.Year > 2020)) + |> Scenario.violating block + + Scenario.rows + "Where with a lambda over another source" + (fun db -> db.Customers.Where(fun c -> db.Orders.Any(fun o -> o.CustomerId = c.Id))) + (customers >> Seq.filter (fun c -> c.Orders.Count > 0)) + |> Scenario.violating + [ Violation.RuntimeHelperCall "LeafExpressionConverter.QuotationToLambdaExpression" + Violation.RuntimeHelperCall "LeafExpressionConverter.SubstHelper" ] + + Scenario.rows + "OrderBy, ThenByDescending" + (fun db -> db.Customers.OrderBy(fun c -> c.City).ThenByDescending(fun c -> c.Age).Select(fun c -> c.Name)) + (customers >> Seq.sortBy (fun c -> c.City, -c.Age) >> Seq.map _.Name) + |> Scenario.ordered + + Scenario.rows + "GroupBy a tuple" + (fun db -> db.Customers.GroupBy(fun c -> (c.City, c.Age > 28)).Select(fun g -> (g.Key, g.Count()))) + (customers >> Seq.countBy (fun c -> c.City, c.Age > 28)) + + Scenario.rows + "SelectMany over a navigation collection" + (fun db -> db.Customers.SelectMany(fun c -> c.Orders :> seq).Select(fun o -> o.Id)) + (fun data -> data.Orders |> Seq.map _.Id) ] diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryOperatorScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryOperatorScenarios.fs new file mode 100644 index 00000000000..6b2339a996b --- /dev/null +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryOperatorScenarios.fs @@ -0,0 +1,197 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Core.UnitTests.Linq.QueryOperatorScenarios + +open System +open System.Linq +open Microsoft.FSharp.Linq +open Microsoft.FSharp.Linq.NullableOperators + +let private customers (data: Data) = Seq.ofList data.Customers + +let private lines (data: Data) = Seq.ofList data.Lines + +let private discountOrLowest (line: OrderLine) = + if line.Discount.HasValue then line.Discount.Value else Int32.MinValue + +let private discounts (data: Data) = + data.Lines |> List.filter _.Discount.HasValue |> List.map _.Discount.Value + +let private sequences () = + [ Scenario.rows + "where" + (fun db -> query { for c in db.Customers do where (c.Age > 26); select c.Name }) + (customers >> Seq.filter (fun c -> c.Age > 26) >> Seq.map _.Name) + + Scenario.rows + "where with string methods" + (fun db -> query { for c in db.Customers do where (c.Name.StartsWith "A" || c.City = "Paris"); select c.Name }) + (customers >> Seq.filter (fun c -> c.Name.StartsWith "A" || c.City = "Paris") >> Seq.map _.Name) + + Scenario.rows + "where on a nullable property" + (fun db -> query { for c in db.Customers do where (c.Registered.HasValue && c.Registered.Value.Year > 2020); select c.Name }) + (customers >> Seq.filter (fun c -> c.Registered.HasValue && c.Registered.Value.Year > 2020) >> Seq.map _.Name) + + Scenario.rows + "where with a nullable operator" + (fun db -> query { for l in db.Lines do where (l.Discount ?> 1); select l.Id }) + (lines >> Seq.filter (fun l -> l.Discount ?> 1) >> Seq.map _.Id) + + Scenario.rows + "if then" + (fun db -> query { for c in db.Customers do if c.Age > 26 then yield c.Name }) + (customers >> Seq.filter (fun c -> c.Age > 26) >> Seq.map _.Name) + + Scenario.rows + "sortBy, thenByDescending" + (fun db -> query { for c in db.Customers do sortBy c.City; thenByDescending c.Age; select c.Name }) + (customers >> Seq.sortBy (fun c -> c.City, -c.Age) >> Seq.map _.Name) + |> Scenario.ordered + |> Scenario.flat [ "OrderBy"; "ThenByDescending"; "Select" ] + + Scenario.rows + "sortByDescending, thenBy" + (fun db -> query { for c in db.Customers do sortByDescending c.City; thenBy c.Age; select c.Name }) + (customers >> Seq.sortBy _.Age >> Seq.sortByDescending _.City >> Seq.map _.Name) + |> Scenario.ordered + |> Scenario.flat [ "OrderByDescending"; "ThenBy"; "Select" ] + + Scenario.rows + "sortByNullable, thenByNullableDescending" + (fun db -> query { for l in db.Lines do sortByNullable l.Discount; thenByNullableDescending (Nullable l.Id); select l.Id }) + (lines >> Seq.sortBy (fun l -> discountOrLowest l, -l.Id) >> Seq.map _.Id) + |> Scenario.ordered + |> Scenario.flat [ "OrderBy"; "ThenByDescending"; "Select" ] + + Scenario.rows + "sortByNullableDescending, thenByNullable" + (fun db -> query { for l in db.Lines do sortByNullableDescending l.Discount; thenByNullable (Nullable l.Id); select l.Id }) + (lines >> Seq.sortBy (fun l -> -(int64 (discountOrLowest l)), l.Id) >> Seq.map _.Id) + |> Scenario.ordered + |> Scenario.flat [ "OrderByDescending"; "ThenBy"; "Select" ] + + Scenario.rows "distinct" (fun db -> query { for c in db.Customers do select c.City; distinct }) (customers >> Seq.map _.City >> Seq.distinct) + + Scenario.rows + "skip, take" + (fun db -> query { for c in db.Customers do sortBy c.Id; skip 1; take 2; select c.Name }) + (customers >> Seq.skip 1 >> Seq.take 2 >> Seq.map _.Name) + |> Scenario.ordered + + Scenario.rows + "skipWhile, takeWhile" + (fun db -> query { for c in db.Customers do sortBy c.Id; skipWhile (c.Id < 2); takeWhile (c.Id < 4); select c.Name }) + (customers >> Seq.skipWhile (fun c -> c.Id < 2) >> Seq.takeWhile (fun c -> c.Id < 4) >> Seq.map _.Name) + |> Scenario.ordered + + Scenario.rows + "join" + (fun db -> query { for o in db.Orders do join l in db.Lines on (o.Id = l.OrderId); select (o.Id, l.Id) }) + (fun data -> seq { for o in data.Orders do for l in o.Lines -> o.Id, l.Id }) + + Scenario.rows + "groupJoin" + (fun db -> query { for o in db.Orders do groupJoin l in db.Lines on (o.Id = l.OrderId) into lines; select (o.Id, lines.Count()) }) + (fun data -> seq { for o in data.Orders -> o.Id, o.Lines.Count }) + + Scenario.rows + "leftOuterJoin" + (fun db -> + query { + for o in db.Orders do + leftOuterJoin l in db.Lines on (o.Id = l.OrderId) into lines + for l in lines do + select (o.Id, (if obj.ReferenceEquals(l, null) then -1 else l.Id)) + }) + (fun data -> + seq { + for o in data.Orders do + if o.Lines.Count = 0 then yield o.Id, -1 else for l in o.Lines -> o.Id, l.Id + }) + + Scenario.rows + "leftOuterJoin tested with isNull" + (fun db -> + query { + for o in db.Orders do + leftOuterJoin l in db.Lines on (o.Id = l.OrderId) into lines + for l in lines do + select (o.Id, isNull l) + }) + (fun data -> + seq { + for o in data.Orders do + if o.Lines.Count = 0 then yield o.Id, true else for _ in o.Lines -> o.Id, false + }) + |> Scenario.violating [ Violation.RuntimeHelperCall "Operators.IsNull" ] ] + +let private values () = + [ Scenario.value "count" (fun db -> query { for c in db.Customers do where (c.Age > 26); count }) (fun _ -> 3) + + Scenario.value "exists" (fun db -> query { for c in db.Customers do exists (c.City = "Paris") }) (fun _ -> true) + + Scenario.value "all" (fun db -> query { for c in db.Customers do all (c.Age > 26) }) (fun _ -> false) + + Scenario.value "contains" (fun db -> query { for c in db.Customers do select c.Id; contains 2 }) (fun _ -> true) + + Scenario.value "find" (fun db -> query { for c in db.Customers do find (c.Age < 30) }) (fun data -> data.Customers.[1]) + + Scenario.value "head" (fun db -> query { for c in db.Customers do sortBy c.Id; head }) (fun data -> data.Customers.[0]) + + Scenario.value + "headOrDefault" + (fun db -> query { for c in db.Customers do where (c.Age > 100); headOrDefault }) + (fun _ -> null) + + Scenario.value "last" (fun db -> query { for c in db.Customers do sortBy c.Id; last }) (fun data -> data.Customers.[3]) + + Scenario.value + "lastOrDefault" + (fun db -> query { for c in db.Customers do where (c.Age > 100); lastOrDefault }) + (fun _ -> null) + + Scenario.value + "exactlyOne" + (fun db -> query { for c in db.Customers do where (c.City = "Paris"); exactlyOne }) + (fun data -> data.Customers.[1]) + + Scenario.value + "exactlyOneOrDefault" + (fun db -> query { for c in db.Customers do where (c.City = "Rome"); exactlyOneOrDefault }) + (fun _ -> null) + + Scenario.value "nth" (fun db -> query { for c in db.Customers do sortBy c.Id; nth 2 }) (fun data -> data.Customers.[2]) + + Scenario.value "minBy" (fun db -> query { for c in db.Customers do minBy c.Age }) (customers >> Seq.map _.Age >> Seq.min) + + Scenario.value "maxBy" (fun db -> query { for c in db.Customers do maxBy c.Age }) (customers >> Seq.map _.Age >> Seq.max) + + Scenario.value "sumBy" (fun db -> query { for c in db.Customers do sumBy c.Age }) (customers >> Seq.sumBy _.Age) + + Scenario.value + "averageBy" + (fun db -> query { for c in db.Customers do averageBy (float c.Age) }) + (customers >> Seq.averageBy (fun c -> float c.Age)) + + Scenario.value + "minByNullable" + (fun db -> query { for l in db.Lines do minByNullable l.Discount }) + (discounts >> List.min >> Nullable) + + Scenario.value + "maxByNullable" + (fun db -> query { for l in db.Lines do maxByNullable l.Discount }) + (discounts >> List.max >> Nullable) + + Scenario.value + "sumByNullable" + (fun db -> query { for l in db.Lines do sumByNullable l.Discount }) + (discounts >> List.sum >> Nullable) + + Scenario.value + "averageByNullable" + (fun db -> query { for l in db.Lines do averageByNullable (Nullable.float l.Discount) }) + (discounts >> List.averageBy float >> Nullable) ] + +let cases () = sequences () @ values () diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs new file mode 100644 index 00000000000..44d598d3d13 --- /dev/null +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs @@ -0,0 +1,130 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Core.UnitTests.Linq.QueryProjectionScenarios + +open System.Linq.Expressions + +let private block = [ Violation.Statement ExpressionType.Block ] + +let private customers (data: Data) = Seq.ofList data.Customers + +let cases () = + [ Scenario.rows "scalar" (fun db -> query { for c in db.Customers do select c.Name }) (customers >> Seq.map _.Name) + + Scenario.rows "entity" (fun db -> query { for c in db.Customers do select c }) customers + + Scenario.rows + "computed scalars" + (fun db -> query { for c in db.Customers do select (c.Name + "!", c.Age + 1, (if c.Age > 28 then "senior" else "junior")) }) + (customers >> Seq.map (fun c -> c.Name + "!", c.Age + 1, (if c.Age > 28 then "senior" else "junior"))) + + Scenario.rows + "tuple of two" + (fun db -> query { for c in db.Customers do select (c.Id, c.Name) }) + (customers >> Seq.map (fun c -> c.Id, c.Name)) + + Scenario.rows + "tuple of seven" + (fun db -> query { for c in db.Customers do select (c.Id, c.Name, c.City, c.Age, c.Id + 100, c.Name.Length, c.Age * 2) }) + (customers >> Seq.map (fun c -> c.Id, c.Name, c.City, c.Age, c.Id + 100, c.Name.Length, c.Age * 2)) + + Scenario.rows + "tuple of eight" + (fun db -> query { for c in db.Customers do select (c.Id, c.Name, c.City, c.Age, c.Id + 100, c.Name.Length, c.Age * 2, c.Id * 7) }) + (customers >> Seq.map (fun c -> c.Id, c.Name, c.City, c.Age, c.Id + 100, c.Name.Length, c.Age * 2, c.Id * 7)) + + Scenario.rows + "tuple of nine" + (fun db -> query { for c in db.Customers do select (c.Id, c.Name, c.City, c.Age, c.Id + 100, c.Name.Length, c.Age * 2, c.Id * 7, c.Age - c.Id) }) + (customers >> Seq.map (fun c -> c.Id, c.Name, c.City, c.Age, c.Id + 100, c.Name.Length, c.Age * 2, c.Id * 7, c.Age - c.Id)) + + Scenario.rows + "nested tuple" + (fun db -> query { for c in db.Customers do select ((c.Id, c.Name), c.Age) }) + (customers >> Seq.map (fun c -> (c.Id, c.Name), c.Age)) + + Scenario.rows + "tuple taken apart by the next for" + (fun db -> + query { + for (id, name) in (query { for c in db.Customers do select (c.Id, c.Name) }) do + where (id > 1) + select name + }) + (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) + |> Scenario.violating [ Violation.ReadsUnnamedMember "Tuple`2" ] + + Scenario.rows + "record, fields in declaration order" + (fun db -> query { for c in db.Customers do select { PersonName = c.Name; PersonAge = c.Age } }) + (customers >> Seq.map (fun c -> { PersonName = c.Name; PersonAge = c.Age })) + + Scenario.rows + "record, fields out of declaration order" + (fun db -> query { for c in db.Customers do select { PersonAge = c.Age; PersonName = c.Name } }) + (customers >> Seq.map (fun c -> { PersonName = c.Name; PersonAge = c.Age })) + |> Scenario.violating block + + Scenario.rows + "record of three, fields rotated" + (fun db -> query { for c in db.Customers do select { Town = c.City; FullName = c.Name; Years = c.Age } }) + (customers >> Seq.map (fun c -> { FullName = c.Name; Years = c.Age; Town = c.City })) + |> Scenario.violating block + + Scenario.rows + "record holding a tuple" + (fun db -> query { for c in db.Customers do select {| Age = c.Age; Pair = (c.Id, c.Name) |} }) + (customers >> Seq.map (fun c -> {| Age = c.Age; Pair = (c.Id, c.Name) |})) + + Scenario.rows + "anonymous record, fields in alphabetical order" + (fun db -> query { for c in db.Customers do select {| Age = c.Age; Name = c.Name |} }) + (customers >> Seq.map (fun c -> {| Age = c.Age; Name = c.Name |})) + + Scenario.rows + "anonymous record, fields out of alphabetical order" + (fun db -> query { for c in db.Customers do select {| Name = c.Name; Age = c.Age |} }) + (customers >> Seq.map (fun c -> {| Age = c.Age; Name = c.Name |})) + |> Scenario.violating block + + Scenario.rows + "anonymous record, method calls in fields out of order" + (fun db -> query { for c in db.Customers do select {| Upper = c.Name.ToUpper(); City = c.City.Trim() |} }) + (customers >> Seq.map (fun c -> {| City = c.City.Trim(); Upper = c.Name.ToUpper() |})) + |> Scenario.violating block + + Scenario.rows + "anonymous record inside an anonymous record" + (fun db -> query { for c in db.Customers do select {| Person = {| Name = c.Name; Age = c.Age |} |} }) + (customers >> Seq.map (fun c -> {| Person = {| Age = c.Age; Name = c.Name |} |})) + |> Scenario.violating block + + Scenario.rows + "struct anonymous record, fields in alphabetical order" + (fun db -> query { for c in db.Customers do select (struct {| Age = c.Age; Name = c.Name |}) }) + (customers >> Seq.map (fun c -> struct {| Age = c.Age; Name = c.Name |})) + + Scenario.rows + "struct anonymous record, fields out of alphabetical order" + (fun db -> query { for c in db.Customers do select (struct {| Name = c.Name; Age = c.Age |}) }) + (customers >> Seq.map (fun c -> struct {| Age = c.Age; Name = c.Name |})) + |> Scenario.violating block + + Scenario.rows + "CLIMutable record, fields in declaration order" + (fun db -> query { for c in db.Customers do select { DtoName = c.Name; DtoAge = c.Age } }) + (customers >> Seq.map (fun c -> { DtoName = c.Name; DtoAge = c.Age })) + + Scenario.rows + "CLIMutable record, fields out of declaration order" + (fun db -> query { for c in db.Customers do select { DtoAge = c.Age; DtoName = c.Name } }) + (customers >> Seq.map (fun c -> { DtoName = c.Name; DtoAge = c.Age })) + |> Scenario.violating block + + Scenario.rows + "class with property initializers" + (fun db -> query { for c in db.Customers do select (CustomerView(ViewName = c.Name, ViewAge = c.Age)) }) + (customers >> Seq.map (fun c -> CustomerView(ViewName = c.Name, ViewAge = c.Age))) + + Scenario.rows "option" (fun db -> query { for c in db.Customers do select (Some c.Age) }) (customers >> Seq.map (fun c -> Some c.Age)) + |> Scenario.violating [ Violation.RuntimeHelperCall "FSharpOption`1.Some" ] ] diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs new file mode 100644 index 00000000000..bad748d75ab --- /dev/null +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs @@ -0,0 +1,355 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +namespace FSharp.Core.UnitTests.Linq + +open System +open System.Collections +open System.Collections.Generic +open System.Linq +open System.Linq.Expressions +open System.Reflection +open Microsoft.FSharp.Reflection + +type ICapturedQuery = + abstract InMemory: IQueryable + +// EnumerableQuery turns Queryable calls into Enumerable ones only over its own constants; +// over any other IQueryable it calls Queryable again, which here would never end. +type private InMemoryRewriter() = + inherit ExpressionVisitor() + + override _.VisitConstant node = + match node.Value with + | :? ICapturedQuery as captured -> Expression.Constant captured.InMemory + | _ -> node + +/// An in-memory query that stays on the provider that created it and prints its root by name. +[] +type CapturedQuery<'T>(provider: IQueryProvider, name: string, items: seq<'T>, derived: Expression voption) = + + let enumerate () = + match derived with + | ValueSome expression -> provider.Execute>(expression).GetEnumerator() + | ValueNone -> items.GetEnumerator() + + override _.ToString() = + match derived with + | ValueSome expression -> expression.ToString() + | ValueNone -> name + + interface ICapturedQuery with + member _.InMemory = + match derived with + | ValueSome expression -> EnumerableQuery<'T>(InMemoryRewriter().Visit expression) + | ValueNone -> EnumerableQuery<'T> items + + interface IOrderedQueryable<'T> with + member this.Expression = + match derived with + | ValueSome expression -> expression + | ValueNone -> Expression.Constant this + + member _.ElementType = typeof<'T> + member _.Provider = provider + member _.GetEnumerator() : IEnumerator<'T> = enumerate () + member _.GetEnumerator() : IEnumerator = enumerate () + +/// Executes in memory, and records what it was asked to execute. +[] +type CapturingQueryProvider() = + let executed = ResizeArray() + let mutable depth = 0 + + let inMemory () = + EnumerableQuery Seq.empty :> IQueryProvider + + let track (expression: Expression) (run: Expression -> 'TResult) = + if depth = 0 then + executed.Add expression + + depth <- depth + 1 + + try + run (InMemoryRewriter().Visit expression) + finally + depth <- depth - 1 + + /// The expressions executed directly, not from inside another execution. + member _.Executed = executed :> IReadOnlyList + + member this.Source(name: string, items: seq<'T>) : IQueryable<'T> = + CapturedQuery<'T>(this, name, items, ValueNone) + + member this.Derived<'T>(expression: Expression) : IQueryable<'T> = + CapturedQuery<'T>(this, "", Seq.empty, ValueSome expression) + + interface IQueryProvider with + member this.CreateQuery<'T>(expression: Expression) : IQueryable<'T> = this.Derived<'T> expression + + member this.CreateQuery(expression: Expression) : IQueryable = + let elementType = + expression.Type.GetInterfaces() + |> Array.append [| expression.Type |] + |> Array.find (fun ty -> ty.IsGenericType && ty.GetGenericTypeDefinition() = typedefof>) + |> fun ty -> ty.GetGenericArguments().[0] + + typeof + .GetMethod(nameof Unchecked.defaultof.Derived) + .MakeGenericMethod(elementType) + .Invoke(this, [| expression |]) + :?> IQueryable + + member _.Execute<'TResult>(expression: Expression) : 'TResult = + track expression (inMemory ()).Execute<'TResult> + + member _.Execute(expression: Expression) : obj = + track expression (inMemory ()).Execute + +/// Something in an expression tree that a C# expression lambda never contains, so no LINQ provider translates it. +[] +type Violation = + | Statement of ExpressionType + | LambdaInvocation + | UnboundParameter of name: string + | RuntimeHelperCall of methodName: string + | NestedInCollectionSelector of methodName: string + | ReadsUnnamedMember of typeName: string + | LeftProvider + | Throws of exceptionType: string + +module ProviderShape = + + let private fsharpCore = typeof.Assembly + + let private allMembers = BindingFlags.Public ||| BindingFlags.NonPublic + + let private isLinqOperator (call: MethodCallExpression) = + call.Arguments.Count > 0 + && (call.Method.DeclaringType = typeof + || call.Method.DeclaringType = typeof) + + let private isStatement nodeType = + match nodeType with + | ExpressionType.Block + | ExpressionType.Assign + | ExpressionType.Loop + | ExpressionType.Goto + | ExpressionType.Label + | ExpressionType.Try + | ExpressionType.Switch + | ExpressionType.RuntimeVariables + | ExpressionType.DebugInfo -> true + | _ -> false + + let private flattensOrJoins (call: MethodCallExpression) = + isLinqOperator call + && (match call.Method.Name with + | "SelectMany" + | "Join" + | "GroupJoin" -> true + | _ -> false) + + type private ShapeVisitor() = + inherit ExpressionVisitor() + + let found = ResizeArray() + let bound = HashSet(HashIdentity.Reference) + let mutable collectionSelectorDepth = 0 + + let bind (parameters: seq) = + parameters |> Seq.filter bound.Add |> Seq.toArray + + let unbind (parameters: ParameterExpression array) = + for parameter in parameters do + bound.Remove parameter |> ignore + + [] + let (|AssignsBound|_|) (expression: Expression) = + match expression with + | :? BinaryExpression as assign when assign.NodeType = ExpressionType.Assign -> + match assign.Left with + | :? ParameterExpression as variable when bound.Contains variable -> ValueSome assign.Right + | _ -> ValueNone + | _ -> ValueNone + + member _.Violations = found |> Seq.distinct |> List.ofSeq + + override _.VisitLambda<'T>(node: Expression<'T>) = + let added = bind node.Parameters + + try + base.VisitLambda node + finally + unbind added + + // A block is reported once: its variables and their initial assignments are part of it. + override this.VisitBlock node = + found.Add(Violation.Statement ExpressionType.Block) + let added = bind node.Variables + + try + for expression in node.Expressions do + match expression with + | AssignsBound value -> this.Visit value |> ignore + | expression -> this.Visit expression |> ignore + + node + finally + unbind added + + override _.VisitParameter node = + if not (bound.Contains node) then + found.Add(Violation.UnboundParameter node.Name) + + node + + override _.VisitInvocation node = + found.Add Violation.LambdaInvocation + base.VisitInvocation node + + // A query built earlier and used in this one is part of what the provider translates. + override this.VisitConstant node = + match node.Value with + | :? IQueryable as query -> + match query.Expression with + | :? ConstantExpression as root when obj.ReferenceEquals(root.Value, query) -> () + | built -> this.Visit built |> ignore + | _ -> () + + node + + override _.VisitNew node = + if node.Type.Assembly = fsharpCore && not (node.Type.Name.StartsWith("AnonymousObject", StringComparison.Ordinal)) then + found.Add(Violation.RuntimeHelperCall $"new {node.Type.Name}") + + base.VisitNew node + + override this.VisitMethodCall node = + if node.Method.DeclaringType.Assembly = fsharpCore then + found.Add(Violation.RuntimeHelperCall $"{node.Method.DeclaringType.Name}.{node.Method.Name}") + + if collectionSelectorDepth > 0 && flattensOrJoins node then + found.Add(Violation.NestedInCollectionSelector node.Method.Name) + + if isLinqOperator node && node.Method.Name = "SelectMany" then + this.Visit node.Object |> ignore + this.Visit node.Arguments.[0] |> ignore + collectionSelectorDepth <- collectionSelectorDepth + 1 + + try + this.Visit node.Arguments.[1] |> ignore + finally + collectionSelectorDepth <- collectionSelectorDepth - 1 + + for argument in Seq.skip 2 node.Arguments do + this.Visit argument |> ignore + + node + else + base.VisitMethodCall node + + override this.Visit node = + match node with + | null -> null + | node -> + if node.NodeType <> ExpressionType.Block && isStatement node.NodeType then + found.Add(Violation.Statement node.NodeType) + + base.Visit node + + [] + let private (|Lambda|_|) (argument: Expression) = + match argument with + | :? UnaryExpression as quote when quote.NodeType = ExpressionType.Quote -> ValueSome(quote.Operand :?> LambdaExpression) + | :? LambdaExpression as lambda -> ValueSome lambda + | _ -> ValueNone + + /// A tuple or record built without saying which member each argument becomes. + let private buildsUnnamed (body: Expression) = + match body with + | :? NewExpression as construction when construction.Arguments.Count > 0 -> + match construction.Members with + | null -> + FSharpType.IsTuple construction.Type + || FSharpType.IsRecord(construction.Type, allMembers) + | _ -> false + | _ -> false + + type private MemberReadFinder(owner: ParameterExpression) = + inherit ExpressionVisitor() + + member val Found = false with get, set + + override this.VisitMember node = + if obj.ReferenceEquals(node.Expression, owner) then + this.Found <- true + + base.VisitMember node + + let private readsMemberOf (owner: ParameterExpression) (body: Expression) = + let finder = MemberReadFinder owner + finder.Visit body |> ignore + finder.Found + + let private yields (elementType: Type) (source: Expression) = + typedefof>.MakeGenericType(elementType).IsAssignableFrom source.Type + + /// Members read from elements that an earlier operator built without naming them: + /// the provider has to take apart an object it was not told how to. + let private unnamedMemberReads (expression: Expression) = + let found = ResizeArray() + + let rec producesUnnamed (expression: Expression) = + match expression with + | :? MethodCallExpression as call when isLinqOperator call -> + let source = call.Arguments.[0] + let sourceUnnamed = producesUnnamed source + + let lambdas = + [ for argument in Seq.skip 1 call.Arguments do + match argument with + | Lambda lambda -> lambda + | _ -> () ] + + if sourceUnnamed then + for lambda in lambdas do + let element = lambda.Parameters.[0] + + if yields element.Type source && readsMemberOf element lambda.Body then + found.Add(Violation.ReadsUnnamedMember element.Type.Name) + + match call.Method.Name with + | "Select" + | "SelectMany" + | "Join" + | "GroupJoin" -> lambdas |> List.last |> _.Body |> buildsUnnamed + | "Where" + | "OrderBy" + | "OrderByDescending" + | "ThenBy" + | "ThenByDescending" + | "Skip" + | "SkipWhile" + | "Take" + | "TakeWhile" + | "Distinct" + | "DefaultIfEmpty" -> sourceUnnamed + | _ -> false + | _ -> false + + producesUnnamed expression |> ignore + List.ofSeq found + + let violations (expression: Expression) = + let visitor = ShapeVisitor() + visitor.Visit expression |> ignore + visitor.Violations @ unnamedMemberReads expression |> List.distinct + + /// The LINQ operators along the source chain, outermost last. + let spine (expression: Expression) = + let rec walk (expression: Expression) operators = + match expression with + | :? MethodCallExpression as call when isLinqOperator call -> walk call.Arguments.[0] (call.Method.Name :: operators) + | _ -> operators + + walk expression [] diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs new file mode 100644 index 00000000000..b2a270042e9 --- /dev/null +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs @@ -0,0 +1,189 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +namespace FSharp.Core.UnitTests.Linq + +open System +open System.Collections +open System.Globalization +open System.Linq +open System.Linq.Expressions +open System.Reflection +open Microsoft.FSharp.Reflection + +[] +type Outcome = + | Rows of IQueryable + | Value of obj + +/// One query, what it must return, and what is known to be wrong with its expression tree today. +[] +type Scenario = + { Name: string + Run: Db -> Outcome + Expected: Data -> obj list + Ordered: bool + Spine: string list voption + Violations: Violation list } + + override this.ToString() = this.Name + +module Scenario = + + let private allMembers = BindingFlags.Public ||| BindingFlags.NonPublic + + [] + let private (|Grouping|_|) (value: obj) = + match value.GetType().GetInterface(typedefof>.Name) with + | null -> ValueNone + | grouping -> ValueSome(grouping.GetProperty("Key").GetValue value) + + [] + let private (|Tuple|_|) (value: obj) = + if FSharpType.IsTuple(value.GetType()) then + ValueSome(FSharpValue.GetTupleFields value) + else + ValueNone + + [] + let private (|Record|_|) (value: obj) = + if FSharpType.IsRecord(value.GetType(), allMembers) then + let names = FSharpType.GetRecordFields(value.GetType(), allMembers) |> Seq.map _.Name + ValueSome(Seq.zip names (FSharpValue.GetRecordFields(value, allMembers))) + else + ValueNone + + [] + let private (|Union|_|) (value: obj) = + if FSharpType.IsUnion(value.GetType(), allMembers) then + let case, fields = FSharpValue.GetUnionFields(value, value.GetType(), allMembers) + ValueSome struct (case.Name, fields) + else + ValueNone + + /// Text that is equal for equal results, whichever provider materialized them. + let rec render (value: obj) = + let renderAll (values: seq) = + values |> Seq.map render |> String.concat "; " + + match value with + | null -> "null" + | :? string as text -> $"\"{text}\"" + | :? IFormattable as number -> number.ToString(null, CultureInfo.InvariantCulture) + | Grouping key -> $"{render key} => [{renderAll (Seq.cast (value :?> IEnumerable))}]" + | :? IEnumerable as items -> $"[{renderAll (Seq.cast items)}]" + | Tuple elements -> $"({renderAll elements})" + | Record fields -> + let rendered = fields |> Seq.map (fun (name, field) -> $"{name} = {render field}") + $"""{{ {String.concat "; " rendered} }}""" + | Union(case, fields) -> $"{case}({renderAll fields})" + | value -> string value + + let rows name (run: Db -> IQueryable<'T>) (expected: Data -> seq<'T>) = + { Name = name + Run = fun db -> Outcome.Rows(run db) + Expected = fun data -> [ for row in expected data -> box row ] + Ordered = false + Spine = ValueNone + Violations = [] } + + let value name (run: Db -> 'T) (expected: Data -> 'T) = + { Name = name + Run = fun db -> Outcome.Value(box (run db)) + Expected = fun data -> [ box (expected data) ] + Ordered = true + Spine = ValueNone + Violations = [] } + + let ordered scenario = { scenario with Ordered = true } + + /// The operators the source chain must consist of, outermost last. + let flat spine scenario = + { scenario with + Spine = ValueSome spine } + + /// What is wrong with the expression tree today; the scenario fails once that stops being true. + let violating violations scenario = + { scenario with + Violations = violations } + + [] + type private Observed = + { Expression: Expression + Violations: Violation list + Rows: obj list } + + let rec private rootCause (error: exn) = + match error, error.InnerException with + | :? TargetInvocationException, null -> error + | :? TargetInvocationException, inner -> rootCause inner + | _ -> error + + let private observe (scenario: Scenario) = + let provider = CapturingQueryProvider() + let data = Data() + + let observed = + try + match scenario.Run(Db.inMemory provider data) with + | Outcome.Rows rows -> + let stayed = obj.ReferenceEquals(rows.Provider, provider) + + { Expression = rows.Expression + Violations = (if stayed then [] else [ Violation.LeftProvider ]) + Rows = [ for row in rows -> row ] } + // The value comes from the last execution; whatever ran before it went to the provider too. + | Outcome.Value value when provider.Executed.Count > 0 -> + let earlier = provider.Executed |> Seq.take (provider.Executed.Count - 1) + + { Expression = provider.Executed.[provider.Executed.Count - 1] + Violations = [ for expression in earlier do yield! ProviderShape.violations expression ] + Rows = [ value ] } + | Outcome.Value value -> + { Expression = Expression.Empty() + Violations = [ Violation.LeftProvider ] + Rows = [ value ] } + with error -> + { Expression = Expression.Empty() + Violations = [ Violation.Throws((rootCause error).GetType().Name) ] + Rows = [] } + + data, observed + + let private inOrder ordered (rows: obj list) = + let rendered = List.map render rows + if ordered then rendered else List.sort rendered + + /// Everything the scenario gets wrong, empty when it behaves as described. + let failures (scenario: Scenario) = + let data, observed = observe scenario + + let violations = + observed.Violations @ ProviderShape.violations observed.Expression + |> List.distinct + + let threw = + violations + |> List.exists (function + | Violation.Throws _ -> true + | _ -> false) + + [ if set violations <> set scenario.Violations then + $"violations: expected %A{scenario.Violations}, got %A{violations}" + + if not threw then + let expected = inOrder scenario.Ordered (scenario.Expected data) + let actual = inOrder scenario.Ordered observed.Rows + + if expected <> actual then + $"rows: expected %A{expected}, got %A{actual}" + + match scenario.Spine with + | ValueSome spine when spine <> ProviderShape.spine observed.Expression -> + $"operators: expected %A{spine}, got %A{ProviderShape.spine observed.Expression}" + | _ -> () ] + |> List.map (fun failure -> $"{scenario.Name}: {failure}\n in {observed.Expression}") + + let check scenario = + match failures scenario with + | [] -> () + | failures -> failwith (String.concat "\n" failures) diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenarioDomain.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenarioDomain.fs new file mode 100644 index 00000000000..ada7f2ba70f --- /dev/null +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenarioDomain.fs @@ -0,0 +1,109 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +namespace FSharp.Core.UnitTests.Linq + +open System +open System.Collections.Generic +open System.Linq + +[] +type Product() = + member val Id = 0 with get, set + member val Name = "" with get, set + member val Category = "" with get, set + override this.ToString() = $"Product#{this.Id}" + +[] +type OrderLine() = + member val Id = 0 with get, set + member val OrderId = 0 with get, set + member val ProductId = 0 with get, set + member val Quantity = 0 with get, set + member val Price = 0.0 with get, set + member val Discount = Nullable() with get, set + override this.ToString() = $"OrderLine#{this.Id}" + +[] +type Order() = + member val Id = 0 with get, set + member val CustomerId = 0 with get, set + member val Total = 0.0 with get, set + member val Lines: ICollection = List() with get, set + override this.ToString() = $"Order#{this.Id}" + +[] +type Customer() = + member val Id = 0 with get, set + member val Name = "" with get, set + member val City = "" with get, set + member val Age = 0 with get, set + member val Registered = Nullable() with get, set + member val Orders: ICollection = List() with get, set + override this.ToString() = $"Customer#{this.Id}" + +type CustomerView() = + member val ViewName = "" with get, set + member val ViewAge = 0 with get, set + override this.ToString() = $"CustomerView({this.ViewName}, {this.ViewAge})" + +type NameAndAge = { PersonName: string; PersonAge: int } + +type NameAgeCity = + { FullName: string + Years: int + Town: string } + +[] +type NameAndAgeDto = { DtoName: string; DtoAge: int } + +type CityAndAge = { KeyCity: string; KeyAge: int } + +type OrderAndQuantity = { OrderNumber: int; Units: int } + +type Data() = + let line id orderId productId quantity price discount = + OrderLine(Id = id, OrderId = orderId, ProductId = productId, Quantity = quantity, Price = price, Discount = discount) + + let order id customerId total (lines: OrderLine list) = + Order(Id = id, CustomerId = customerId, Total = total, Lines = List lines) + + let customer id name city age registered (orders: Order list) = + Customer(Id = id, Name = name, City = city, Age = age, Registered = registered, Orders = List orders) + + let orders = + [ order 10 1 12.5 [ line 100 10 1 2 2.5 (Nullable 1); line 101 10 2 1 7.5 (Nullable()) ] + order 11 1 3.0 [ line 102 11 3 1 3.0 (Nullable 2) ] + order 12 2 5.0 [ line 103 12 1 2 2.5 (Nullable()) ] + order 13 3 0.0 [] ] + + let ordersOf customerId = + orders |> List.filter (fun order -> order.CustomerId = customerId) + + member val Products = + [ Product(Id = 1, Name = "Pen", Category = "Office") + Product(Id = 2, Name = "Ink", Category = "Office") + Product(Id = 3, Name = "Tea", Category = "Food") ] + + member val Customers = + [ customer 1 "Alice" "London" 30 (Nullable(DateTime(2020, 1, 1))) (ordersOf 1) + customer 2 "Bob" "Paris" 25 (Nullable()) (ordersOf 2) + customer 3 "Carol" "London" 35 (Nullable(DateTime(2021, 6, 15))) (ordersOf 3) + customer 4 "Dave" "Berlin" 28 (Nullable()) [] ] + + member _.Orders = orders + member _.Lines = orders |> List.collect (fun order -> List.ofSeq order.Lines) + +[] +type Db = + { Customers: IQueryable + Orders: IQueryable + Lines: IQueryable + Products: IQueryable } + +module Db = + + let inMemory (provider: CapturingQueryProvider) (data: Data) = + { Customers = provider.Source("Customers", data.Customers) + Orders = provider.Source("Orders", data.Orders) + Lines = provider.Source("Lines", data.Lines) + Products = provider.Source("Products", data.Products) } diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs new file mode 100644 index 00000000000..6e284ed7d4d --- /dev/null +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs @@ -0,0 +1,31 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +namespace FSharp.Core.UnitTests.Linq + +open Xunit + +type QueryShapeTests() = + + static let rows (cases: Scenario list) = + cases |> Seq.map (fun scenario -> [| box scenario |]) + + static member Projections = rows (QueryProjectionScenarios.cases ()) + static member Operators = rows (QueryOperatorScenarios.cases ()) + static member Keys = rows (QueryKeyScenarios.cases ()) + static member Structures = rows (QueryStructureScenarios.cases ()) + static member MethodSyntax = rows (QueryMethodSyntaxScenarios.cases ()) + + [] + member _.Projection(scenario: Scenario) = Scenario.check scenario + + [] + member _.Operator(scenario: Scenario) = Scenario.check scenario + + [] + member _.Key(scenario: Scenario) = Scenario.check scenario + + [] + member _.Structure(scenario: Scenario) = Scenario.check scenario + + [] + member _.``Method syntax``(scenario: Scenario) = Scenario.check scenario diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs new file mode 100644 index 00000000000..6f806f4bb43 --- /dev/null +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs @@ -0,0 +1,268 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Core.UnitTests.Linq.QueryStructureScenarios + +open System.Linq + +let private quotedLambda = + [ Violation.RuntimeHelperCall "LeafExpressionConverter.QuotationToLambdaExpression" + Violation.RuntimeHelperCall "LeafExpressionConverter.SubstHelper" ] + +let private nested operators = + operators |> List.map Violation.NestedInCollectionSelector + +let private orDefault (items: seq<'T>) = + if Seq.isEmpty items then Seq.singleton Unchecked.defaultof<'T> else items + +let private idOrNone (order: Order) = + match order with + | null -> -1 + | order -> order.Id + +let private nameOrNone (product: Product) = + match product with + | null -> "" + | product -> product.Name + +let private ordersWithLines (data: Data) = + seq { for c in data.Customers do for o in c.Orders do for l in o.Lines -> c.Name, o.Id, l.Id } + +let private customersWithOrders (data: Data) = + seq { for c in data.Customers do for o in c.Orders -> c.Name, o.Id } + +let private leftJoinedOrders (data: Data) = + seq { for c in data.Customers do for o in orDefault c.Orders -> c.Name, idOrNone o } + +let private leftJoinedOrdersAndProducts (data: Data) = + seq { + for c in data.Customers do + for o in orDefault c.Orders do + for p in orDefault (data.Products |> List.filter (fun p -> p.Id = c.Id)) -> c.Name, idOrNone o, nameOrNone p + } + +let private loops () = + [ Scenario.rows + "two for" + (fun db -> query { for c in db.Customers do for p in db.Products do select (c.Name, p.Name) }) + (fun data -> seq { for c in data.Customers do for p in data.Products -> c.Name, p.Name }) + |> Scenario.flat [ "SelectMany"; "Select"; "Select" ] + + Scenario.rows + "two for, where over both" + (fun db -> query { for c in db.Customers do for o in db.Orders do where (o.CustomerId = c.Id); select (c.Name, o.Id) }) + customersWithOrders + |> Scenario.flat [ "SelectMany"; "Where"; "Select"; "Select" ] + + Scenario.rows + "for, where, for, where" + (fun db -> + query { + for c in db.Customers do + where (c.Age > 26) + for o in db.Orders do + where (o.CustomerId = c.Id) + select (c.Name, o.Id) + }) + (fun data -> seq { for c in data.Customers do if c.Age > 26 then for o in c.Orders -> c.Name, o.Id }) + |> Scenario.flat [ "Where"; "SelectMany"; "Where"; "Select"; "Select" ] + + Scenario.rows + "for over a navigation collection" + (fun db -> query { for c in db.Customers do for o in c.Orders do select (c.Name, o.Id) }) + customersWithOrders + |> Scenario.flat [ "SelectMany"; "Select"; "Select" ] + + Scenario.rows + "for over two levels of navigation collections" + (fun db -> query { for c in db.Customers do for o in c.Orders do for l in o.Lines do select (c.Name, o.Id, l.Id) }) + ordersWithLines + |> Scenario.violating (nested [ "SelectMany" ]) + + Scenario.rows + "three for" + (fun db -> + query { + for c in db.Customers do + for o in db.Orders do + for l in db.Lines do + where (o.CustomerId = c.Id && l.OrderId = o.Id) + select (c.Name, o.Id, l.Id) + }) + ordersWithLines + |> Scenario.violating (nested [ "SelectMany" ]) + + Scenario.rows + "for after sortBy" + (fun db -> query { for c in db.Customers do sortBy c.Age; for p in db.Products do select (c.Name, p.Name) }) + (fun data -> seq { for c in data.Customers |> List.sortBy _.Age do for p in data.Products -> c.Name, p.Name }) + |> Scenario.ordered + |> Scenario.flat [ "OrderBy"; "SelectMany"; "Select"; "Select" ] + + Scenario.rows + "for after join" + (fun db -> + query { + for c in db.Customers do + join o in db.Orders on (c.Id = o.CustomerId) + for l in db.Lines do + where (l.OrderId = o.Id) + select (c.Name, o.Id, l.Id) + }) + ordersWithLines + |> Scenario.flat [ "Join"; "SelectMany"; "Where"; "Select"; "Select" ] + + Scenario.rows + "let between for" + (fun db -> + query { for c in db.Customers do let next = c.Id + 9 in for o in db.Orders do where (o.Id = next); select (c.Name, o.Id) }) + (fun data -> seq { for c in data.Customers do for o in data.Orders do if o.Id = c.Id + 9 then yield c.Name, o.Id }) + |> Scenario.flat [ "SelectMany"; "Where"; "Select"; "Select" ] + + Scenario.rows + "if then around for" + (fun db -> query { for c in db.Customers do if c.Age > 26 then for o in c.Orders do yield (c.Name, o.Id) }) + (fun data -> seq { for c in data.Customers do if c.Age > 26 then for o in c.Orders -> c.Name, o.Id }) + |> Scenario.flat [ "Where"; "SelectMany"; "Select" ] + + Scenario.rows + "for around if then" + (fun db -> query { for c in db.Customers do for o in db.Orders do if o.CustomerId = c.Id then yield (c.Name, o.Id) }) + customersWithOrders + |> Scenario.flat [ "SelectMany"; "Select" ] + + Scenario.rows + "yield! of a navigation collection" + (fun db -> query { for c in db.Customers do yield! c.Orders }) + (fun data -> Seq.ofList data.Orders) + |> Scenario.flat [ "SelectMany" ] + + Scenario.rows + "distinct and take after two for" + (fun db -> query { for c in db.Customers do for o in c.Orders do select c.City; distinct; take 1 }) + (fun _ -> seq { "London" }) + |> Scenario.ordered + |> Scenario.flat [ "SelectMany"; "Select"; "Distinct"; "Take" ] + + Scenario.rows + "groupBy after two for" + (fun db -> query { for c in db.Customers do for o in c.Orders do groupBy c.City into g; select (g.Key, g.Count()) }) + (fun data -> seq { for c in data.Customers do for _ in c.Orders -> c.City } |> Seq.countBy id) + |> Scenario.violating [ Violation.RuntimeHelperCall "new Grouping`2" ] + + Scenario.value "count over two for" (fun db -> query { for c in db.Customers do for o in c.Orders do count }) (fun _ -> 4) + + Scenario.value + "exists over two for" + (fun db -> query { for c in db.Customers do for o in c.Orders do exists (o.Total > 10.0 && c.City = "London") }) + (fun _ -> true) + |> Scenario.violating [ Violation.ReadsUnnamedMember "Tuple`2" ] ] + +let private subqueries () = + [ Scenario.rows + "nested query as the source" + (fun db -> query { for o in (query { for o in db.Orders do where (o.Total > 4.0); select o }) do select o.Id }) + (fun data -> seq { for o in data.Orders do if o.Total > 4.0 then yield o.Id }) + |> Scenario.flat [ "Where"; "Select" ] + + Scenario.rows + "nested query as the source of the second for" + (fun db -> + query { + for c in db.Customers do + for o in (query { for o in db.Orders do where (o.CustomerId = c.Id); select o }) do + select (c.Name, o.Id) + }) + customersWithOrders + |> Scenario.flat [ "SelectMany"; "Select"; "Select" ] + + Scenario.rows + "nested query in select" + (fun db -> + query { for c in db.Customers do select (c.Name, query { for o in db.Orders do where (o.CustomerId = c.Id); select o.Id }) }) + (fun data -> seq { for c in data.Customers -> c.Name, (seq { for o in c.Orders -> o.Id }).AsQueryable() }) + + Scenario.rows + "second for over a source filtered by a lambda" + (fun db -> query { for c in db.Customers do for o in db.Orders.Where(fun o -> o.CustomerId = c.Id) do select (c.Name, o.Id) }) + customersWithOrders + |> Scenario.violating quotedLambda + + Scenario.rows + "where with a lambda over another source" + (fun db -> query { for c in db.Customers do where (db.Orders.Any(fun o -> o.CustomerId = c.Id)); select c.Name }) + (fun data -> seq { for c in data.Customers do if c.Orders.Count > 0 then yield c.Name }) + |> Scenario.violating quotedLambda ] + +let private leftJoins () = + [ Scenario.rows + "left join spelled leftOuterJoin" + (fun db -> + query { + for c in db.Customers do + leftOuterJoin o in db.Orders on (c.Id = o.CustomerId) into orders + for o in orders do + select (c.Name, (if obj.ReferenceEquals(o, null) then -1 else o.Id)) + }) + leftJoinedOrders + |> Scenario.flat [ "GroupJoin"; "SelectMany"; "Select"; "Select" ] + + Scenario.rows + "left join spelled groupJoin and DefaultIfEmpty" + (fun db -> + query { + for c in db.Customers do + groupJoin o in db.Orders on (c.Id = o.CustomerId) into orders + for o in orders.DefaultIfEmpty() do + select (c.Name, (if obj.ReferenceEquals(o, null) then -1 else o.Id)) + }) + leftJoinedOrders + |> Scenario.flat [ "GroupJoin"; "SelectMany"; "Select"; "Select" ] + + Scenario.rows + "two left joins spelled leftOuterJoin" + (fun db -> + query { + for c in db.Customers do + leftOuterJoin o in db.Orders on (c.Id = o.CustomerId) into orders + for o in orders do + leftOuterJoin p in db.Products on (c.Id = p.Id) into products + for p in products do + select + (c.Name, + (if obj.ReferenceEquals(o, null) then -1 else o.Id), + (if obj.ReferenceEquals(p, null) then "" else p.Name)) + }) + leftJoinedOrdersAndProducts + |> Scenario.violating (nested [ "GroupJoin"; "SelectMany" ]) + + Scenario.rows + "two left joins spelled groupJoin and DefaultIfEmpty" + (fun db -> + query { + for c in db.Customers do + groupJoin o in db.Orders on (c.Id = o.CustomerId) into orders + for o in orders.DefaultIfEmpty() do + groupJoin p in db.Products on (c.Id = p.Id) into products + for p in products.DefaultIfEmpty() do + select + (c.Name, + (if obj.ReferenceEquals(o, null) then -1 else o.Id), + (if obj.ReferenceEquals(p, null) then "" else p.Name)) + }) + leftJoinedOrdersAndProducts + |> Scenario.violating (nested [ "GroupJoin"; "SelectMany" ]) + + Scenario.rows + "join after a left join" + (fun db -> + query { + for c in db.Customers do + leftOuterJoin o in db.Orders on (c.Id = o.CustomerId) into orders + for o in orders do + join p in db.Products on (c.Id = p.Id) + select (c.Name, (if obj.ReferenceEquals(o, null) then -1 else o.Id), p.Name) + }) + (leftJoinedOrdersAndProducts >> Seq.filter (fun (_, _, product) -> product <> "")) + |> Scenario.violating (nested [ "Join" ]) ] + +let cases () = loops () @ subqueries () @ leftJoins () From 69d6f14d445a246ece7eb9b3d9e67b8c7ad7d262 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 27 Sep 2026 04:29:30 +0200 Subject: [PATCH 02/14] Translate struct tuples in queries 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 #15522 Co-Authored-By: Claude Fable 5.1 --- docs/release-notes/.FSharp.Core/11.0.200.md | 1 + src/FSharp.Core/Linq.fs | 9 ++++- src/FSharp.Core/Query.fs | 5 +-- src/FSharp.Core/QueryExtensions.fs | 35 ++++++------------- .../QueryMethodSyntaxScenarios.fs | 16 +++++++++ .../QueryProjectionScenarios.fs | 32 +++++++++++++++++ 6 files changed, 71 insertions(+), 27 deletions(-) diff --git a/docs/release-notes/.FSharp.Core/11.0.200.md b/docs/release-notes/.FSharp.Core/11.0.200.md index 49e81a0efdf..fd72f4da417 100644 --- a/docs/release-notes/.FSharp.Core/11.0.200.md +++ b/docs/release-notes/.FSharp.Core/11.0.200.md @@ -6,5 +6,6 @@ * Correct `FSharpValue.PreComputeTuplePropertyInfo` to report that struct tuples store elements in fields, not properties, and recommend supported tuple readers instead of reporting a valid index as out of range. ([Issue #20801](https://github.com/dotnet/fsharp/issues/20801), [PR #20815](https://github.com/dotnet/fsharp/pull/20815)) * Quotation debug text from `Expr.ToString()`, `Expr.ToString(full)`, and `%A` now prints struct tuple construction as `NewStructTuple`; reference tuples remain `NewTuple`. ([Issue #20800](https://github.com/dotnet/fsharp/issues/20800), [PR #20814](https://github.com/dotnet/fsharp/pull/20814)) +* Fix struct tuples in query expressions and in lambdas converted to LINQ expression trees: selecting one no longer fails with "unreachable", and taking one apart no longer fails with a tuple index out of range. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522)) ### Changed diff --git a/src/FSharp.Core/Linq.fs b/src/FSharp.Core/Linq.fs index b8025b39bb4..431f6fd9514 100644 --- a/src/FSharp.Core/Linq.fs +++ b/src/FSharp.Core/Linq.fs @@ -475,7 +475,14 @@ module LeafExpressionConverter = | TupleGet(arg, n) -> let argP = ConvExprToLinqInContext env arg - let rec build ty argP n = + let rec build (ty: Type) argP n = + // A struct tuple holds its elements in fields, which PreComputeTuplePropertyInfo cannot return + if ty.IsValueType then + if n < 7 then + Expression.Field(argP, ty.GetField("Item" + string (n + 1))) |> asExpr + else + build (ty.GetGenericArguments().[7]) (Expression.Field(argP, ty.GetField "Rest") |> asExpr) (n - 7) + else match FSharpValue.PreComputeTuplePropertyInfo(ty, n) with | propInfo, None -> Expression.Property(argP, propInfo) |> asExpr diff --git a/src/FSharp.Core/Query.fs b/src/FSharp.Core/Query.fs index 2f84efd0eee..56171c629f9 100644 --- a/src/FSharp.Core/Query.fs +++ b/src/FSharp.Core/Query.fs @@ -1156,8 +1156,9 @@ module Query = /// immutable-to-mutable conversion information, convert it back to an expression involving immutable tuples or records. let rec ConvMutableToImmutable conv mutExpr = match conv with - | TupleConv convs -> - Expr.NewTuple (convs |> List.mapi (fun i conv -> ConvMutableToImmutable conv (AnonymousObjectGet (mutExpr, i)))) + | 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)))) diff --git a/src/FSharp.Core/QueryExtensions.fs b/src/FSharp.Core/QueryExtensions.fs index eafdae2d187..18edd4b9b5b 100644 --- a/src/FSharp.Core/QueryExtensions.fs +++ b/src/FSharp.Core/QueryExtensions.fs @@ -108,14 +108,6 @@ module internal Adapters = let anonObjectTypes = tupleTypes |> Array.map snd - let tupleToAnonTypeMap = - let t = Dictionary() - - for k, v in tupleTypes do - t.[k] <- v - - t - let anonToTupleTypeMap = let t = Dictionary() @@ -173,18 +165,13 @@ module internal Adapters = walk i e e.Type + /// Reference and struct tuples of the same arity share one anonymous object type. let RewriteTupleType (ty: Type) conv = - // Tuples are generic, so lookup only for generic types - assert ty.IsGenericType - let generic = ty.GetGenericTypeDefinition() - - match tupleToAnonTypeMap.TryGetValue generic with - | true, mutableTupleType -> - // Recursively transform type arguments - mutableTupleType.MakeGenericType(ty.GetGenericArguments() |> Array.toList |> conv |> Array.ofList) - | _ -> - assert false - failwith "unreachable" + assert (FSharpType.IsTuple ty) + let elementTypes = ty.GetGenericArguments() + let mutableTupleType = anonObjectTypes.[elementTypes.Length - 1] + // Recursively transform type arguments + mutableTupleType.MakeGenericType(elementTypes |> Array.toList |> conv |> Array.ofList) let (|RecordFieldGetSimplification|_|) (expr: Expr) = match expr with @@ -206,7 +193,7 @@ module internal Adapters = /// that we can invert the process in final query results. [] type ConversionDescription = - | TupleConv of ConversionDescription list + | TupleConv of isStruct: bool * ConversionDescription list | RecordConv of Type * ConversionDescription list | GroupingConv (* origKeyType: *) of Type (* origElemType: *) * Type * ConversionDescription | SeqConv of ConversionDescription @@ -216,18 +203,18 @@ module internal Adapters = /// "yield" or "select", convert it to a type involving anonymous objects according to the conversion data. let rec ConvImmutableTypeToMutableType conv ty = match conv with - | TupleConv convs -> + | TupleConv(isStruct, convs) -> assert (FSharpType.IsTuple ty) match convs with | x1 :: x2 :: x3 :: x4 :: x5 :: x6 :: x7 :: x8 :: tail -> - let els = [ x1; x2; x3; x4; x5; x6; x7; TupleConv(x8 :: tail) ] + let els = [ x1; x2; x3; x4; x5; x6; x7; TupleConv(isStruct, x8 :: tail) ] RewriteTupleType ty (List.map2 ConvImmutableTypeToMutableType els) | _ -> RewriteTupleType ty (List.map2 ConvImmutableTypeToMutableType convs) | RecordConv(_, convs) -> assert (isPartiallyImmutableRecord ty) let types = [| for f in FSharpType.GetRecordFields ty -> f.PropertyType |] - ConvImmutableTypeToMutableType (TupleConv convs) (FSharpType.MakeTupleType types) + ConvImmutableTypeToMutableType (TupleConv(false, convs)) (FSharpType.MakeTupleType types) | GroupingConv(_keyTy, _elemTy, conv) -> assert ty.IsGenericType assert (ty.GetGenericTypeDefinition() = typedefof>) @@ -324,7 +311,7 @@ module internal Adapters = let argExprsNow, argScripts = exprs |> List.map (ProduceMoreMutables tipf) |> List.unzip - NewAnonymousObject argExprsNow, TupleConv argScripts + NewAnonymousObject argExprsNow, TupleConv(expr.Type.IsValueType, argScripts) // Replace immutable records by anonymous objects | Patterns.NewRecord(typ, args) when isPartiallyImmutableRecord typ -> diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs index b8a704a846e..5325268456b 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs @@ -69,6 +69,12 @@ let cases () = (customers >> Seq.map (fun c -> c.Id, c.Name) >> Seq.filter (fun (id, _) -> id > 1)) |> Scenario.violating (reading "Tuple`2" @ block) + Scenario.rows + "Where taking a struct tuple apart" + (fun db -> db.Customers.Select(fun c -> struct (c.Id, c.Name)).Where(fun (struct (id, _)) -> id > 1)) + (customers >> Seq.map (fun c -> struct (c.Id, c.Name)) >> Seq.filter (fun (struct (id, _)) -> id > 1)) + |> Scenario.violating (reading "ValueTuple`2" @ block) + Scenario.rows "Where taking the last of nine apart" (fun db -> @@ -79,6 +85,16 @@ let cases () = (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) |> Scenario.violating (reading "Tuple`8" @ block) + Scenario.rows + "Where taking the last of a struct nine apart" + (fun db -> + db.Customers + .Select(fun c -> struct (c.Id + 100, c.Name, c.City, c.Age, c.Id * 7, c.Name.Length, c.City.Length, c.Age * 2, c.Id)) + .Where(fun (struct (_, _, _, _, _, _, _, _, last)) -> last > 1) + .Select(fun (struct (_, name, _, _, _, _, _, _, _)) -> name)) + (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) + |> Scenario.violating (reading "ValueTuple`8" @ block) + Scenario.rows "Where over a query that selects a tuple" (fun db -> (query { for c in db.Customers do select (c.Id, c.Name) }).Where(fun (id, _) -> id > 1)) diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs index 44d598d3d13..8c1e684614e 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs @@ -43,6 +43,27 @@ let cases () = (fun db -> query { for c in db.Customers do select ((c.Id, c.Name), c.Age) }) (customers >> Seq.map (fun c -> (c.Id, c.Name), c.Age)) + Scenario.rows + "struct tuple of two" + (fun db -> query { for c in db.Customers do select (struct (c.Id, c.Name)) }) + (customers >> Seq.map (fun c -> struct (c.Id, c.Name))) + + Scenario.rows + "struct tuple of nine" + (fun db -> + query { for c in db.Customers do select (struct (c.Id, c.Name, c.City, c.Age, c.Id + 100, c.Name.Length, c.Age * 2, c.Id * 7, c.Age - c.Id)) }) + (customers >> Seq.map (fun c -> struct (c.Id, c.Name, c.City, c.Age, c.Id + 100, c.Name.Length, c.Age * 2, c.Id * 7, c.Age - c.Id))) + + Scenario.rows + "struct tuple inside a tuple" + (fun db -> query { for c in db.Customers do select (struct (c.Id, c.Name), c.Age) }) + (customers >> Seq.map (fun c -> struct (c.Id, c.Name), c.Age)) + + Scenario.rows + "tuple inside a struct tuple" + (fun db -> query { for c in db.Customers do select (struct ((c.Id, c.Name), c.Age)) }) + (customers >> Seq.map (fun c -> struct ((c.Id, c.Name), c.Age))) + Scenario.rows "tuple taken apart by the next for" (fun db -> @@ -54,6 +75,17 @@ let cases () = (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) |> Scenario.violating [ Violation.ReadsUnnamedMember "Tuple`2" ] + Scenario.rows + "struct tuple taken apart by the next for" + (fun db -> + query { + for struct (id, name) in (query { for c in db.Customers do select (struct (c.Id, c.Name)) }) do + where (id > 1) + select name + }) + (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) + |> Scenario.violating [ Violation.ReadsUnnamedMember "ValueTuple`2" ] + Scenario.rows "record, fields in declaration order" (fun db -> query { for c in db.Customers do select { PersonName = c.Name; PersonAge = c.Age } }) From 27d8b78a550e7e68bb7f393fb855371f8449b87d Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 1 Nov 2026 22:12:20 +0100 Subject: [PATCH 03/14] Link the release note to the pull request Co-Authored-By: Claude Sonnet 5.5 --- docs/release-notes/.FSharp.Core/11.0.200.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release-notes/.FSharp.Core/11.0.200.md b/docs/release-notes/.FSharp.Core/11.0.200.md index fd72f4da417..0aa52c42d5a 100644 --- a/docs/release-notes/.FSharp.Core/11.0.200.md +++ b/docs/release-notes/.FSharp.Core/11.0.200.md @@ -6,6 +6,6 @@ * Correct `FSharpValue.PreComputeTuplePropertyInfo` to report that struct tuples store elements in fields, not properties, and recommend supported tuple readers instead of reporting a valid index as out of range. ([Issue #20801](https://github.com/dotnet/fsharp/issues/20801), [PR #20815](https://github.com/dotnet/fsharp/pull/20815)) * Quotation debug text from `Expr.ToString()`, `Expr.ToString(full)`, and `%A` now prints struct tuple construction as `NewStructTuple`; reference tuples remain `NewTuple`. ([Issue #20800](https://github.com/dotnet/fsharp/issues/20800), [PR #20814](https://github.com/dotnet/fsharp/pull/20814)) -* Fix struct tuples in query expressions and in lambdas converted to LINQ expression trees: selecting one no longer fails with "unreachable", and taking one apart no longer fails with a tuple index out of range. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522)) +* Fix struct tuples in query expressions and in lambdas converted to LINQ expression trees: selecting one no longer fails with "unreachable", and taking one apart no longer fails with a tuple index out of range. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522), [PR #20894](https://github.com/dotnet/fsharp/pull/20894)) ### Changed From 1d654692cb7b25d339b86255957dc25872743ad6 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 1 Nov 2026 22:55:55 +0100 Subject: [PATCH 04/14] Follow every query chain when checking for unnamed member reads 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 --- .../QueryProviderShape.fs | 171 +++++++++++-- .../Microsoft.FSharp.Linq/QueryScenario.fs | 12 +- .../Microsoft.FSharp.Linq/QueryShapeTests.fs | 231 ++++++++++++++++++ 3 files changed, 389 insertions(+), 25 deletions(-) diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs index bad748d75ab..ae25aa7b1d6 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs @@ -294,35 +294,70 @@ module ProviderShape = let private yields (elementType: Type) (source: Expression) = typedefof>.MakeGenericType(elementType).IsAssignableFrom source.Type - /// Members read from elements that an earlier operator built without naming them: - /// the provider has to take apart an object it was not told how to. - let private unnamedMemberReads (expression: Expression) = + /// Whether the lambda gives its first parameter back as it is. + let private returnsParameter (lambda: LambdaExpression) = + lambda.Parameters.Count > 0 && obj.ReferenceEquals(lambda.Body, lambda.Parameters.[0]) + + /// A query that is its own root, as opposed to one built from another by operators. + let private isRoot (query: IQueryable) = + match query.Expression with + | :? ConstantExpression as root -> obj.ReferenceEquals(root.Value, query) + | _ -> false + + /// Finds members read from elements that an earlier operator built without naming them: the provider has to + /// take apart an object it was not told how to. Every chain of operators is followed, the ones inside lambdas + /// and the ones of a query captured from earlier too, and each parameter of a lambda is tied to the sequence + /// it stands for, which is not always the first source of the operator. + type private UnnamedMemberFinder() = + inherit ExpressionVisitor() + let found = ResizeArray() - let rec producesUnnamed (expression: Expression) = - match expression with + // Whether the elements are unnamed, for the parameters that stand for a sequence: the group of a GroupJoin + let sequences = Dictionary(HashIdentity.Reference) + + let lambdaAt (call: MethodCallExpression) index = + if index < call.Arguments.Count then + match call.Arguments.[index] with + | Lambda lambda -> ValueSome lambda + | _ -> ValueNone + else + ValueNone + + // The parameter of the lambda stands for an element of the sequence, which was built without names + let report (lambda: LambdaExpression) index (sequence: Expression) (unnamed: bool) = + if unnamed && index < lambda.Parameters.Count then + let parameter = lambda.Parameters.[index] + + if yields parameter.Type sequence && readsMemberOf parameter lambda.Body then + found.Add(Violation.ReadsUnnamedMember parameter.Type.Name) + + member _.Found = found |> Seq.distinct |> List.ofSeq + + /// Whether the elements of the sequence were built without naming their members. + member this.Unnamed(sequence: Expression) : bool = + match sequence with | :? MethodCallExpression as call when isLinqOperator call -> let source = call.Arguments.[0] - let sourceUnnamed = producesUnnamed source let lambdas = - [ for argument in Seq.skip 1 call.Arguments do - match argument with - | Lambda lambda -> lambda - | _ -> () ] - - if sourceUnnamed then - for lambda in lambdas do - let element = lambda.Parameters.[0] - - if yields element.Type source && readsMemberOf element lambda.Body then - found.Add(Violation.ReadsUnnamedMember element.Type.Name) + [ for index in 1 .. call.Arguments.Count - 1 do + match lambdaAt call index with + | ValueSome lambda -> lambda + | ValueNone -> () ] match call.Method.Name with - | "Select" - | "SelectMany" + | "Select" -> + lambdas + |> List.tryLast + |> Option.exists (fun selector -> buildsUnnamed selector.Body || (returnsParameter selector && this.Unnamed source)) + | "SelectMany" -> + match lambdas with + | [ collection ] -> this.Unnamed collection.Body + | [ _; result ] -> buildsUnnamed result.Body + | _ -> false | "Join" - | "GroupJoin" -> lambdas |> List.last |> _.Body |> buildsUnnamed + | "GroupJoin" -> lambdas |> List.tryLast |> Option.exists (fun result -> buildsUnnamed result.Body) | "Where" | "OrderBy" | "OrderByDescending" @@ -333,18 +368,108 @@ module ProviderShape = | "Take" | "TakeWhile" | "Distinct" - | "DefaultIfEmpty" -> sourceUnnamed + | "DefaultIfEmpty" + | "Reverse" + | "AsQueryable" + | "AsEnumerable" -> this.Unnamed source + | "Concat" + | "Union" + | "Intersect" + | "Except" -> this.Unnamed source || (call.Arguments.Count > 1 && this.Unnamed call.Arguments.[1]) + | _ -> false + | :? ConstantExpression as constant -> + match constant.Value with + | :? IQueryable as query when not (isRoot query) -> this.Unnamed query.Expression + | _ -> false + | :? ParameterExpression as parameter -> + match sequences.TryGetValue parameter with + | true, unnamed -> unnamed | _ -> false + | :? UnaryExpression as conversion when conversion.NodeType = ExpressionType.Convert -> this.Unnamed conversion.Operand | _ -> false - producesUnnamed expression |> ignore - List.ofSeq found + /// Reports the members that the lambdas of the operator read from elements built without names. + member this.Check(call: MethodCallExpression) = + let source = call.Arguments.[0] + + match call.Method.Name with + | "Join" + | "GroupJoin" when call.Arguments.Count >= 5 -> + let outer, inner = call.Arguments.[0], call.Arguments.[1] + let outerUnnamed, innerUnnamed = this.Unnamed outer, this.Unnamed inner + + lambdaAt call 2 |> ValueOption.iter (fun key -> report key 0 outer outerUnnamed) + lambdaAt call 3 |> ValueOption.iter (fun key -> report key 0 inner innerUnnamed) + + lambdaAt call 4 + |> ValueOption.iter (fun result -> + report result 0 outer outerUnnamed + + if call.Method.Name = "Join" then + report result 1 inner innerUnnamed + elif result.Parameters.Count > 1 then + // The second parameter is the group of the matching inner elements, not one of them + sequences.[result.Parameters.[1]] <- innerUnnamed) + | "SelectMany" when call.Arguments.Count >= 3 -> + // A collection selector, then a result selector over the element and an element of the collection + let sourceUnnamed = this.Unnamed source + let collection = lambdaAt call 1 + collection |> ValueOption.iter (fun selector -> report selector 0 source sourceUnnamed) + + lambdaAt call 2 + |> ValueOption.iter (fun result -> + report result 0 source sourceUnnamed + + collection + |> ValueOption.iter (fun selector -> report result 1 selector.Body (this.Unnamed selector.Body))) + | "Zip" when call.Arguments.Count = 3 -> + let second = call.Arguments.[1] + + lambdaAt call 2 + |> ValueOption.iter (fun result -> + report result 0 source (this.Unnamed source) + report result 1 second (this.Unnamed second)) + // The first parameter of the function is the accumulated value + | "Aggregate" -> () + | _ -> + let sourceUnnamed = this.Unnamed source + + for index in 1 .. call.Arguments.Count - 1 do + lambdaAt call index |> ValueOption.iter (fun lambda -> report lambda 0 source sourceUnnamed) + + override this.VisitMethodCall node = + if isLinqOperator node then + this.Check node + + base.VisitMethodCall node + + // A query built earlier and used in this one is part of what the provider translates. + override this.VisitConstant node = + match node.Value with + | :? IQueryable as query when not (isRoot query) -> this.Visit query.Expression |> ignore + | _ -> () + + node + + let private unnamedMemberReads (expression: Expression) = + let finder = UnnamedMemberFinder() + finder.Visit expression |> ignore + finder.Found let violations (expression: Expression) = let visitor = ShapeVisitor() visitor.Visit expression |> ignore visitor.Violations @ unnamedMemberReads expression |> List.distinct + /// Whether the expression calls a Queryable operator that gives a value and not a query, as a provider is asked + /// to execute when a query is counted, summed or searched. + let isTerminalOperator (expression: Expression) = + match expression with + | :? MethodCallExpression as call -> + call.Method.DeclaringType = typeof + && not (typeof.IsAssignableFrom call.Type) + | _ -> false + /// The LINQ operators along the source chain, outermost last. let spine (expression: Expression) = let rec walk (expression: Expression) operators = diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs index b2a270042e9..094519b89e0 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs @@ -133,10 +133,18 @@ module Scenario = Rows = [ for row in rows -> row ] } // The value comes from the last execution; whatever ran before it went to the provider too. | Outcome.Value value when provider.Executed.Count > 0 -> + let last = provider.Executed.[provider.Executed.Count - 1] let earlier = provider.Executed |> Seq.take (provider.Executed.Count - 1) - { Expression = provider.Executed.[provider.Executed.Count - 1] - Violations = [ for expression in earlier do yield! ProviderShape.violations expression ] + { Expression = last + Violations = + [ for expression in earlier do + yield! ProviderShape.violations expression + + // Enumerating the rows of a query is an execution too, so the last one has to be the + // operator that gives the value: a count made over those rows is made by the client. + if not (ProviderShape.isTerminalOperator last) then + Violation.LeftProvider ] Rows = [ value ] } | Outcome.Value value -> { Expression = Expression.Empty() diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs index 6e284ed7d4d..0b2ce330374 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs @@ -2,6 +2,11 @@ namespace FSharp.Core.UnitTests.Linq +open System +open System.Collections.Generic +open System.Linq +open System.Linq.Expressions +open System.Reflection open Xunit type QueryShapeTests() = @@ -29,3 +34,229 @@ type QueryShapeTests() = [] member _.``Method syntax``(scenario: Scenario) = Scenario.check scenario + +/// Trees built by hand, the way a C# compiler builds them, to check the validator itself. They do not depend on how +/// FSharp.Core translates a query, so they mean the same in every branch. +module private ValidatorTrees = + + let customers = Queryable.AsQueryable Seq.empty + let orders = Queryable.AsQueryable Seq.empty + + let customerPair = typeof> + let orderPair = typeof> + + let call (name: string) (typeArguments: Type list) (arguments: Expression list) = + Expression.Call(typeof, name, Array.ofList typeArguments, Array.ofList arguments) :> Expression + + let quote (lambda: LambdaExpression) = Expression.Quote lambda :> Expression + + let pair (tuple: Type) (arguments: Expression list) (named: bool) = + let constructor = tuple.GetConstructors().[0] + + if named then + Expression.New(constructor, arguments, [| tuple.GetProperty "Item1" :> MemberInfo; tuple.GetProperty "Item2" :> MemberInfo |]) + else + Expression.New(constructor, arguments) + + /// customers.Select(c => new Tuple(c.Id, c.Name)), with or without naming the members + let customerPairs (named: bool) = + let c = Expression.Parameter(typeof, "c") + let body = pair customerPair [ Expression.Property(c, "Id"); Expression.Property(c, "Name") ] named + + call "Select" [ typeof; customerPair ] [ customers.Expression; quote (Expression.Lambda(body, c)) ] + + /// orders.Select(o => new Tuple(o.Id, o.Total)), with or without naming the members + let orderPairs (named: bool) = + let o = Expression.Parameter(typeof, "o") + let body = pair orderPair [ Expression.Property(o, "Id"); Expression.Property(o, "Total") ] named + + call "Select" [ typeof; orderPair ] [ orders.Expression; quote (Expression.Lambda(body, o)) ] + + /// source.Where(t => t.Item1 > 0) + let whereItem1Positive (source: Expression) (tuple: Type) = + let t = Expression.Parameter(tuple, "t") + let predicate = Expression.Lambda(Expression.GreaterThan(Expression.Property(t, "Item1"), Expression.Constant(0)), t) + + call "Where" [ tuple ] [ source; quote predicate ] + + let item1 (p: ParameterExpression) = Expression.Property(p, "Item1") :> Expression + let item2 (p: ParameterExpression) = Expression.Property(p, "Item2") :> Expression + let zero (_: ParameterExpression) = Expression.Constant(0) :> Expression + + /// outer.Join(inner, o => outerKey(o), i => innerKey(i), (o, i) => result(o, i)), with int keys + let join + (outer: Expression) + (outerElement: Type) + (inner: Expression) + (innerElement: Type) + (outerKey: ParameterExpression -> Expression) + (innerKey: ParameterExpression -> Expression) + (result: ParameterExpression -> ParameterExpression -> Expression) + = + let parameter (ty: Type) name = Expression.Parameter(ty, name) + let o1 = parameter outerElement "o" + let i1 = parameter innerElement "i" + let o2 = parameter outerElement "o" + let i2 = parameter innerElement "i" + let resultBody = result o2 i2 + + call + "Join" + [ outerElement; innerElement; typeof; resultBody.Type ] + [ outer + inner + quote (Expression.Lambda(outerKey o1, o1)) + quote (Expression.Lambda(innerKey i1, i1)) + quote (Expression.Lambda(resultBody, o2, i2)) ] + +type ProviderShapeValidatorTests() = + + static let unnamedRead = [ Violation.ReadsUnnamedMember "Tuple`2" ] + + static let reported tree = ProviderShape.violations tree + + [] + member _.``A member read from an unnamed projection is reported``() = + let unnamed = ValidatorTrees.whereItem1Positive (ValidatorTrees.customerPairs false) ValidatorTrees.customerPair + let named = ValidatorTrees.whereItem1Positive (ValidatorTrees.customerPairs true) ValidatorTrees.customerPair + + Assert.Equal(unnamedRead, reported unnamed) + Assert.Empty(reported named) + + [] + member _.``A chain inside the lambda of another operator is checked``() = + let nested named = + let inner = ValidatorTrees.whereItem1Positive (ValidatorTrees.orderPairs named) ValidatorTrees.orderPair + let any = ValidatorTrees.call "Any" [ ValidatorTrees.orderPair ] [ inner ] + let c = Expression.Parameter(typeof, "c") + + ValidatorTrees.call "Where" [ typeof ] [ ValidatorTrees.customers.Expression; ValidatorTrees.quote (Expression.Lambda(any, c)) ] + + Assert.Equal(unnamedRead, reported (nested false)) + Assert.Empty(reported (nested true)) + + [] + member _.``A query captured from earlier is checked where it is used``() = + let capturing named = + let captured = ValidatorTrees.customers.Provider.CreateQuery>(ValidatorTrees.customerPairs named) + let o = Expression.Parameter(typeof, "o") + let t = Expression.Parameter(ValidatorTrees.customerPair, "t") + let predicate = Expression.Lambda(Expression.GreaterThan(Expression.Property(t, "Item1"), Expression.Property(o, "Id")), t) + let any = ValidatorTrees.call "Any" [ ValidatorTrees.customerPair ] [ Expression.Constant(captured) :> Expression; ValidatorTrees.quote predicate ] + + ValidatorTrees.call "Where" [ typeof ] [ ValidatorTrees.orders.Expression; ValidatorTrees.quote (Expression.Lambda(any, o)) ] + + Assert.Equal(unnamedRead, reported (capturing false)) + Assert.Empty(reported (capturing true)) + + [] + member _.``An identity Select keeps the elements unnamed``() = + let t = Expression.Parameter(ValidatorTrees.customerPair, "t") + + let identity = + ValidatorTrees.call + "Select" + [ ValidatorTrees.customerPair; ValidatorTrees.customerPair ] + [ ValidatorTrees.customerPairs false; ValidatorTrees.quote (Expression.Lambda(t, t)) ] + + Assert.Equal(unnamedRead, reported (ValidatorTrees.whereItem1Positive identity ValidatorTrees.customerPair)) + + [] + member _.``The elements of a collection selector stay unnamed after SelectMany``() = + let flattened named = + let c = Expression.Parameter(typeof, "c") + let collectionType = typedefof>.MakeGenericType(typeof, typeof>>) + let collection = Expression.Lambda(collectionType, ValidatorTrees.orderPairs named, c) + + ValidatorTrees.call + "SelectMany" + [ typeof; ValidatorTrees.orderPair ] + [ ValidatorTrees.customers.Expression; ValidatorTrees.quote collection ] + + Assert.Equal(unnamedRead, reported (ValidatorTrees.whereItem1Positive (flattened false) ValidatorTrees.orderPair)) + Assert.Empty(reported (ValidatorTrees.whereItem1Positive (flattened true) ValidatorTrees.orderPair)) + + [] + member _.``A named input of a join is not blamed for the other one being unnamed``() = + // The two inputs have the same tuple type, and only the outer one is unnamed. Neither key reads a member of it. + let tree = + ValidatorTrees.join + (ValidatorTrees.customerPairs false) + ValidatorTrees.customerPair + (ValidatorTrees.customerPairs true) + ValidatorTrees.customerPair + ValidatorTrees.zero + ValidatorTrees.item1 + (fun _ _ -> ValidatorTrees.zero null) + + Assert.Empty(reported tree) + + [] + member _.``An unnamed inner input of a join is checked against the inner key``() = + let tree = + ValidatorTrees.join + (ValidatorTrees.customerPairs true) + ValidatorTrees.customerPair + (ValidatorTrees.customerPairs false) + ValidatorTrees.customerPair + ValidatorTrees.zero + ValidatorTrees.item1 + (fun _ _ -> ValidatorTrees.zero null) + + Assert.Equal(unnamedRead, reported tree) + + [] + member _.``An unnamed inner input of a join is checked against the second parameter of the result``() = + let tree = + ValidatorTrees.join + (ValidatorTrees.customerPairs true) + ValidatorTrees.customerPair + (ValidatorTrees.customerPairs false) + ValidatorTrees.customerPair + ValidatorTrees.zero + ValidatorTrees.zero + (fun _ i -> ValidatorTrees.item2 i) + + Assert.Equal(unnamedRead, reported tree) + + [] + member _.``An unnamed inner input of a join is checked when the outer input is a plain source``() = + let tree = + ValidatorTrees.join + ValidatorTrees.customers.Expression + typeof + (ValidatorTrees.orderPairs false) + ValidatorTrees.orderPair + (fun c -> Expression.Property(c, "Id") :> Expression) + ValidatorTrees.item1 + (fun _ _ -> ValidatorTrees.zero null) + + Assert.Equal(unnamedRead, reported tree) + + [] + member _.``A selector that is not a lambda is passed over``() = + let selector = + Expression.Constant( + Expression.Lambda>(Expression.Constant(0), Expression.Parameter(typeof, "c")), + typeof>> + ) + + let tree = + ValidatorTrees.call "Select" [ typeof; typeof ] [ ValidatorTrees.customers.Expression; selector :> Expression ] + + Assert.Empty(reported tree) + + [] + member _.``A count made by the provider passes``() = + Scenario.value "count by the provider" (fun db -> db.Customers.Where(fun c -> c.Age > 26).Count()) (fun _ -> 3) + |> Scenario.check + + [] + member _.``A count made over the rows of the provider leaves it``() = + Scenario.value "count on the client" (fun db -> db.Customers.Where(fun c -> c.Age > 26).AsEnumerable().Count()) (fun _ -> 3) + |> Scenario.violating [ Violation.LeftProvider ] + |> Scenario.check + + Scenario.value "count by Enumerable" (fun db -> Enumerable.Count(db.Customers.Where(fun c -> c.Age > 26))) (fun _ -> 3) + |> Scenario.violating [ Violation.LeftProvider ] + |> Scenario.check From 8ed04f2ad0875b4b072212a0dc408fe4e45708da Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Mon, 2 Nov 2026 14:24:09 +0100 Subject: [PATCH 05/14] Count every execution and every violation when checking a scenario 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 --- docs/release-notes/.FSharp.Core/11.0.200.md | 2 +- .../QueryKeyScenarios.fs | 8 ++- .../QueryMethodSyntaxScenarios.fs | 14 +++-- .../QueryProviderShape.fs | 30 ++++++---- .../Microsoft.FSharp.Linq/QueryScenario.fs | 22 ++++--- .../Microsoft.FSharp.Linq/QueryShapeTests.fs | 59 +++++++++++++++++++ 6 files changed, 107 insertions(+), 28 deletions(-) diff --git a/docs/release-notes/.FSharp.Core/11.0.200.md b/docs/release-notes/.FSharp.Core/11.0.200.md index 0aa52c42d5a..74a8bab02a3 100644 --- a/docs/release-notes/.FSharp.Core/11.0.200.md +++ b/docs/release-notes/.FSharp.Core/11.0.200.md @@ -6,6 +6,6 @@ * Correct `FSharpValue.PreComputeTuplePropertyInfo` to report that struct tuples store elements in fields, not properties, and recommend supported tuple readers instead of reporting a valid index as out of range. ([Issue #20801](https://github.com/dotnet/fsharp/issues/20801), [PR #20815](https://github.com/dotnet/fsharp/pull/20815)) * Quotation debug text from `Expr.ToString()`, `Expr.ToString(full)`, and `%A` now prints struct tuple construction as `NewStructTuple`; reference tuples remain `NewTuple`. ([Issue #20800](https://github.com/dotnet/fsharp/issues/20800), [PR #20814](https://github.com/dotnet/fsharp/pull/20814)) -* Fix struct tuples in query expressions and in lambdas converted to LINQ expression trees: selecting one no longer fails with "unreachable", and taking one apart no longer fails with a tuple index out of range. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522), [PR #20894](https://github.com/dotnet/fsharp/pull/20894)) +* Fix struct tuples in query expressions: selecting one no longer fails with "unreachable", and the items of one that a later operator reads are taken from its fields instead of failing with `ArgumentException: Struct tuples store their elements in fields, not properties`. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522), [PR #20894](https://github.com/dotnet/fsharp/pull/20894)) ### Changed diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryKeyScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryKeyScenarios.fs index 03011a4d99f..0c7e32749cb 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryKeyScenarios.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryKeyScenarios.fs @@ -8,6 +8,10 @@ open Microsoft.FSharp.Linq.RuntimeHelpers let private block = [ Violation.Statement ExpressionType.Block ] +/// A tree is pinned with every block it has: one that goes from three to two is a change as well +let private blocks count = + List.replicate count (Violation.Statement ExpressionType.Block) + let private customers (data: Data) = Seq.ofList data.Customers /// Orders paired with the lines that share both the order number and, as quantity, the customer number. @@ -68,7 +72,7 @@ let private joins () = select (o.Id, l.Id) }) matchingLines - |> Scenario.violating block + |> Scenario.violating (blocks 2) Scenario.rows "join on an anonymous record, fields in alphabetical order" @@ -89,7 +93,7 @@ let private joins () = select (o.Id, l.Id) }) matchingLines - |> Scenario.violating block + |> Scenario.violating (blocks 2) Scenario.rows "groupJoin on a tuple" diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs index 5325268456b..760c9b78a49 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs @@ -7,6 +7,10 @@ open System.Linq.Expressions let private block = [ Violation.Statement ExpressionType.Block ] +/// A tree is pinned with every block it has: one that goes from three to two is a change as well +let private blocks count = + List.replicate count (Violation.Statement ExpressionType.Block) + let private reading typeName = [ Violation.ReadsUnnamedMember typeName ] @@ -67,7 +71,7 @@ let cases () = "Where taking a tuple apart" (fun db -> db.Customers.Select(fun c -> (c.Id, c.Name)).Where(fun (id, _) -> id > 1)) (customers >> Seq.map (fun c -> c.Id, c.Name) >> Seq.filter (fun (id, _) -> id > 1)) - |> Scenario.violating (reading "Tuple`2" @ block) + |> Scenario.violating (reading "Tuple`2" @ blocks 2) Scenario.rows "Where taking a struct tuple apart" @@ -83,7 +87,7 @@ let cases () = .Where(fun (_, _, _, _, _, _, _, _, last) -> last > 1) .Select(fun (_, name, _, _, _, _, _, _, _) -> name)) (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) - |> Scenario.violating (reading "Tuple`8" @ block) + |> Scenario.violating (reading "Tuple`8" @ reading "Tuple`8" @ blocks 18) Scenario.rows "Where taking the last of a struct nine apart" @@ -93,19 +97,19 @@ let cases () = .Where(fun (struct (_, _, _, _, _, _, _, _, last)) -> last > 1) .Select(fun (struct (_, name, _, _, _, _, _, _, _)) -> name)) (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) - |> Scenario.violating (reading "ValueTuple`8" @ block) + |> Scenario.violating (reading "ValueTuple`8" @ reading "ValueTuple`8" @ blocks 2) Scenario.rows "Where over a query that selects a tuple" (fun db -> (query { for c in db.Customers do select (c.Id, c.Name) }).Where(fun (id, _) -> id > 1)) (customers >> Seq.map (fun c -> c.Id, c.Name) >> Seq.filter (fun (id, _) -> id > 1)) - |> Scenario.violating (reading "Tuple`2" @ block) + |> Scenario.violating (reading "Tuple`2" @ blocks 2) Scenario.rows "Where on a nullable property" (fun db -> db.Customers.Where(fun c -> c.Registered.HasValue && c.Registered.Value.Year > 2020)) (customers >> Seq.filter (fun c -> c.Registered.HasValue && c.Registered.Value.Year > 2020)) - |> Scenario.violating block + |> Scenario.violating (blocks 3) Scenario.rows "Where with a lambda over another source" diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs index ae25aa7b1d6..20665171824 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs @@ -25,12 +25,15 @@ type private InMemoryRewriter() = /// An in-memory query that stays on the provider that created it and prints its root by name. [] -type CapturedQuery<'T>(provider: IQueryProvider, name: string, items: seq<'T>, derived: Expression voption) = +type CapturedQuery<'T>(provider: IQueryProvider, name: string, items: seq<'T>, derived: Expression voption, scanned: Expression -> unit) = - let enumerate () = + let enumerate (self: Expression) = match derived with | ValueSome expression -> provider.Execute>(expression).GetEnumerator() - | ValueNone -> items.GetEnumerator() + | ValueNone -> + // No operator runs at the provider here: whoever enumerates the source takes all of its rows + scanned self + items.GetEnumerator() override _.ToString() = match derived with @@ -51,8 +54,8 @@ type CapturedQuery<'T>(provider: IQueryProvider, name: string, items: seq<'T>, d member _.ElementType = typeof<'T> member _.Provider = provider - member _.GetEnumerator() : IEnumerator<'T> = enumerate () - member _.GetEnumerator() : IEnumerator = enumerate () + member this.GetEnumerator() : IEnumerator<'T> = enumerate (Expression.Constant this) + member this.GetEnumerator() : IEnumerator = enumerate (Expression.Constant this) /// Executes in memory, and records what it was asked to execute. [] @@ -74,14 +77,19 @@ type CapturingQueryProvider() = finally depth <- depth - 1 - /// The expressions executed directly, not from inside another execution. + /// The expressions executed directly, not from inside another execution, and the sources that were enumerated. member _.Executed = executed :> IReadOnlyList + /// A source that is enumerated as it is, without an operator + member _.Scanned(source: Expression) = + if depth = 0 then + executed.Add source + member this.Source(name: string, items: seq<'T>) : IQueryable<'T> = - CapturedQuery<'T>(this, name, items, ValueNone) + CapturedQuery<'T>(this, name, items, ValueNone, this.Scanned) member this.Derived<'T>(expression: Expression) : IQueryable<'T> = - CapturedQuery<'T>(this, "", Seq.empty, ValueSome expression) + CapturedQuery<'T>(this, "", Seq.empty, ValueSome expression, this.Scanned) interface IQueryProvider with member this.CreateQuery<'T>(expression: Expression) : IQueryable<'T> = this.Derived<'T> expression @@ -172,7 +180,7 @@ module ProviderShape = | _ -> ValueNone | _ -> ValueNone - member _.Violations = found |> Seq.distinct |> List.ofSeq + member _.Violations = List.ofSeq found override _.VisitLambda<'T>(node: Expression<'T>) = let added = bind node.Parameters @@ -332,7 +340,7 @@ module ProviderShape = if yields parameter.Type sequence && readsMemberOf parameter lambda.Body then found.Add(Violation.ReadsUnnamedMember parameter.Type.Name) - member _.Found = found |> Seq.distinct |> List.ofSeq + member _.Found = List.ofSeq found /// Whether the elements of the sequence were built without naming their members. member this.Unnamed(sequence: Expression) : bool = @@ -459,7 +467,7 @@ module ProviderShape = let violations (expression: Expression) = let visitor = ShapeVisitor() visitor.Visit expression |> ignore - visitor.Violations @ unnamedMemberReads expression |> List.distinct + visitor.Violations @ unnamedMemberReads expression /// Whether the expression calls a Queryable operator that gives a value and not a query, as a provider is asked /// to execute when a query is counted, summed or searched. diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs index 094519b89e0..042b108c16a 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs @@ -131,19 +131,22 @@ module Scenario = { Expression = rows.Expression Violations = (if stayed then [] else [ Violation.LeftProvider ]) Rows = [ for row in rows -> row ] } - // The value comes from the last execution; whatever ran before it went to the provider too. + // A query that is counted, summed or searched is executed by the operator that gives the value. + // Enumerating the rows of a query is an execution too, and so is every query that a function on the + // client runs while it goes through those rows: the provider made the value only if every + // execution is such an operator. The last one is the expression that is checked as a whole; whatever + // ran before it went to the provider too. | Outcome.Value value when provider.Executed.Count > 0 -> - let last = provider.Executed.[provider.Executed.Count - 1] - let earlier = provider.Executed |> Seq.take (provider.Executed.Count - 1) + let executed = List.ofSeq provider.Executed + let last = List.last executed + let earlier = List.take (executed.Length - 1) executed { Expression = last Violations = [ for expression in earlier do yield! ProviderShape.violations expression - // Enumerating the rows of a query is an execution too, so the last one has to be the - // operator that gives the value: a count made over those rows is made by the client. - if not (ProviderShape.isTerminalOperator last) then + if not (List.forall ProviderShape.isTerminalOperator executed) then Violation.LeftProvider ] Rows = [ value ] } | Outcome.Value value -> @@ -165,9 +168,10 @@ module Scenario = let failures (scenario: Scenario) = let data, observed = observe scenario + // Every occurrence counts: a tree with two blocks is not a tree with one let violations = observed.Violations @ ProviderShape.violations observed.Expression - |> List.distinct + |> List.sort let threw = violations @@ -175,8 +179,8 @@ module Scenario = | Violation.Throws _ -> true | _ -> false) - [ if set violations <> set scenario.Violations then - $"violations: expected %A{scenario.Violations}, got %A{violations}" + [ if violations <> List.sort scenario.Violations then + $"violations: expected %A{List.sort scenario.Violations}, got %A{violations}" if not threw then let expected = inOrder scenario.Ordered (scenario.Expected data) diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs index 0b2ce330374..35c106eedab 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs @@ -246,6 +246,48 @@ type ProviderShapeValidatorTests() = Assert.Empty(reported tree) + [] + member _.``A block is reported once for each block``() = + let c = Expression.Parameter(typeof, "c") + + let block (name: string) = + let x = Expression.Variable(typeof, name) + Expression.Block([| x |], Expression.Assign(x, Expression.Property(c, "Id")), Expression.GreaterThan(x, Expression.Constant 0)) :> Expression + + let where (body: Expression) = + ValidatorTrees.call "Where" [ typeof ] [ ValidatorTrees.customers.Expression; ValidatorTrees.quote (Expression.Lambda(body, c)) ] + + let blockViolation = Violation.Statement ExpressionType.Block + + Assert.Equal([ blockViolation ], reported (where (block "x"))) + Assert.Equal([ blockViolation; blockViolation ], reported (where (Expression.AndAlso(block "x", block "y")))) + + [] + member _.``A lambda that is invoked is reported``() = + let c = Expression.Parameter(typeof, "c") + let n = Expression.Parameter(typeof, "n") + let positive = Expression.Lambda>(Expression.GreaterThan(n, Expression.Constant 0), n) + let tree = Expression.Invoke(positive, Expression.Property(c, "Id")) + + let where = + ValidatorTrees.call "Where" [ typeof ] [ ValidatorTrees.customers.Expression; ValidatorTrees.quote (Expression.Lambda(tree, c)) ] + + Assert.Equal([ Violation.LambdaInvocation ], reported where) + + [] + member _.``A parameter that no lambda binds is reported``() = + let c = Expression.Parameter(typeof, "c") + let n = Expression.Parameter(typeof, "n") + + let where = + ValidatorTrees.call + "Where" + [ typeof ] + [ ValidatorTrees.customers.Expression + ValidatorTrees.quote (Expression.Lambda(Expression.GreaterThan(Expression.Property(c, "Id"), n), c)) ] + + Assert.Equal([ Violation.UnboundParameter "n" ], reported where) + [] member _.``A count made by the provider passes``() = Scenario.value "count by the provider" (fun db -> db.Customers.Where(fun c -> c.Age > 26).Count()) (fun _ -> 3) @@ -260,3 +302,20 @@ type ProviderShapeValidatorTests() = Scenario.value "count by Enumerable" (fun db -> Enumerable.Count(db.Customers.Where(fun c -> c.Age > 26))) (fun _ -> 3) |> Scenario.violating [ Violation.LeftProvider ] |> Scenario.check + + [] + member _.``A count made by the client over counts made by the provider leaves it``() = + // The provider counts the orders once for each customer, and the client counts the customers + Scenario.value + "client count with a provider count in its projection" + (fun db -> db.Customers.Where(fun c -> c.Age > 26).AsEnumerable().Select(fun _ -> db.Orders.Count()).Count()) + (fun _ -> 3) + |> Scenario.violating [ Violation.LeftProvider ] + |> Scenario.check + + Scenario.value + "client count with an inner provider count" + (fun db -> Enumerable.Count(db.Customers.Select(fun _ -> db.Orders.Count()))) + (fun data -> data.Customers.Length) + |> Scenario.violating [ Violation.LeftProvider ] + |> Scenario.check From f73968d32fbf78074454230afaefa05a15dca1b3 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Mon, 2 Nov 2026 17:37:24 +0100 Subject: [PATCH 06/14] Check the elements a result selector hands on, and compare entities by 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 --- .../QueryProviderShape.fs | 38 ++++- .../Microsoft.FSharp.Linq/QueryScenario.fs | 32 ++++ .../Microsoft.FSharp.Linq/QueryShapeTests.fs | 146 ++++++++++++++++++ 3 files changed, 208 insertions(+), 8 deletions(-) diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs index 20665171824..8709087cd59 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs @@ -302,9 +302,9 @@ module ProviderShape = let private yields (elementType: Type) (source: Expression) = typedefof>.MakeGenericType(elementType).IsAssignableFrom source.Type - /// Whether the lambda gives its first parameter back as it is. - let private returnsParameter (lambda: LambdaExpression) = - lambda.Parameters.Count > 0 && obj.ReferenceEquals(lambda.Body, lambda.Parameters.[0]) + /// Whether the lambda gives the parameter at the index back as it is. + let private returnsParameterAt index (lambda: LambdaExpression) = + index < lambda.Parameters.Count && obj.ReferenceEquals(lambda.Body, lambda.Parameters.[index]) /// A query that is its own root, as opposed to one built from another by operators. let private isRoot (query: IQueryable) = @@ -354,18 +354,30 @@ module ProviderShape = | ValueSome lambda -> lambda | ValueNone -> () ] + // A function of an element of each of two sequences gives unnamed elements when it builds them without + // names, and when it gives back one of its parameters of which the sequence has unnamed elements + let unnamedResult (result: LambdaExpression) (second: Expression) = + buildsUnnamed result.Body + || (returnsParameterAt 0 result && this.Unnamed source) + || (returnsParameterAt 1 result && this.Unnamed second) + match call.Method.Name with | "Select" -> lambdas |> List.tryLast - |> Option.exists (fun selector -> buildsUnnamed selector.Body || (returnsParameter selector && this.Unnamed source)) + |> Option.exists (fun selector -> buildsUnnamed selector.Body || (returnsParameterAt 0 selector && this.Unnamed source)) | "SelectMany" -> match lambdas with | [ collection ] -> this.Unnamed collection.Body - | [ _; result ] -> buildsUnnamed result.Body + | [ collection; result ] -> unnamedResult result collection.Body | _ -> false | "Join" - | "GroupJoin" -> lambdas |> List.tryLast |> Option.exists (fun result -> buildsUnnamed result.Body) + | "Zip" -> lambdas |> List.tryLast |> Option.exists (fun result -> unnamedResult result call.Arguments.[1]) + // The second parameter is the group of the matching inner elements, which no element is + | "GroupJoin" -> + lambdas + |> List.tryLast + |> Option.exists (fun result -> buildsUnnamed result.Body || (returnsParameterAt 0 result && this.Unnamed source)) | "Where" | "OrderBy" | "OrderByDescending" @@ -437,8 +449,18 @@ module ProviderShape = |> ValueOption.iter (fun result -> report result 0 source (this.Unnamed source) report result 1 second (this.Unnamed second)) - // The first parameter of the function is the accumulated value - | "Aggregate" -> () + // The function takes the accumulated value, then an element. With no seed the accumulated value starts as + // an element, and a seed is of a type of its own, which is no element. + | "Aggregate" -> + let sourceUnnamed = this.Unnamed source + let seeded = call.Arguments.Count > 2 + + lambdaAt call (if seeded then 2 else 1) + |> ValueOption.iter (fun func -> + if not seeded then + report func 0 source sourceUnnamed + + report func 1 source sourceUnnamed) | _ -> let sourceUnnamed = this.Unnamed source diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs index 042b108c16a..9ae260a7226 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs @@ -60,6 +60,35 @@ module Scenario = else ValueNone + let rec private isPlain (ty: Type) = + ty.IsPrimitive + || ty.IsEnum + || ty = typeof + || ty = typeof + || ty = typeof + || ty = typeof + || ty = typeof + || (match Nullable.GetUnderlyingType ty with + | null -> false + | underlying -> isPlain underlying) + + /// An object of a class of the scenarios, with what it holds in the properties that give plain values. Its own + /// text says no more than the key of an entity, and a result that is wrong in any other property is not equal to + /// the right one. + [] + let private (|Entity|_|) (value: obj) = + let ty = value.GetType() + + if ty.IsClass && String.Equals(ty.Namespace, typeof.Namespace, StringComparison.Ordinal) then + ValueSome( + ty.GetProperties(BindingFlags.Public ||| BindingFlags.Instance) + |> Seq.filter (fun property -> property.GetIndexParameters().Length = 0 && isPlain property.PropertyType) + |> Seq.map (fun property -> struct (property.Name, property.GetValue value)) + |> Seq.toList + ) + else + ValueNone + /// Text that is equal for equal results, whichever provider materialized them. let rec render (value: obj) = let renderAll (values: seq) = @@ -76,6 +105,9 @@ module Scenario = let rendered = fields |> Seq.map (fun (name, field) -> $"{name} = {render field}") $"""{{ {String.concat "; " rendered} }}""" | Union(case, fields) -> $"{case}({renderAll fields})" + | Entity properties -> + let rendered = properties |> Seq.map (fun (struct (name, property)) -> $"{name} = {render property}") + $"""{value.GetType().Name} {{ {String.concat "; " rendered} }}""" | value -> string value let rows name (run: Db -> IQueryable<'T>) (expected: Data -> seq<'T>) = diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs index 35c106eedab..94b9d25edf6 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs @@ -319,3 +319,149 @@ type ProviderShapeValidatorTests() = (fun data -> data.Customers.Length) |> Scenario.violating [ Violation.LeftProvider ] |> Scenario.check + + [] + member _.``A result selector that gives back an unnamed element keeps the elements unnamed``() = + let item = Expression.Parameter(typeof, "item") + + let flattened named = + let pair = Expression.Parameter(ValidatorTrees.customerPair, "pair") + let collectionType = typedefof>.MakeGenericType(ValidatorTrees.customerPair, typeof>) + let collection = Expression.Lambda(collectionType, Expression.NewArrayInit(typeof, Expression.Constant 0), pair) + let first = Expression.Parameter(ValidatorTrees.customerPair, "pair") + + ValidatorTrees.call + "SelectMany" + [ ValidatorTrees.customerPair; typeof; ValidatorTrees.customerPair ] + [ ValidatorTrees.customerPairs named + ValidatorTrees.quote collection + ValidatorTrees.quote (Expression.Lambda(first, first, item)) ] + + Assert.Equal(unnamedRead, reported (ValidatorTrees.whereItem1Positive (flattened false) ValidatorTrees.customerPair)) + Assert.Empty(reported (ValidatorTrees.whereItem1Positive (flattened true) ValidatorTrees.customerPair)) + + [] + member _.``The second parameter of a SelectMany result selector is an element of the collection``() = + let collected named = + let customer = Expression.Parameter(typeof, "c") + let collectionType = typedefof>.MakeGenericType(typeof, typeof>>) + let collection = Expression.Lambda(collectionType, ValidatorTrees.orderPairs named, customer) + let owner = Expression.Parameter(typeof, "c") + let order = Expression.Parameter(ValidatorTrees.orderPair, "o") + + ValidatorTrees.call + "SelectMany" + [ typeof; ValidatorTrees.orderPair; ValidatorTrees.orderPair ] + [ ValidatorTrees.customers.Expression + ValidatorTrees.quote collection + ValidatorTrees.quote (Expression.Lambda(order, owner, order)) ] + + Assert.Equal(unnamedRead, reported (ValidatorTrees.whereItem1Positive (collected false) ValidatorTrees.orderPair)) + Assert.Empty(reported (ValidatorTrees.whereItem1Positive (collected true) ValidatorTrees.orderPair)) + + [] + member _.``A join or a zip that gives back an unnamed element keeps the elements unnamed``() = + let joined outerNamed innerNamed (result: ParameterExpression -> ParameterExpression -> Expression) = + ValidatorTrees.join + (ValidatorTrees.customerPairs outerNamed) + ValidatorTrees.customerPair + (ValidatorTrees.customerPairs innerNamed) + ValidatorTrees.customerPair + ValidatorTrees.zero + ValidatorTrees.zero + result + + let check tree = reported (ValidatorTrees.whereItem1Positive tree ValidatorTrees.customerPair) + + Assert.Equal(unnamedRead, check (joined false true (fun o _ -> o))) + Assert.Empty(check (joined true false (fun o _ -> o))) + Assert.Equal(unnamedRead, check (joined true false (fun _ i -> i))) + Assert.Empty(check (joined false true (fun _ i -> i))) + + [] + member _.``A zip that gives back an unnamed element keeps the elements unnamed``() = + let zipped firstNamed secondNamed = + let a = Expression.Parameter(ValidatorTrees.customerPair, "a") + let b = Expression.Parameter(ValidatorTrees.customerPair, "b") + + ValidatorTrees.whereItem1Positive + (ValidatorTrees.call + "Zip" + [ ValidatorTrees.customerPair; ValidatorTrees.customerPair; ValidatorTrees.customerPair ] + [ ValidatorTrees.customerPairs firstNamed + ValidatorTrees.customerPairs secondNamed + ValidatorTrees.quote (Expression.Lambda(a, a, b)) ]) + ValidatorTrees.customerPair + + Assert.Equal(unnamedRead, reported (zipped false true)) + Assert.Empty(reported (zipped true false)) + + [] + member _.``The element an Aggregate function takes is checked, and a seed that is accumulated is not``() = + // customers.Select(c => new Tuple(c.Id, c.Name)).Aggregate(0, (acc, pair) => acc + pair.Item1) + let summed named = + let acc = Expression.Parameter(typeof, "acc") + let pair = Expression.Parameter(ValidatorTrees.customerPair, "pair") + let func = Expression.Lambda(Expression.Add(acc, ValidatorTrees.item1 pair), acc, pair) + + ValidatorTrees.call + "Aggregate" + [ ValidatorTrees.customerPair; typeof ] + [ ValidatorTrees.customerPairs named; Expression.Constant 0; ValidatorTrees.quote func ] + + Assert.Equal(unnamedRead, reported (summed false)) + Assert.Empty(reported (summed true)) + + // The accumulated value has the type of the elements here, and a member read from it is no read from an element + let counted = + let acc = Expression.Parameter(ValidatorTrees.customerPair, "acc") + let pair = Expression.Parameter(ValidatorTrees.customerPair, "pair") + let seed = ValidatorTrees.pair ValidatorTrees.customerPair [ Expression.Constant 0; Expression.Constant "" ] true + let count = Expression.Add(ValidatorTrees.item1 acc, Expression.Constant 1) + let next = ValidatorTrees.pair ValidatorTrees.customerPair [ count; Expression.Constant "" ] true + + ValidatorTrees.call + "Aggregate" + [ ValidatorTrees.customerPair; ValidatorTrees.customerPair ] + [ ValidatorTrees.customerPairs false; seed :> Expression; ValidatorTrees.quote (Expression.Lambda(next, acc, pair)) ] + + Assert.Empty(reported counted) + + [] + member _.``Both parameters of an Aggregate function with no seed are elements``() = + // customers.Select(c => new Tuple(c.Id, c.Name)).Aggregate((a, b) => new Tuple(a.Item1 + b.Item1, a.Item2)) + let combined named = + let a = Expression.Parameter(ValidatorTrees.customerPair, "a") + let b = Expression.Parameter(ValidatorTrees.customerPair, "b") + let sum = Expression.Add(ValidatorTrees.item1 a, ValidatorTrees.item1 b) + let body = ValidatorTrees.pair ValidatorTrees.customerPair [ sum; ValidatorTrees.item2 a ] true + + ValidatorTrees.call + "Aggregate" + [ ValidatorTrees.customerPair ] + [ ValidatorTrees.customerPairs named; ValidatorTrees.quote (Expression.Lambda(body, a, b)) ] + + Assert.Equal(unnamedRead @ unnamedRead, reported (combined false)) + Assert.Empty(reported (combined true)) + + [] + member _.``An entity that is wrong in a property other than its key does not pass``() = + let fails (build: Customer -> Customer) = + let scenario = + Scenario.value + "payload" + (fun db -> + query { + for c in db.Customers do + select (build c) into p + head + }) + (fun data -> data.Customers.[0]) + + Scenario.failures scenario |> List.exists (fun failure -> failure.Contains("rows: expected", StringComparison.Ordinal)) + + let copy (c: Customer) = Customer(Id = c.Id, Name = c.Name, City = c.City, Age = c.Age, Registered = c.Registered) + + Assert.True(fails (fun c -> Customer(Id = c.Id, Name = "CORRUPTED", City = c.City, Age = -1, Registered = c.Registered))) + Assert.True(fails (fun c -> Customer(Id = c.Id, Name = c.Name, City = c.City, Age = c.Age))) + Assert.False(fails copy) From d9decc03abf4f478d2e1f210860ca5159e88a965 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Wed, 4 Nov 2026 10:23:54 +0100 Subject: [PATCH 07/14] Spell the check for a wrong row with IndexOf, which net472 has 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 --- .../FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs index 94b9d25edf6..a9cc9d9535f 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs @@ -458,7 +458,7 @@ type ProviderShapeValidatorTests() = }) (fun data -> data.Customers.[0]) - Scenario.failures scenario |> List.exists (fun failure -> failure.Contains("rows: expected", StringComparison.Ordinal)) + Scenario.failures scenario |> List.exists (fun failure -> failure.IndexOf("rows: expected", StringComparison.Ordinal) >= 0) let copy (c: Customer) = Customer(Id = c.Id, Name = c.Name, City = c.City, Age = c.Age, Registered = c.Registered) From 9f9d2f618ba3eb18a272dae012ef9ef0814af32c Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Wed, 4 Nov 2026 10:39:14 +0100 Subject: [PATCH 08/14] Render the properties of an entity in the order of their names 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 --- .../FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs index 9ae260a7226..51d51d47696 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs @@ -83,6 +83,8 @@ module Scenario = ValueSome( ty.GetProperties(BindingFlags.Public ||| BindingFlags.Instance) |> Seq.filter (fun property -> property.GetIndexParameters().Length = 0 && isPlain property.PropertyType) + // Reflection promises no order of the properties, and on .NET Framework two calls for one class gave two orders + |> Seq.sortBy (fun property -> property.Name) |> Seq.map (fun property -> struct (property.Name, property.GetValue value)) |> Seq.toList ) From affc56118dfa1b970aeec438647d2974df526bad Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 27 Sep 2026 21:04:55 +0200 Subject: [PATCH 09/14] Consume the elements a query translation produces 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 --- docs/release-notes/.FSharp.Core/11.0.200.md | 3 + src/FSharp.Core/Query.fs | 78 +++++++++++-------- .../QueryStructureScenarios.fs | 53 ++++++++++++- 3 files changed, 99 insertions(+), 35 deletions(-) diff --git a/docs/release-notes/.FSharp.Core/11.0.200.md b/docs/release-notes/.FSharp.Core/11.0.200.md index 74a8bab02a3..e5632d5373d 100644 --- a/docs/release-notes/.FSharp.Core/11.0.200.md +++ b/docs/release-notes/.FSharp.Core/11.0.200.md @@ -7,5 +7,8 @@ * Correct `FSharpValue.PreComputeTuplePropertyInfo` to report that struct tuples store elements in fields, not properties, and recommend supported tuple readers instead of reporting a valid index as out of range. ([Issue #20801](https://github.com/dotnet/fsharp/issues/20801), [PR #20815](https://github.com/dotnet/fsharp/pull/20815)) * Quotation debug text from `Expr.ToString()`, `Expr.ToString(full)`, and `%A` now prints struct tuple construction as `NewStructTuple`; reference tuples remain `NewTuple`. ([Issue #20800](https://github.com/dotnet/fsharp/issues/20800), [PR #20814](https://github.com/dotnet/fsharp/pull/20814)) * Fix struct tuples in query expressions: selecting one no longer fails with "unreachable", and the items of one that a later operator reads are taken from its fields instead of failing with `ArgumentException: Struct tuples store their elements in fields, not properties`. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522), [PR #20894](https://github.com/dotnet/fsharp/pull/20894)) +* Fix struct tuples in query expressions and in lambdas converted to LINQ expression trees: selecting one no longer fails with "unreachable", and taking one apart no longer fails with a tuple index out of range. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522), [PR #20894](https://github.com/dotnet/fsharp/pull/20894)) +* Fix LINQ providers failing on, or evaluating on the client, queries that end by consuming the tuples or records a query produces: `exists`, `all`, `sumBy`, `averageBy`, `minBy`, `maxBy` and `count` after two `for`s, a `join` or a tuple or record `select` now work on the query's own elements instead of on tuples and records rebuilt at the end of the query. +* Fix struct tuples in query expressions and in lambdas converted to LINQ expression trees: selecting one no longer fails with "unreachable", and taking one apart no longer fails with a tuple index out of range. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522)) ### Changed diff --git a/src/FSharp.Core/Query.fs b/src/FSharp.Core/Query.fs index 56171c629f9..29727aaf2d5 100644 --- a/src/FSharp.Core/Query.fs +++ b/src/FSharp.Core/Query.fs @@ -1836,29 +1836,43 @@ module Query = result + /// Evaluate the inner core of a query whose elements a function consumes. The function is rewritten over the + /// elements as the query translation produces them, so they are not converted back to immutable tuples and + /// records for it. + let EvalNonNestedInnerWithConsumer (queryProducingSequence: Expr) (immutConsumingVar: Var, immutConsumingExpr: Expr) = + let mutSource, _, mutConsumingVar, mutConsumingExpr = + TransInnerApplicativeAndCommit true queryProducingSequence (immutConsumingVar, immutConsumingExpr) + EvaluateQuotation (EliminateNestedQueries mutSource), mutConsumingVar, mutConsumingExpr + + /// Evaluate the inner core of a query whose elements are only counted, with the type of the elements as the + /// query translation produces them. + let EvalNonNestedInnerForCount (queryProducingSequence: Expr) (immutElemTy: Type) = + let mutSource, conv = TransInnerAndCommit CanEliminate.Yes true queryProducingSequence + EvaluateQuotation (EliminateNestedQueries mutSource), ConvImmutableTypeToMutableType conv immutElemTy + /// Evaluate the outer calls of a query until the inner core that actually produces a sequence of results is reached. let rec EvalNonNestedOuter canElim (tm: Expr) = match tm with - | CallMinBy (_, [srcItemTy; qTy; keyItemTy], source, Lambda(v, valSelector)) -> - let sourcev = EvalNonNestedInner CanEliminate.Yes source - CallMinBy (qTyIsIQueryable qTy, srcItemTy, keyItemTy, sourcev, keyItemTy, v, MacroExpand valSelector) + | CallMinBy (_, [_; qTy; keyItemTy], source, Lambda(v, valSelector)) -> + let sourcev, v, valSelector = EvalNonNestedInnerWithConsumer source (v, valSelector) + CallMinBy (qTyIsIQueryable qTy, v.Type, keyItemTy, sourcev, keyItemTy, v, valSelector) - | CallMaxBy (_, [srcItemTy; qTy; keyItemTy], source, Lambda(v, valSelector)) -> - let sourcev = EvalNonNestedInner CanEliminate.Yes source - CallMaxBy (qTyIsIQueryable qTy, srcItemTy, keyItemTy, sourcev, keyItemTy, v, MacroExpand valSelector) + | CallMaxBy (_, [_; qTy; keyItemTy], source, Lambda(v, valSelector)) -> + let sourcev, v, valSelector = EvalNonNestedInnerWithConsumer source (v, valSelector) + CallMaxBy (qTyIsIQueryable qTy, v.Type, keyItemTy, sourcev, keyItemTy, v, valSelector) - | CallMinByNullable (_, [srcItemTy; qTy; keyItemTy ], source, Lambda(v, valSelector)) -> - let sourcev = EvalNonNestedInner CanEliminate.Yes source - CallMinByNullable (qTyIsIQueryable qTy, srcItemTy, keyItemTy, sourcev, MakeNullableTy keyItemTy, v, MacroExpand valSelector) + | CallMinByNullable (_, [_; qTy; keyItemTy ], source, Lambda(v, valSelector)) -> + let sourcev, v, valSelector = EvalNonNestedInnerWithConsumer source (v, valSelector) + CallMinByNullable (qTyIsIQueryable qTy, v.Type, keyItemTy, sourcev, MakeNullableTy keyItemTy, v, valSelector) - | CallMaxByNullable (_, [srcItemTy; qTy; keyItemTy ], source, Lambda(v, valSelector)) -> - let sourcev = EvalNonNestedInner CanEliminate.Yes source - CallMaxByNullable (qTyIsIQueryable qTy, srcItemTy, keyItemTy, sourcev, MakeNullableTy keyItemTy, v, MacroExpand valSelector) + | CallMaxByNullable (_, [_; qTy; keyItemTy ], source, Lambda(v, valSelector)) -> + let sourcev, v, valSelector = EvalNonNestedInnerWithConsumer source (v, valSelector) + CallMaxByNullable (qTyIsIQueryable qTy, v.Type, keyItemTy, sourcev, MakeNullableTy keyItemTy, v, valSelector) | CallCount (_, [srcItemTy; qTy], source) -> - let sourcev = EvalNonNestedInner CanEliminate.Yes source - CallCount (qTyIsIQueryable qTy, srcItemTy, sourcev) + let sourcev, mutSrcItemTy = EvalNonNestedInnerForCount source srcItemTy + CallCount (qTyIsIQueryable qTy, mutSrcItemTy, sourcev) | CallHead (_, [srcItemTy; qTy], source) -> let sourcev = EvalNonNestedInner CanEliminate.Yes source @@ -1884,29 +1898,29 @@ module Query = let sourcev = EvalNonNestedInner CanEliminate.Yes source CallSingleOrDefault(qTyIsIQueryable qTy, srcItemTy, sourcev) - | CallAverageBy (qb, [srcItemTy; qTy; resTy], source, Lambda(v, valSelector)) -> - let sourcev = EvalNonNestedInner CanEliminate.Yes source - CallAverageBy (qb, qTyIsIQueryable qTy, srcItemTy, resTy, sourcev, resTy, v, MacroExpand valSelector) + | CallAverageBy (qb, [_; qTy; resTy], source, Lambda(v, valSelector)) -> + let sourcev, v, valSelector = EvalNonNestedInnerWithConsumer source (v, valSelector) + CallAverageBy (qb, qTyIsIQueryable qTy, v.Type, resTy, sourcev, resTy, v, valSelector) - | CallAverageByNullable (qb, [srcItemTy; qTy; resTyNoNullable], source, Lambda(v, valSelector)) -> - let sourcev = EvalNonNestedInner CanEliminate.Yes source - CallAverageByNullable(qb, qTyIsIQueryable qTy, srcItemTy, resTyNoNullable, sourcev, MakeNullableTy resTyNoNullable, v, MacroExpand valSelector) + | CallAverageByNullable (qb, [_; qTy; resTyNoNullable], source, Lambda(v, valSelector)) -> + let sourcev, v, valSelector = EvalNonNestedInnerWithConsumer source (v, valSelector) + CallAverageByNullable(qb, qTyIsIQueryable qTy, v.Type, resTyNoNullable, sourcev, MakeNullableTy resTyNoNullable, v, valSelector) - | CallSumBy (qb, [srcItemTy; qTy; resTy], source, Lambda(v, valSelector)) -> - let sourcev = EvalNonNestedInner CanEliminate.Yes source - CallSumBy (qb, qTyIsIQueryable qTy, srcItemTy, resTy, sourcev, resTy, v, MacroExpand valSelector) + | CallSumBy (qb, [_; qTy; resTy], source, Lambda(v, valSelector)) -> + let sourcev, v, valSelector = EvalNonNestedInnerWithConsumer source (v, valSelector) + CallSumBy (qb, qTyIsIQueryable qTy, v.Type, resTy, sourcev, resTy, v, valSelector) - | CallSumByNullable (qb, [srcItemTy; qTy; resTyNoNullable], source, Lambda(v, valSelector)) -> - let sourcev = EvalNonNestedInner CanEliminate.Yes source - CallSumByNullable (qb, qTyIsIQueryable qTy, srcItemTy, resTyNoNullable, sourcev, MakeNullableTy resTyNoNullable, v, MacroExpand valSelector) + | CallSumByNullable (qb, [_; qTy; resTyNoNullable], source, Lambda(v, valSelector)) -> + let sourcev, v, valSelector = EvalNonNestedInnerWithConsumer source (v, valSelector) + CallSumByNullable (qb, qTyIsIQueryable qTy, v.Type, resTyNoNullable, sourcev, MakeNullableTy resTyNoNullable, v, valSelector) - | CallExists (_, [srcItemTy; qTy], source, Lambda(v, predicate)) -> - let sourcev = EvalNonNestedInner CanEliminate.Yes source - CallAny (qTyIsIQueryable qTy, srcItemTy, sourcev, v, MacroExpand predicate) + | CallExists (_, [_; qTy], source, Lambda(v, predicate)) -> + let sourcev, v, predicate = EvalNonNestedInnerWithConsumer source (v, predicate) + CallAny (qTyIsIQueryable qTy, v.Type, sourcev, v, predicate) - | CallForAll (_, [srcItemTy; qTy], source, Lambda(v, predicate)) -> - let sourcev = EvalNonNestedInner CanEliminate.Yes source - CallAll (qTyIsIQueryable qTy, srcItemTy, sourcev, v, MacroExpand predicate) + | CallForAll (_, [_; qTy], source, Lambda(v, predicate)) -> + let sourcev, v, predicate = EvalNonNestedInnerWithConsumer source (v, predicate) + CallAll (qTyIsIQueryable qTy, v.Type, sourcev, v, predicate) | CallFind (_, [srcItemTy; qTy], source, Lambda(v, f)) -> let sourcev = EvalNonNestedInner CanEliminate.Yes source diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs index 6f806f4bb43..a75601888e2 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs @@ -154,8 +154,7 @@ let private loops () = Scenario.value "exists over two for" (fun db -> query { for c in db.Customers do for o in c.Orders do exists (o.Total > 10.0 && c.City = "London") }) - (fun _ -> true) - |> Scenario.violating [ Violation.ReadsUnnamedMember "Tuple`2" ] ] + (fun _ -> true) ] let private subqueries () = [ Scenario.rows @@ -265,4 +264,52 @@ let private leftJoins () = (leftJoinedOrdersAndProducts >> Seq.filter (fun (_, _, product) -> product <> "")) |> Scenario.violating (nested [ "Join" ]) ] -let cases () = loops () @ subqueries () @ leftJoins () +let private consumers () = + let totals (data: Data) = data.Orders |> List.map _.Total + let discounts (data: Data) = data.Lines |> List.filter _.Discount.HasValue |> List.map _.Discount.Value + + [ Scenario.value + "all over two for" + (fun db -> query { for c in db.Customers do for o in c.Orders do all (o.Total > 4.0 || c.City = "Paris") }) + (fun data -> data.Customers |> List.forall (fun c -> c.Orders |> Seq.forall (fun o -> o.Total > 4.0 || c.City = "Paris"))) + + Scenario.value "sumBy over two for" (fun db -> query { for c in db.Customers do for o in c.Orders do sumBy o.Total }) (totals >> List.sum) + + Scenario.value + "averageBy after a join" + (fun db -> query { for c in db.Customers do join o in db.Orders on (c.Id = o.CustomerId); averageBy o.Total }) + (totals >> List.average) + + Scenario.value + "minByNullable over two for" + (fun db -> query { for o in db.Orders do for l in o.Lines do minByNullable l.Discount }) + (discounts >> List.min >> System.Nullable) + + Scenario.value + "sumByNullable after a join" + (fun db -> query { for o in db.Orders do join l in db.Lines on (o.Id = l.OrderId); sumByNullable l.Discount }) + (discounts >> List.sum >> System.Nullable) + + Scenario.value + "maxBy after a record select" + (fun db -> query { for c in db.Customers do select { PersonName = c.Name; PersonAge = c.Age } into p; maxBy p.PersonAge }) + (fun data -> data.Customers |> List.map _.Age |> List.max) + + Scenario.value + "exists after a record select" + (fun db -> query { for c in db.Customers do select { PersonName = c.Name; PersonAge = c.Age } into p; exists (p.PersonAge > 30) }) + (fun _ -> true) + + Scenario.value + "exists after a tuple select" + (fun db -> query { for c in db.Customers do select (c.City, c.Age) into t; exists (match t with city, age -> city = "Paris" && age < 30) }) + (fun _ -> true) + |> Scenario.violating [ Violation.Statement System.Linq.Expressions.ExpressionType.Block ] + + Scenario.value + "count after a tuple select" + (fun db -> query { for c in db.Customers do select (c.City, c.Age) into t; count }) + (fun data -> data.Customers.Length) + |> Scenario.flat [ "Select"; "Select"; "Count" ] ] + +let cases () = loops () @ subqueries () @ leftJoins () @ consumers () From 37e7daf65f657cb19ceabffa87d4d271812836b0 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 1 Nov 2026 22:18:14 +0100 Subject: [PATCH 10/14] Link the release note to the pull request Co-Authored-By: Claude Sonnet 5.5 --- docs/release-notes/.FSharp.Core/11.0.200.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/release-notes/.FSharp.Core/11.0.200.md b/docs/release-notes/.FSharp.Core/11.0.200.md index e5632d5373d..5198a9fbc32 100644 --- a/docs/release-notes/.FSharp.Core/11.0.200.md +++ b/docs/release-notes/.FSharp.Core/11.0.200.md @@ -8,7 +8,6 @@ * Quotation debug text from `Expr.ToString()`, `Expr.ToString(full)`, and `%A` now prints struct tuple construction as `NewStructTuple`; reference tuples remain `NewTuple`. ([Issue #20800](https://github.com/dotnet/fsharp/issues/20800), [PR #20814](https://github.com/dotnet/fsharp/pull/20814)) * Fix struct tuples in query expressions: selecting one no longer fails with "unreachable", and the items of one that a later operator reads are taken from its fields instead of failing with `ArgumentException: Struct tuples store their elements in fields, not properties`. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522), [PR #20894](https://github.com/dotnet/fsharp/pull/20894)) * Fix struct tuples in query expressions and in lambdas converted to LINQ expression trees: selecting one no longer fails with "unreachable", and taking one apart no longer fails with a tuple index out of range. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522), [PR #20894](https://github.com/dotnet/fsharp/pull/20894)) -* Fix LINQ providers failing on, or evaluating on the client, queries that end by consuming the tuples or records a query produces: `exists`, `all`, `sumBy`, `averageBy`, `minBy`, `maxBy` and `count` after two `for`s, a `join` or a tuple or record `select` now work on the query's own elements instead of on tuples and records rebuilt at the end of the query. -* Fix struct tuples in query expressions and in lambdas converted to LINQ expression trees: selecting one no longer fails with "unreachable", and taking one apart no longer fails with a tuple index out of range. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522)) +* Fix LINQ providers failing on, or evaluating on the client, queries that end by consuming the tuples or records a query produces: `exists`, `all`, `sumBy`, `averageBy`, `minBy`, `maxBy` and `count` after two `for`s, a `join` or a tuple or record `select` now work on the query's own elements instead of on tuples and records rebuilt at the end of the query. ([PR #20901](https://github.com/dotnet/fsharp/pull/20901)) ### Changed From f9bfc8e7d7479c5e18173eaec8b1239455bc4f25 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 1 Nov 2026 23:34:15 +0100 Subject: [PATCH 11/14] Hand a function the one object the query produced when it uses it whole 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 --- src/FSharp.Core/Query.fs | 36 ++++++++++++++++--- .../QueryStructureScenarios.fs | 25 +++++++++++++ 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/src/FSharp.Core/Query.fs b/src/FSharp.Core/Query.fs index 29727aaf2d5..f420b0869c1 100644 --- a/src/FSharp.Core/Query.fs +++ b/src/FSharp.Core/Query.fs @@ -1836,13 +1836,41 @@ module Query = result + /// A tuple or record of a class, which the translation builds anew from the elements it produces each time one + /// is asked for + let IsRebuiltObject (ty: Type) = + not ty.IsValueType + && (Microsoft.FSharp.Reflection.FSharpType.IsTuple ty || isPartiallyImmutableRecord ty) + + /// Whether the expression uses the element, or a tuple or record in it, as a value of its own, and not only for + /// what it holds. Where the function is rewritten over the elements the translation produces, each such use + /// is an object of its own, and not the one object the element is. + let rec UsesWholeElement (element: Var) (expr: Expr) = + let rec readsPartOfElement (expr: Expr) = + match expr with + | Var used -> used = element + | TupleGet (inner, _) + | PropertyGet (Some inner, _, []) -> readsPartOfElement inner + | _ -> false + + if readsPartOfElement expr then + IsRebuiltObject expr.Type + else + match expr with + | ExprShape.ShapeVar _ -> false + | ExprShape.ShapeLambda (_, body) -> UsesWholeElement element body + | ExprShape.ShapeCombination (_, args) -> List.exists (UsesWholeElement element) args + /// Evaluate the inner core of a query whose elements a function consumes. The function is rewritten over the /// elements as the query translation produces them, so they are not converted back to immutable tuples and - /// records for it. + /// records for it. A function that uses an element as a value of its own gets the elements as they are. let EvalNonNestedInnerWithConsumer (queryProducingSequence: Expr) (immutConsumingVar: Var, immutConsumingExpr: Expr) = - let mutSource, _, mutConsumingVar, mutConsumingExpr = - TransInnerApplicativeAndCommit true queryProducingSequence (immutConsumingVar, immutConsumingExpr) - EvaluateQuotation (EliminateNestedQueries mutSource), mutConsumingVar, mutConsumingExpr + if UsesWholeElement immutConsumingVar immutConsumingExpr then + EvalNonNestedInner CanEliminate.Yes queryProducingSequence, immutConsumingVar, MacroExpand immutConsumingExpr + else + let mutSource, _, mutConsumingVar, mutConsumingExpr = + TransInnerApplicativeAndCommit true queryProducingSequence (immutConsumingVar, immutConsumingExpr) + EvaluateQuotation (EliminateNestedQueries mutSource), mutConsumingVar, mutConsumingExpr /// Evaluate the inner core of a query whose elements are only counted, with the type of the elements as the /// query translation produces them. diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs index a75601888e2..c641177574f 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs @@ -1,5 +1,8 @@ // Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. +// Suppress FS3220: a scenario reads the items of a System.Tuple by name, as the queries it checks do +#nowarn "3220" + module FSharp.Core.UnitTests.Linq.QueryStructureScenarios open System.Linq @@ -300,6 +303,28 @@ let private consumers () = (fun db -> query { for c in db.Customers do select { PersonName = c.Name; PersonAge = c.Age } into p; exists (p.PersonAge > 30) }) (fun _ -> true) + // A function that uses an element as a value of its own gets the one object the query produced + Scenario.value + "exists after a record select, the record compared with itself" + (fun db -> query { for c in db.Customers do select { PersonName = c.Name; PersonAge = c.Age } into p; exists (obj.ReferenceEquals(p, p)) }) + (fun _ -> true) + + Scenario.value + "all after a tuple select, the tuple compared with itself" + (fun db -> query { for c in db.Customers do select (c.Name, c.Age) into t; all (obj.ReferenceEquals(t, t)) }) + (fun _ -> true) + + Scenario.value + "sumBy after a tuple select, a tuple in it compared with itself" + (fun db -> + query { + for c in db.Customers do + select (c.Id, (c.Name, c.Age)) into t + sumBy (if obj.ReferenceEquals(t.Item2, t.Item2) then 1 else 0) + }) + (fun data -> data.Customers.Length) + |> Scenario.violating [ Violation.ReadsUnnamedMember "Tuple`2" ] + Scenario.value "exists after a tuple select" (fun db -> query { for c in db.Customers do select (c.City, c.Age) into t; exists (match t with city, age -> city = "Paris" && age < 30) }) From f708e2cf9374b2ffb98cb7940aeace8d9aaa64a3 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Mon, 2 Nov 2026 14:58:34 +0100 Subject: [PATCH 12/14] Count a member that is no field, and a struct that holds a rebuilt object, 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 --- docs/release-notes/.FSharp.Core/11.0.200.md | 1 - src/FSharp.Core/Query.fs | 27 ++++++++++--- .../QueryStructureScenarios.fs | 38 ++++++++++++++++++- 3 files changed, 59 insertions(+), 7 deletions(-) diff --git a/docs/release-notes/.FSharp.Core/11.0.200.md b/docs/release-notes/.FSharp.Core/11.0.200.md index 5198a9fbc32..5021c27c1ec 100644 --- a/docs/release-notes/.FSharp.Core/11.0.200.md +++ b/docs/release-notes/.FSharp.Core/11.0.200.md @@ -7,7 +7,6 @@ * Correct `FSharpValue.PreComputeTuplePropertyInfo` to report that struct tuples store elements in fields, not properties, and recommend supported tuple readers instead of reporting a valid index as out of range. ([Issue #20801](https://github.com/dotnet/fsharp/issues/20801), [PR #20815](https://github.com/dotnet/fsharp/pull/20815)) * Quotation debug text from `Expr.ToString()`, `Expr.ToString(full)`, and `%A` now prints struct tuple construction as `NewStructTuple`; reference tuples remain `NewTuple`. ([Issue #20800](https://github.com/dotnet/fsharp/issues/20800), [PR #20814](https://github.com/dotnet/fsharp/pull/20814)) * Fix struct tuples in query expressions: selecting one no longer fails with "unreachable", and the items of one that a later operator reads are taken from its fields instead of failing with `ArgumentException: Struct tuples store their elements in fields, not properties`. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522), [PR #20894](https://github.com/dotnet/fsharp/pull/20894)) -* Fix struct tuples in query expressions and in lambdas converted to LINQ expression trees: selecting one no longer fails with "unreachable", and taking one apart no longer fails with a tuple index out of range. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522), [PR #20894](https://github.com/dotnet/fsharp/pull/20894)) * Fix LINQ providers failing on, or evaluating on the client, queries that end by consuming the tuples or records a query produces: `exists`, `all`, `sumBy`, `averageBy`, `minBy`, `maxBy` and `count` after two `for`s, a `join` or a tuple or record `select` now work on the query's own elements instead of on tuples and records rebuilt at the end of the query. ([PR #20901](https://github.com/dotnet/fsharp/pull/20901)) ### Changed diff --git a/src/FSharp.Core/Query.fs b/src/FSharp.Core/Query.fs index f420b0869c1..d7f7631e6d8 100644 --- a/src/FSharp.Core/Query.fs +++ b/src/FSharp.Core/Query.fs @@ -1838,9 +1838,26 @@ module Query = /// A tuple or record of a class, which the translation builds anew from the elements it produces each time one /// is asked for - let IsRebuiltObject (ty: Type) = - not ty.IsValueType - && (Microsoft.FSharp.Reflection.FSharpType.IsTuple ty || isPartiallyImmutableRecord ty) + let rec IsRebuiltObject (ty: Type) = + if ty.IsValueType then + // A struct holds an object that is rebuilt when one of its parts is one + (Microsoft.FSharp.Reflection.FSharpType.IsTuple ty + && Microsoft.FSharp.Reflection.FSharpType.GetTupleElements ty |> Array.exists IsRebuiltObject) + || (isPartiallyImmutableRecord ty + && Microsoft.FSharp.Reflection.FSharpType.GetRecordFields ty + |> Array.exists (fun field -> IsRebuiltObject field.PropertyType)) + else + Microsoft.FSharp.Reflection.FSharpType.IsTuple ty || isPartiallyImmutableRecord ty + + /// Whether the property is a part of the record or tuple it is read from, and not a member that computes something + /// from it: such a member gets the object itself + let IsPartOf (owner: Type) (property: System.Reflection.PropertyInfo) = + if Microsoft.FSharp.Reflection.FSharpType.IsRecord owner then + Microsoft.FSharp.Reflection.FSharpType.GetRecordFields owner + |> Array.exists (fun field -> field.Name = property.Name) + else + Microsoft.FSharp.Reflection.FSharpType.IsTuple owner + && property.Name.StartsWith("Item", StringComparison.Ordinal) /// Whether the expression uses the element, or a tuple or record in it, as a value of its own, and not only for /// what it holds. Where the function is rewritten over the elements the translation produces, each such use @@ -1849,8 +1866,8 @@ module Query = let rec readsPartOfElement (expr: Expr) = match expr with | Var used -> used = element - | TupleGet (inner, _) - | PropertyGet (Some inner, _, []) -> readsPartOfElement inner + | TupleGet (inner, _) -> readsPartOfElement inner + | PropertyGet (Some inner, property, []) when IsPartOf inner.Type property -> readsPartOfElement inner | _ -> false if readsPartOfElement expr then diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs index c641177574f..821c1f6e9ab 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs @@ -7,6 +7,24 @@ module FSharp.Core.UnitTests.Linq.QueryStructureScenarios open System.Linq +/// A record whose member gives the record itself +type SelfRecord = + { SelfN: int } + member x.Identity: obj = x + +/// A struct with a tuple in it +[] +type StructPair = { SPair: int * int } + +/// A record with a struct in it +type StructHolder = { Wrapped: StructPair } + +let private pairOf (x: StructPair) = x.SPair + +/// A tree is pinned with every block it has: one that goes from three to two is a change as well +let private blocks count = + List.replicate count (Violation.Statement System.Linq.Expressions.ExpressionType.Block) + let private quotedLambda = [ Violation.RuntimeHelperCall "LeafExpressionConverter.QuotationToLambdaExpression" Violation.RuntimeHelperCall "LeafExpressionConverter.SubstHelper" ] @@ -309,6 +327,24 @@ let private consumers () = (fun db -> query { for c in db.Customers do select { PersonName = c.Name; PersonAge = c.Age } into p; exists (obj.ReferenceEquals(p, p)) }) (fun _ -> true) + // A member that is no field of the record gets the record itself + Scenario.value + "exists after a record select, a member of the record that gives the record itself" + (fun db -> query { for c in db.Customers do select { SelfN = c.Id } into p; exists (obj.ReferenceEquals(p.Identity, p.Identity)) }) + (fun _ -> true) + |> Scenario.violating [ Violation.ReadsUnnamedMember "SelfRecord" ] + + Scenario.value + "exists after a record select, a struct in it with a tuple in it compared with itself" + (fun db -> + query { + for c in db.Customers do + select { Wrapped = { SPair = (c.Id, c.Age) } } into p + exists (obj.ReferenceEquals(pairOf p.Wrapped, pairOf p.Wrapped)) + }) + (fun _ -> true) + |> Scenario.violating [ Violation.ReadsUnnamedMember "StructHolder" ] + Scenario.value "all after a tuple select, the tuple compared with itself" (fun db -> query { for c in db.Customers do select (c.Name, c.Age) into t; all (obj.ReferenceEquals(t, t)) }) @@ -329,7 +365,7 @@ let private consumers () = "exists after a tuple select" (fun db -> query { for c in db.Customers do select (c.City, c.Age) into t; exists (match t with city, age -> city = "Paris" && age < 30) }) (fun _ -> true) - |> Scenario.violating [ Violation.Statement System.Linq.Expressions.ExpressionType.Block ] + |> Scenario.violating (blocks 2) Scenario.value "count after a tuple select" From a87ac8c962f7b39f50ee4c2ce07ab3767b8a73d0 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Mon, 2 Nov 2026 17:39:03 +0100 Subject: [PATCH 13/14] Keep a nested query that a consumer mentions twice one sequence 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 --- src/FSharp.Core/Query.fs | 6 ++-- .../QueryStructureScenarios.fs | 33 +++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/FSharp.Core/Query.fs b/src/FSharp.Core/Query.fs index d7f7631e6d8..d66be4a5587 100644 --- a/src/FSharp.Core/Query.fs +++ b/src/FSharp.Core/Query.fs @@ -1837,7 +1837,7 @@ module Query = result /// A tuple or record of a class, which the translation builds anew from the elements it produces each time one - /// is asked for + /// is asked for, and a sequence of them, which it selects anew over those elements let rec IsRebuiltObject (ty: Type) = if ty.IsValueType then // A struct holds an object that is rebuilt when one of its parts is one @@ -1847,7 +1847,9 @@ module Query = && Microsoft.FSharp.Reflection.FSharpType.GetRecordFields ty |> Array.exists (fun field -> IsRebuiltObject field.PropertyType)) else - Microsoft.FSharp.Reflection.FSharpType.IsTuple ty || isPartiallyImmutableRecord ty + Microsoft.FSharp.Reflection.FSharpType.IsTuple ty + || isPartiallyImmutableRecord ty + || ((IsIEnumerableTy ty || IsIQueryableTy ty) && IsRebuiltObject (ty.GetGenericArguments().[0])) /// Whether the property is a part of the record or tuple it is read from, and not a member that computes something /// from it: such a member gets the object itself diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs index 821c1f6e9ab..970b920ea66 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs @@ -21,6 +21,16 @@ type StructHolder = { Wrapped: StructPair } let private pairOf (x: StructPair) = x.SPair +/// A record that is changed through any copy of the struct that holds it +type SharedChild = { SharedId: int; mutable State: int } + +[] +type SharedHolder = { Child: SharedChild } + +let private changeThenRead (x: SharedHolder) (y: SharedHolder) = + x.Child.State <- 99 + y.Child.State + /// A tree is pinned with every block it has: one that goes from three to two is a change as well let private blocks count = List.replicate count (Violation.Statement System.Linq.Expressions.ExpressionType.Block) @@ -345,6 +355,29 @@ let private consumers () = (fun _ -> true) |> Scenario.violating [ Violation.ReadsUnnamedMember "StructHolder" ] + // Copies of a struct hold the same objects + Scenario.value + "sumBy after a struct select, a record in it changed through the struct passed twice" + (fun db -> + query { + for c in db.Customers do + select { Child = { SharedId = c.Id; State = 0 } } into p + sumBy (changeThenRead p p) + }) + (fun data -> 99 * data.Customers.Length) + + // A nested query that a function gets from the element is one sequence, however often it mentions it + Scenario.value + "exists with a nested query, the nested query compared with itself" + (fun db -> + query { + for c in db.Customers do + let orders = query { for o in db.Orders do where (o.CustomerId = c.Id); select (o.Id, o.Total) } + exists (obj.ReferenceEquals(orders, orders)) + }) + (fun _ -> true) + |> Scenario.violating [ Violation.ReadsUnnamedMember "Tuple`2" ] + Scenario.value "all after a tuple select, the tuple compared with itself" (fun db -> query { for c in db.Customers do select (c.Name, c.Age) into t; all (obj.ReferenceEquals(t, t)) }) From a2ff09c344e526829049f20ba2c880aae08d4e4a Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Wed, 4 Nov 2026 10:52:08 +0100 Subject: [PATCH 14/14] Follow the elements of a sequence to the objects the translation converts 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 --- src/FSharp.Core/Query.fs | 16 +++++++++++-- .../QueryStructureScenarios.fs | 23 +++++++++++++++++++ .../Microsoft.FSharp.Linq/QueryTests.fs | 14 +++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/FSharp.Core/Query.fs b/src/FSharp.Core/Query.fs index d66be4a5587..fef467bfcc1 100644 --- a/src/FSharp.Core/Query.fs +++ b/src/FSharp.Core/Query.fs @@ -1836,8 +1836,20 @@ module Query = result + /// Whether the translation turns the elements of this type into anonymous objects, and builds them again from those: + /// tuples and records, of a class or of a struct, groups of such elements, and sequences of any of these. Only the + /// type arguments of the sequences and groups are followed, so that a type which holds a sequence of itself ends + /// the search at once. + let rec IsConvertedElement (ty: Type) = + Microsoft.FSharp.Reflection.FSharpType.IsTuple ty + || isPartiallyImmutableRecord ty + || ((IsIEnumerableTy ty || IsIQueryableTy ty) && IsConvertedElement (ty.GetGenericArguments().[0])) + || (ty.IsGenericType + && ty.GetGenericTypeDefinition() = typedefof> + && IsConvertedElement (ty.GetGenericArguments().[1])) + /// A tuple or record of a class, which the translation builds anew from the elements it produces each time one - /// is asked for, and a sequence of them, which it selects anew over those elements + /// is asked for, and a sequence of converted elements, which it selects anew over those elements let rec IsRebuiltObject (ty: Type) = if ty.IsValueType then // A struct holds an object that is rebuilt when one of its parts is one @@ -1849,7 +1861,7 @@ module Query = else Microsoft.FSharp.Reflection.FSharpType.IsTuple ty || isPartiallyImmutableRecord ty - || ((IsIEnumerableTy ty || IsIQueryableTy ty) && IsRebuiltObject (ty.GetGenericArguments().[0])) + || ((IsIEnumerableTy ty || IsIQueryableTy ty) && IsConvertedElement (ty.GetGenericArguments().[0])) /// Whether the property is a part of the record or tuple it is read from, and not a member that computes something /// from it: such a member gets the object itself diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs index 970b920ea66..dd704e083d5 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs @@ -378,6 +378,29 @@ let private consumers () = (fun _ -> true) |> Scenario.violating [ Violation.ReadsUnnamedMember "Tuple`2" ] + // The groups of a nested query are built anew from the groups of the provider, like its tuples + Scenario.value + "exists with a nested query of groups, the nested query compared with itself" + (fun db -> + query { + for c in db.Customers do + let groups = query { for o in db.Orders do where (o.CustomerId = c.Id); groupValBy (o.Id, o.Total) o.CustomerId } + exists (obj.ReferenceEquals(groups, groups)) + }) + (fun _ -> true) + |> Scenario.violating [ Violation.RuntimeHelperCall "new Grouping`2"; Violation.ReadsUnnamedMember "Tuple`2" ] + + Scenario.value + "exists with a nested query of struct tuples, the nested query compared with itself" + (fun db -> + query { + for c in db.Customers do + let orders = query { for o in db.Orders do where (o.CustomerId = c.Id); select (struct (o.Id, o.Total)) } + exists (obj.ReferenceEquals(orders, orders)) + }) + (fun _ -> true) + |> Scenario.violating [ Violation.ReadsUnnamedMember "Tuple`2" ] + Scenario.value "all after a tuple select, the tuple compared with itself" (fun db -> query { for c in db.Customers do select (c.Name, c.Age) into t; all (obj.ReferenceEquals(t, t)) }) diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryTests.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryTests.fs index faebf58c869..32eac05bd86 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryTests.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryTests.fs @@ -27,6 +27,10 @@ module TestData = { Id = 2; DeptName = "Marketing" } ] + /// A struct that holds a sequence of itself + [] + type Node = { Children: seq } + type Table1 = { Id1: int; Id2: int; Value1: string } type Table2 = { Id1: int; Id2: int; Value2: string } @@ -495,3 +499,13 @@ type QueryHeadOrDefaultTests() = // The result is null (default for reference type tuple) // Accessing .Item1/.Item2 would cause NRE - this is the known issue Assert.True(obj.ReferenceEquals(null, result)) + +type QueryRecursiveTypeTests() = + + [] + member _.``exists over a struct that holds a sequence of itself``() = + let empty = Queryable.AsQueryable Seq.empty + let single = Queryable.AsQueryable [ { TestData.Node.Children = Seq.empty } ] + + Assert.False(query { for n in empty do exists (Seq.isEmpty n.Children) }) + Assert.True(query { for n in single do exists (Seq.isEmpty n.Children) })