Efcore: Remove/Change ICollection constraint on QueryBuffer.CorrelateSubqueryAsync which breaks support for ReadOnly navigation properties

[SaebAmini]

This constraint seems to break the pattern where you expose entity collections as readonly.

Exception message:
GenericArguments[2], 'System.Collections.Generic.IReadOnlyCollection`1[ChildEntity]', on
 'System.Threading.Tasks.Task`1[TCollection] CorrelateSubqueryAsync[TInner,TOut,TCollection](Int32,
 Microsoft.EntityFrameworkCore.Metadata.INavigation, 
System.Func`2[Microsoft.EntityFrameworkCore.Metadata.INavigation,TCollection], 
Microsoft.EntityFrameworkCore.Query.Internal.MaterializedAnonymousObject, Boolean, 
System.Func`1[System.Collections.Generic.IAsyncEnumerable`1[System.Tuple`3[TInner,Microsoft.EntityFra
meworkCore.Query.Internal.MaterializedAnonymousObject,Microsoft.EntityFrameworkCore.Query.Interna
l.MaterializedAnonymousObject]]], 
System.Func`3[Microsoft.EntityFrameworkCore.Query.Internal.MaterializedAnonymousObject,Microsoft.E
ntityFrameworkCore.Query.Internal.MaterializedAnonymousObject,System.Boolean], 
System.Threading.CancellationToken)' violates the constraint of type 'TCollection'.



Stack trace:
   at System.RuntimeType.ValidateGenericArguments(MemberInfo definition, RuntimeType[] genericArguments, Exception e)
   at System.Reflection.RuntimeMethodInfo.MakeGenericMethod(Type[] methodInstantiation)
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.CorrelatedCollectionOptimizingVisitor.Rewrite(Int32 correlatedCollectionIndex, QueryModel collectionQueryModel, INavigation navigation, Boolean trackingQuery, QuerySourceReferenceExpression originQuerySource, Boolean forceListResult, Type listResultElementType)
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.CorrelatedCollectionOptimizingVisitor.TryRewrite(SubQueryExpression subQueryExpression, Boolean forceToListResult, Type listResultElementType, Expression& result)
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.CorrelatedCollectionOptimizingVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at System.Linq.Expressions.MethodCallExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitUnary(UnaryExpression node)
   at System.Linq.Expressions.UnaryExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitAndConvert[T](ReadOnlyCollection`1 nodes, String callerName)
   at Remotion.Linq.Parsing.RelinqExpressionVisitor.VisitNew(NewExpression expression)
   at System.Linq.Expressions.NewExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.TryOptimizeCorrelatedCollections(QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.VisitSelectClause(SelectClause selectClause, QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryModelVisitor.VisitSelectClause(SelectClause selectClause, QueryModel queryModel)
   at Remotion.Linq.Clauses.SelectClause.Accept(IQueryModelVisitor visitor, QueryModel queryModel)
   at Remotion.Linq.QueryModelVisitorBase.VisitQueryModel(QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.VisitQueryModel(QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryModelVisitor.VisitQueryModel(QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.CreateAsyncQueryExecutor[TResult](QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Storage.Database.CompileAsyncQuery[TResult](QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileAsyncQueryCore[TResult](Expression query, IQueryModelGenerator queryModelGenerator, IDatabase database)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass22_0`1.<CompileAsyncQuery>b__0()
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQueryCore[TFunc](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddAsyncQuery[TResult](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileAsyncQuery[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.ExecuteAsync[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.ExecuteAsync[TResult](Expression expression)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1.System.Collections.Generic.IAsyncEnumerable<TResult>.GetEnumerator()
   at System.Linq.AsyncEnumerable.Aggregate_[TSource,TAccumulate,TResult](IAsyncEnumerable`1 source, TAccumulate seed, Func`3 accumulator, Func`2 resultSelector, CancellationToken cancellationToken)
   at UserQuery.Query(GetParentEntityQuery query, CancellationToken cancellationToken) in xxxx:line xx

Steps to reproduce

We have many Entities with child Entities using the following pattern for encapsulation and to protect the child collections being added to from outside the parent entity:

public class ParentEntity
{
    private List<ChildEntity> _children = new List<ChildEntity>();
    public IReadOnlyCollection<ChildEntity> Children => _children .AsReadOnly();
}

And that works fine most of the time. However, under some circumstances and with some of the more complex queries, the CorrelateSubqueryAsync from above kicks in and apparently because it's passed the IReadOnlyCollection while it has a constraint of ICollection, it throws.

Unfortunately, I don't have reproduction steps, because this is happening in an enterprise custom solution in a fairly complicated query and I'm not sure what's causing CorrelateSubqueryAsync to kick in, but hopefully the above exception details and stack trace can shed some light.

Further technical details

EF Core version: 2.1.1
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 10 Enterprise latest update
IDE: Visual Studio Professional 2017 15.7.4

[ajcvickers]

See also test case in #12558 when fixing this.

[ajcvickers]

Another repro in #12925

[maumar]

need to discuss this with @divega

[ajcvickers]

Outcomes from the discussion in triage:

• For now @maumar to test if casting in the projection can be used as a workaround
• For 3.0, make sure that if the projected type is a mapped type, then we use the appropriate property-access mode to add items to the backing field.
• For 3.0/backlog, depending on workarounds:
  
  
  
  • Special-case well-known read-only types to build the collection before assigning
  
  
  • Don't constrain to ICollection and still attempt to call Add
  
  

[ajcvickers]

As a workaround I was able to use an explicit cast with a ToList, at least when projecting to a type that doesn't need to be IReadOnlyCollection. For example:
C# using (var context = new BloggingContext()) { var blogs = context.Set<Blog>().Select(b => new { b.Id, Posts = ((ICollection<Post>)b.Posts).ToList() }).ToList(); }

[SaebAmini]

@smitpatel care to explain why you've closed this issue? why is it "not needed" assuming from the label?

[SaebAmini]

@ajcvickers it'd be great if some explanation is given when issues are closed out of nowhere.

[smitpatel]

QueryBuffer has been removed from query pipeline in 3.0 release so the root cause of this issue is gone. Hence not-needed.