Fix to #9551 - Expression of type 'System.Object' cannot be used for …

…parameter of type 'Microsoft.EntityFrameworkCore.Metadata.IEntityType'

Problem was that we made some assumptions when compiling include, that do not hold true for complex queries involving group by and other operations that follow.

Once we build _Include(...) expression and try to substitute a qsre with it, we have compensation logic in case of groupby - trying to apply the substitution on the ElementSelector of the groupby. This logic is located in QueryModelExtensions.GetOutputExpression.
However, we assume that the outermost query model is the one that contains the groupby result operator. For the queries in the bug, this is not the case - GroupBy result operator is a subquery and therefore our old logic was not detecting it correctly, causing an error.

Fix is to find a correct query model (based on the qsre used in the Include result operator) and only after the correct QM (which can be in a subquery) is obtained, perform the compensation logic in QueryModelExtensions.GetOutputExpression.

Additional fix is needed when compiling collection navigation includes. We manually craft the QM that fetches child elements, and also assume that the outermost parent QM is the one that needs to be joined to. Similar approach is used here - find the "real" QM referenced in the include and build the collection-fetching QM based on that instead.

src/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs

@@ -3611,6 +3611,81 @@ public void Entries_for_detached_entities_are_removed()
             }
         }
 
+        [ConditionalFact]
+        public virtual void Include_reference_with_groupby_in_subquery()
+        {
+            AssertIncludeQuery<Level1>(
+                l1s => l1s
+                    .Include(l1 => l1.OneToOne_Optional_FK)
+                    .GroupBy(g => g.Name)
+                    .Select(g => g.OrderBy(e => e.Id).FirstOrDefault()),
+                expectedIncludes: new List<IExpectedInclude> { new ExpectedInclude<Level1>(e => e.OneToOne_Optional_FK, "OneToOne_Optional_FK") }); 
+        }
+
+        [ConditionalFact]
+        public virtual void Include_collection_with_groupby_in_subquery()
+        {
+            AssertIncludeQuery<Level1>(
+                l1s => l1s
+                    .Include(l1 => l1.OneToMany_Optional)
+                    .GroupBy(g => g.Name)
+                    .Select(g => g.OrderBy(e => e.Id).FirstOrDefault()),
+                expectedIncludes: new List<IExpectedInclude> { new ExpectedInclude<Level1>(e => e.OneToMany_Optional, "OneToMany_Optional") });
+        }
+
+        [ConditionalFact]
+        public virtual void Multi_include_with_groupby_in_subquery()
+        {
+            var expectedIncludes = new List<IExpectedInclude>
+            {
+                new ExpectedInclude<Level1>(e => e.OneToOne_Optional_FK, "OneToOne_Optional_FK"),
+                new ExpectedInclude<Level2>(e => e.OneToMany_Optional, "OneToMany_Optional", "OneToOne_Optional_FK"),
+            };
+
+            AssertIncludeQuery<Level1>(
+                l1s => l1s
+                    .Include(l1 => l1.OneToOne_Optional_FK.OneToMany_Optional)
+                    .GroupBy(g => g.Name)
+                    .Select(g => g.OrderBy(e => e.Id).FirstOrDefault()),
+                expectedIncludes);
+        }
+
+        [ConditionalFact]
+        public virtual void Include_collection_with_groupby_in_subquery_and_filter_before_groupby()
+        {
+            AssertIncludeQuery<Level1>(
+                l1s => l1s
+                    .Include(l1 => l1.OneToMany_Optional)
+                    .Where(l1 => l1.Id > 3)
+                    .GroupBy(g => g.Name)
+                    .Select(g => g.OrderBy(e => e.Id).FirstOrDefault()),
+                expectedIncludes: new List<IExpectedInclude> { new ExpectedInclude<Level1>(e => e.OneToMany_Optional, "OneToMany_Optional") });
+        }
+
+        [ConditionalFact]
+        public virtual void Include_collection_with_groupby_in_subquery_and_filter_after_groupby()
+        {
+            AssertIncludeQuery<Level1>(
+                l1s => l1s
+                    .Include(l1 => l1.OneToMany_Optional)
+                    .GroupBy(g => g.Name)
+                    .Where(g => g.Key != "Foo")
+                    .Select(g => g.OrderBy(e => e.Id).FirstOrDefault()),
+                expectedIncludes: new List<IExpectedInclude> { new ExpectedInclude<Level1>(e => e.OneToMany_Optional, "OneToMany_Optional") });
+
+
+            //using (var context = CreateContext())
+            //{
+            //    var query = context.LevelOne
+            //        .Include(l1 => l1.OneToMany_Optional)
+            //        .GroupBy(g => g.Name)
+            //        .Where(g => g.Key != "Foo")
+            //        .Select(g => g.OrderBy(e => e.Id).FirstOrDefault());
+
+            //    var result = query.ToList();
+            //}
+        }
+
         private static TResult Maybe<TResult>(object caller, Func<TResult> expression) where TResult : class
         {
             if (caller == null)

src/EFCore/Query/ExpressionVisitors/Internal/QueryModelFindingVisitor.cs

@@ -0,0 +1,95 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using JetBrains.Annotations;
+using Microsoft.EntityFrameworkCore.Utilities;
+using Remotion.Linq;
+using Remotion.Linq.Clauses;
+
+namespace Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal
+{
+    /// <summary>
+    ///     This API supports the Entity Framework Core infrastructure and is not intended to be used
+    ///     directly from your code. This API may change or be removed in future releases.
+    /// </summary>
+    public class QueryModelFindingVisitor : QueryModelVisitorBase
+    {
+        private readonly IQuerySource _querySource;
+
+        /// <summary>
+        ///     This API supports the Entity Framework Core infrastructure and is not intended to be used
+        ///     directly from your code. This API may change or be removed in future releases.
+        /// </summary>
+        public QueryModelFindingVisitor([NotNull] IQuerySource querySource)
+        {
+            Check.NotNull(querySource, nameof(querySource));
+
+            _querySource = querySource;
+        }
+
+        /// <summary>
+        ///     This API supports the Entity Framework Core infrastructure and is not intended to be used
+        ///     directly from your code. This API may change or be removed in future releases.
+        /// </summary>
+        public virtual QueryModel QueryModel { get; private set; }
+
+        /// <summary>
+        ///     This API supports the Entity Framework Core infrastructure and is not intended to be used
+        ///     directly from your code. This API may change or be removed in future releases.
+        /// </summary>
+        public override void VisitQueryModel(QueryModel queryModel)
+        {
+            queryModel.TransformExpressions(new TransformingQueryModelExpressionVisitor<QueryModelFindingVisitor>(this).Visit);
+
+            base.VisitQueryModel(queryModel);
+        }
+
+        /// <summary>
+        ///     This API supports the Entity Framework Core infrastructure and is not intended to be used
+        ///     directly from your code. This API may change or be removed in future releases.
+        /// </summary>
+        public override void VisitMainFromClause(MainFromClause fromClause, QueryModel queryModel)
+        {
+            if (fromClause == _querySource)
+            {
+                QueryModel = queryModel;
+            }
+        }
+
+        /// <summary>
+        ///     This API supports the Entity Framework Core infrastructure and is not intended to be used
+        ///     directly from your code. This API may change or be removed in future releases.
+        /// </summary>
+        public override void VisitAdditionalFromClause(AdditionalFromClause fromClause, QueryModel queryModel, int index)
+        {
+            if (fromClause == _querySource)
+            {
+                QueryModel = queryModel;
+            }
+        }
+
+        /// <summary>
+        ///     This API supports the Entity Framework Core infrastructure and is not intended to be used
+        ///     directly from your code. This API may change or be removed in future releases.
+        /// </summary>
+        public override void VisitJoinClause(JoinClause joinClause, QueryModel queryModel, int index)
+        {
+            if (joinClause == _querySource)
+            {
+                QueryModel = queryModel;
+            }
+        }
+
+        /// <summary>
+        ///     This API supports the Entity Framework Core infrastructure and is not intended to be used
+        ///     directly from your code. This API may change or be removed in future releases.
+        /// </summary>
+        public override void VisitGroupJoinClause(GroupJoinClause groupJoinClause, QueryModel queryModel, int index)
+        {
+            if (groupJoinClause == _querySource)
+            {
+                QueryModel = queryModel;
+            }
+        }
+    }
+}

src/EFCore/Query/Internal/IncludeCompiler.CollectionQueryModelRewritingExpressionVisitor.cs

@@ -11,6 +11,7 @@
 using Microsoft.EntityFrameworkCore.Metadata;
 using Microsoft.EntityFrameworkCore.Metadata.Internal;
 using Microsoft.EntityFrameworkCore.Query.Expressions.Internal;
+using Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal;
 using Remotion.Linq;
 using Remotion.Linq.Clauses;
 using Remotion.Linq.Clauses.Expressions;
@@ -43,6 +44,8 @@ public CollectionQueryModelRewritingExpressionVisitor(
 
             public List<Ordering> ParentOrderings { get; } = new List<Ordering>();
 
+            public QueryModel ParentQueryModel { get; set; }
+
             protected override Expression VisitMethodCall(MethodCallExpression methodCallExpression)
             {
                 if (typeof(IQueryBuffer).GetTypeInfo()
@@ -104,14 +107,21 @@ var parentQuerySourceReferenceExpression
 
                 var parentQuerySource = parentQuerySourceReferenceExpression.ReferencedQuerySource;
 
+                var qmFinder = new QueryModelFindingVisitor(parentQuerySourceReferenceExpression.ReferencedQuerySource);
+                qmFinder.VisitQueryModel(_parentQueryModel);
+
+                var realParentQueryModel = qmFinder.QueryModel ?? _parentQueryModel;
+                ParentQueryModel = realParentQueryModel;
+
                 BuildParentOrderings(
-                    _parentQueryModel,
+                    realParentQueryModel,
                     navigation,
                     parentQuerySourceReferenceExpression,
                     ParentOrderings);
 
                 var querySourceMapping = new QuerySourceMapping();
-                var clonedParentQueryModel = _parentQueryModel.Clone(querySourceMapping);
+
+                var clonedParentQueryModel = realParentQueryModel.Clone(querySourceMapping);
                 _queryCompilationContext.UpdateMapping(querySourceMapping);
 
                 _queryCompilationContext.CloneAnnotations(querySourceMapping, clonedParentQueryModel);

src/EFCore/Query/Internal/IncludeCompiler.IncludeLoadTreeNodeBase.cs

@@ -12,6 +12,7 @@
 using Remotion.Linq.Clauses.Expressions;
 using Remotion.Linq.Clauses.ResultOperators;
 using Remotion.Linq.Parsing;
+using Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal;
 
 namespace Microsoft.EntityFrameworkCore.Query.Internal
 {
@@ -120,8 +121,34 @@ var includeExpression
                                     entityParameter,
                                     _includedParameter));
 
-                    ApplyIncludeExpressionsToQueryModel(
-                        queryModel, targetQuerySourceReferenceExpression, includeExpression);
+                    var includeApplyingVisitor = new IncludeApplyingQueryModelVisitor(
+                        targetQuerySourceReferenceExpression,
+                        includeExpression);
+
+                    includeApplyingVisitor.VisitQueryModel(queryModel);
+                }
+            }
+
+            private class IncludeApplyingQueryModelVisitor : QueryModelVisitorBase
+            {
+                public IncludeApplyingQueryModelVisitor(
+                    QuerySourceReferenceExpression querySourceReferenceExpression,
+                    Expression expression)
+                {
+                    _querySourceReferenceExpression = querySourceReferenceExpression;
+                    _expression = expression;
+                }
+
+                private readonly QuerySourceReferenceExpression _querySourceReferenceExpression;
+                private readonly Expression _expression;
+
+                public override void VisitQueryModel(QueryModel queryModel)
+                {
+                    queryModel.TransformExpressions(new TransformingQueryModelExpressionVisitor<IncludeApplyingQueryModelVisitor>(this).Visit);
+
+                    ApplyIncludeExpressionsToQueryModel(queryModel, _querySourceReferenceExpression, _expression);
+
+                    base.VisitQueryModel(queryModel);
                 }
             }
 

src/EFCore/Query/Internal/IncludeCompiler.cs

@@ -96,7 +96,12 @@ var collectionQueryModelRewritingExpressionVisitor
 
             queryModel.TransformExpressions(collectionQueryModelRewritingExpressionVisitor.Visit);
 
-            ApplyParentOrderings(queryModel, collectionQueryModelRewritingExpressionVisitor.ParentOrderings);
+            if (collectionQueryModelRewritingExpressionVisitor.ParentQueryModel != null)
+            {
+                ApplyParentOrderings(
+                    collectionQueryModelRewritingExpressionVisitor.ParentQueryModel,
+                    collectionQueryModelRewritingExpressionVisitor.ParentOrderings);
+            }
         }
 
         /// <summary>
@@ -134,6 +139,22 @@ var querySourceReferenceExpression
                     continue;
                 }
 
+                if (querySourceReferenceExpression.Type.IsGrouping())
+                {
+                    var qmFinder = new QueryModelFindingVisitor(includeResultOperator.QuerySource);
+                    qmFinder.VisitQueryModel(queryModel);
+
+                    if (qmFinder.QueryModel != null
+                        && qmFinder.QueryModel != queryModel)
+                    {
+                        querySourceReferenceExpression
+                            = querySourceTracingExpressionVisitor
+                                .FindResultQuerySourceReferenceExpression(
+                                    qmFinder.QueryModel.GetOutputExpression(),
+                                    includeResultOperator.QuerySource);
+                    }
+                }
+
                 var includeLoadTree
                     = includeLoadTrees
                         .SingleOrDefault(

test/EFCore.SqlServer.FunctionalTests/GraphUpdatesWithSequenceSqlServerTest.cs

@@ -24,4 +24,4 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
             }
         }
     }
-}
+}