From c64c15d67c7a08c901157d3221da68b24ae44c74 Mon Sep 17 00:00:00 2001
From: Maurycy Markowski <maumar@microsoft.com>
Date: Fri, 4 May 2018 16:54:23 -0700
Subject: [PATCH] Fix to #11904 - 2.1 regression: "variable referenced from
 scope '', but it is not defined" exception in GroupBy query translation

Problem was that we were not correctly handling scenarios where multiple groupbys are present:

- if combined with Include, we would try to create orderings based on grouping keys for each group by. This is incorrect because all group keys apart from the innermost are out of scope of the inner query model (where we attempted to add them)
- if combined with aggregate we would try to translate into group-aggregate pattern and fail.

Fix is to only add ordering for the first grouping in case of include (includes are ignored for multi-groupby scenarios, so there is no risk of collection misalignment) and don't try to convert queries into group-aggregate pattern if multiple groupbys are present in the query.
---
 .../Query/RelationalQueryModelVisitor.cs      | 17 +++--
 .../Query/IncludeTestBase.cs                  | 67 +++++++++++++++++++
 ...equiresMaterializationExpressionVisitor.cs |  4 +-
 3 files changed, 78 insertions(+), 10 deletions(-)

diff --git a/src/EFCore.Relational/Query/RelationalQueryModelVisitor.cs b/src/EFCore.Relational/Query/RelationalQueryModelVisitor.cs
index 0340250fc77..37bb77a8177 100644
--- a/src/EFCore.Relational/Query/RelationalQueryModelVisitor.cs
+++ b/src/EFCore.Relational/Query/RelationalQueryModelVisitor.cs
@@ -1293,16 +1293,15 @@ protected override void VisitResultOperators(ObservableCollection<ResultOperator
                         queryModel.BodyClauses.Add(orderByClause);
                     }
 
-                    foreach (var groupResultOperator in groupResultOperators)
-                    {
-                        var groupKeys = groupResultOperator.KeySelector is NewExpression compositeGroupKey
-                            ? compositeGroupKey.Arguments.Reverse()
-                            : new[] { groupResultOperator.KeySelector };
+                    var firstGroupResultOperator = groupResultOperators.First();
 
-                        foreach (var groupKey in groupKeys)
-                        {
-                            orderByClause.Orderings.Insert(0, new Ordering(groupKey, OrderingDirection.Asc));
-                        }
+                    var groupKeys = firstGroupResultOperator.KeySelector is NewExpression compositeGroupKey
+                        ? compositeGroupKey.Arguments.Reverse()
+                        : new[] { firstGroupResultOperator.KeySelector };
+
+                    foreach (var groupKey in groupKeys)
+                    {
+                        orderByClause.Orderings.Insert(0, new Ordering(groupKey, OrderingDirection.Asc));
                     }
                 }
             }
diff --git a/src/EFCore.Specification.Tests/Query/IncludeTestBase.cs b/src/EFCore.Specification.Tests/Query/IncludeTestBase.cs
index 6811542d95c..e45ad75cd93 100644
--- a/src/EFCore.Specification.Tests/Query/IncludeTestBase.cs
+++ b/src/EFCore.Specification.Tests/Query/IncludeTestBase.cs
@@ -3996,6 +3996,73 @@ public virtual async Task Include_empty_reference_sets_IsLoaded(bool useString,
             }
         }
 
+        [Theory(Skip = "Issue #11916")]
+        [InlineData(false, false)]
+        [InlineData(true, false)]
+        // async blocked by issue #11917
+        //[InlineData(false, true)] 
+        //[InlineData(true, true)]
+        public virtual async Task Include_with_double_group_by(bool useString, bool async)
+        {
+            using (var context = CreateContext())
+            {
+                var groups = (useString
+                    ? context.Orders.Include(nameof(Order.OrderDetails))
+                    : context.Orders.Include(e => e.OrderDetails))
+                    .GroupBy(o => new { o.OrderID, o.OrderDate })
+                    .GroupBy(g => g.Key.OrderDate);
+
+                var employee = async
+                    ? await groups.ToListAsync()
+                    : groups.ToList();
+            }
+        }
+
+        [Theory]
+        [InlineData(false, false)]
+        [InlineData(true, false)]
+        // async blocked by issue #11917
+        //[InlineData(false, true)] 
+        //[InlineData(true, true)]
+        public virtual async Task Include_with_double_group_by_no_tracking(bool useString, bool async)
+        {
+            using (var context = CreateContext())
+            {
+                var groups = (useString
+                    ? context.Orders.Include(nameof(Order.OrderDetails)).AsNoTracking()
+                    : context.Orders.Include(e => e.OrderDetails)).AsNoTracking()
+                    .GroupBy(o => new { o.OrderID, o.OrderDate })
+                    .GroupBy(g => g.Key.OrderDate);
+
+                var employee = async
+                    ? await groups.ToListAsync()
+                    : groups.ToList();
+            }
+        }
+
+        [Theory]
+        [InlineData(false, false)]
+        [InlineData(true, false)]
+        // async blocked by issue #11917
+        //[InlineData(false, true)] 
+        //[InlineData(true, true)]
+        public virtual async Task Include_with_double_group_by_and_aggregate(bool useString, bool async)
+        {
+            using (var context = CreateContext())
+            {
+                var groups = (useString
+                    ? context.Orders.Include(nameof(Order.OrderDetails))
+                    : context.Orders.Include(e => e.OrderDetails))
+                    .GroupBy(o => new { o.OrderID, o.OrderDate })
+                    .GroupBy(g => g.Key.OrderDate)
+                    .Select(g => new { g.Key, Lastest = g.OrderBy(e => e.Key.OrderID).FirstOrDefault() });
+
+                var query = async
+                    ? await groups.ToListAsync()
+                    : groups.ToList();
+            }
+        }
+
         private static void CheckIsLoaded(
             NorthwindContext context,
             Customer customer,
diff --git a/src/EFCore/Query/ExpressionVisitors/Internal/RequiresMaterializationExpressionVisitor.cs b/src/EFCore/Query/ExpressionVisitors/Internal/RequiresMaterializationExpressionVisitor.cs
index 834a6f85192..34788db504e 100644
--- a/src/EFCore/Query/ExpressionVisitors/Internal/RequiresMaterializationExpressionVisitor.cs
+++ b/src/EFCore/Query/ExpressionVisitors/Internal/RequiresMaterializationExpressionVisitor.cs
@@ -428,7 +428,9 @@ var referencedQuerySource
             var isSubQuery = _queryModelStack.Count > 0;
             var finalResultOperator = queryModel.ResultOperators.LastOrDefault();
 
-            if (isSubQuery && finalResultOperator is GroupResultOperator groupResultOperator)
+            if (isSubQuery
+                && finalResultOperator is GroupResultOperator groupResultOperator
+                && queryModel.ResultOperators.OfType<GroupResultOperator>().Count() == 1)
             {
                 if (!(groupResultOperator.KeySelector is MemberInitExpression))
                 {