Translate nested for and joins into one flat LINQ chain

docs/release-notes/.FSharp.Core/11.0.200.md

@@ -6,5 +6,7 @@
 
 * 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 LINQ providers failing on, or evaluating on the client, queries with a `for`, `join`, `groupJoin` or `leftOuterJoin` after two `for`s: the query now continues one flat chain of `SelectMany`, `Join` and `GroupJoin`, as C# builds it, instead of nesting the rest of the query inside the first `SelectMany`, which EF Core could translate only as a lateral join, if at all. ([Issue #8459](https://github.com/dotnet/fsharp/issues/8459), [PR #20902](https://github.com/dotnet/fsharp/pull/20902))
 
 ### Changed

docs/release-notes/PENDING_BREAKING_CHANGES.md

@@ -52,6 +52,20 @@ a.Equals(b)  // Was: false, Now: true
 // New: if (node.NodeType == ExpressionType.ArrayIndex)
 ```
 
+### A `for` or join after two `for`s continues the chain
+
+Over an `IQueryable`, a query that goes on after two `for`s no longer nests the rest inside the collection selector of the
+first `SelectMany`; like C#, it pairs the elements of the first two sources and continues from the pairs.
+
+```fsharp
+query { for c in db.Customers do for o in c.Orders do for l in o.Lines do select (c.Name, l.Id) }
+// Was: Customers.SelectMany(c => c.Orders.SelectMany(o => o.Lines, ...), (c, x) => x)
+// Now: Customers.SelectMany(c => c.Orders, (c, o) => new AnonymousObject(c, o)).SelectMany(pair => pair.Item2.Lines, ...)
+```
+
+**Impact:** Code that walks the expression tree of such a query sees a longer chain and no nested `SelectMany`, `Join`
+or `GroupJoin`. The rows are the same.
+
 ---
 
 ## IQueryable Type Preservation 🟡

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

src/FSharp.Core/QueryExtensions.fs

@@ -108,14 +108,6 @@ module internal Adapters =
 
     let anonObjectTypes = tupleTypes |> Array.map snd
 
-    let tupleToAnonTypeMap =
-        let t = Dictionary<Type, Type>()
-
-        for k, v in tupleTypes do
-            t.[k] <- v
-
-        t
-
     let anonToTupleTypeMap =
         let t = Dictionary<Type, Type>()
 
@@ -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.
     [<NoComparison; NoEquality>]
     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<IGrouping<_, _>>)
@@ -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 ->

tests/FSharp.Core.UnitTests/FSharp.Core.UnitTests.fsproj

@@ -92,6 +92,15 @@
     <Compile Include="FSharp.Core\Microsoft.FSharp.Quotations\FSharpQuotations.fs" />
     <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\NullableOperators.fs" />
     <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\QueryTests.fs" />
+    <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\QueryProviderShape.fs" />
+    <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\QueryScenarioDomain.fs" />
+    <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\QueryScenario.fs" />
+    <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\QueryProjectionScenarios.fs" />
+    <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\QueryOperatorScenarios.fs" />
+    <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\QueryKeyScenarios.fs" />
+    <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\QueryStructureScenarios.fs" />
+    <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\QueryMethodSyntaxScenarios.fs" />
+    <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\QueryShapeTests.fs" />
     <Compile Include="FSharp.Core\XmlDocumentationValidation.fs" />
     <Compile Include="Interop\CSharpCollectionExpressions.fs" />
     <Compile Include="StructTuples.fs" />

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 ()