Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query: compilation error in shaper for complex query with nested collection with ToList for inner collection and later on top level projection #23303

Closed
maumar opened this issue Nov 13, 2020 · 7 comments
Assignees
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported punted-for-6.0 type-bug
Milestone

Comments

@maumar
Copy link
Contributor

maumar commented Nov 13, 2020

query:

from l1 in ss.Set<Level1>()
                      orderby l1.Id
                      let inner = (from l2 in l1.OneToMany_Optional1
                                   where l2.Name != "Foo"
                                   let innerL1s = from innerL1 in ss.Set<Level1>()
                                                  where innerL1.OneToMany_Optional1.Any(innerL2 => innerL2.Id == l2.Id)
                                                  select innerL1.Name
                                   select innerL1s.ToList()).FirstOrDefault()
                      select inner.ToList()

exception:

System.ArgumentException : GenericArguments[2], 'System.Collections.Generic.List`1[System.String]', on 'Void PopulateCollection[TCollection,TElement,TRelatedEntity](Int32, Microsoft.EntityFrameworkCore.Query.QueryContext, System.Data.Common.DbDataReader, Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryResultCoordinator, System.Func`3[Microsoft.EntityFrameworkCore.Query.QueryContext,System.Data.Common.DbDataReader,System.Object[]], System.Func`3[Microsoft.EntityFrameworkCore.Query.QueryContext,System.Data.Common.DbDataReader,System.Object[]], System.Func`3[Microsoft.EntityFrameworkCore.Query.QueryContext,System.Data.Common.DbDataReader,System.Object[]], System.Collections.Generic.IReadOnlyList`1[Microsoft.EntityFrameworkCore.ChangeTracking.ValueComparer], System.Collections.Generic.IReadOnlyList`1[Microsoft.EntityFrameworkCore.ChangeTracking.ValueComparer], System.Collections.Generic.IReadOnlyList`1[Microsoft.EntityFrameworkCore.ChangeTracking.ValueComparer], System.Func`5[Microsoft.EntityFrameworkCore.Query.QueryContext,System.Data.Common.DbDataReader,Microsoft.EntityFrameworkCore.Query.Internal.ResultContext,Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryResultCoordinator,TRelatedEntity])' violates the constraint of type 'TRelatedEntity'.
    ---- System.Security.VerificationException : Method Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor+ShaperProcessingExpressionVisitor.PopulateCollection: type argument 'System.Collections.Generic.List`1[System.String]' violates the constraint of type parameter 'TRelatedEntity'.
  Stack Trace: 
    RuntimeType.ValidateGenericArguments(MemberInfo definition, RuntimeType[] genericArguments, Exception e)
    RuntimeMethodInfo.MakeGenericMethod(Type[] methodInstantiation)
    ShaperProcessingExpressionVisitor.VisitExtension(Expression extensionExpression) line 757
    Expression.Accept(ExpressionVisitor visitor)
    ExpressionVisitor.Visit(Expression node)
    ShaperProcessingExpressionVisitor.ProcessShaper(Expression shaperExpression, RelationalCommandCache& relationalCommandCache, LambdaExpression& relatedDataLoaders) line 279
    RelationalShapedQueryCompilingExpressionVisitor.VisitShapedQuery(ShapedQueryExpression shapedQueryExpression) line 66
    ShapedQueryCompilingExpressionVisitor.VisitExtension(Expression extensionExpression) line 97
    Expression.Accept(ExpressionVisitor visitor)
    ExpressionVisitor.Visit(Expression node)
    QueryCompilationContext.CreateQueryExecutor[TResult](Expression query) line 181
    Database.CompileQuery[TResult](Expression query, Boolean async) line 72
    QueryCompiler.CompileQueryCore[TResult](IDatabase database, Expression query, IModel model, Boolean async) line 114
    <>c__DisplayClass9_0`1.<Execute>b__0() line 98
    CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func`1 compiler) line 78
    QueryCompiler.Execute[TResult](Expression query) line 94
    EntityQueryProvider.Execute[TResult](Expression expression) line 81
    EntityQueryable`1.GetEnumerator() line 93
@maumar
Copy link
Contributor Author

maumar commented Nov 13, 2020

related to #23205

@maumar
Copy link
Contributor Author

maumar commented Nov 13, 2020

Note: for weak entities model, this query throws different exception:

System.Collections.Generic.KeyNotFoundException : The given key 'EmptyProjectionMember' was not present in the dictionary.
  Stack Trace: 
    Dictionary`2.get_Item(TKey key)
    SelectExpression.GetMappedProjection(ProjectionMember projectionMember) line 417
    RelationalSqlTranslatingExpressionVisitor.VisitExtension(Expression extensionExpression) line 413
    Expression.Accept(ExpressionVisitor visitor)
    ExpressionVisitor.Visit(Expression node)
    RelationalSqlTranslatingExpressionVisitor.BindProperty(EntityReferenceExpression entityReferenceExpression, IProperty property) line 1092
    RelationalSqlTranslatingExpressionVisitor.TryBindMember(Expression source, MemberIdentity member) line 1077
    RelationalSqlTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression) line 464
    MethodCallExpression.Accept(ExpressionVisitor visitor)
    ExpressionVisitor.Visit(Expression node)
    RelationalSqlTranslatingExpressionVisitor.VisitBinary(BinaryExpression binaryExpression) line 331
    SqlServerSqlTranslatingExpressionVisitor.VisitBinary(BinaryExpression binaryExpression) line 67
    BinaryExpression.Accept(ExpressionVisitor visitor)
    ExpressionVisitor.Visit(Expression node)
    RelationalSqlTranslatingExpressionVisitor.VisitBinary(BinaryExpression binaryExpression) line 331
    SqlServerSqlTranslatingExpressionVisitor.VisitBinary(BinaryExpression binaryExpression) line 67
    BinaryExpression.Accept(ExpressionVisitor visitor)
    ExpressionVisitor.Visit(Expression node)
    RelationalSqlTranslatingExpressionVisitor.TranslateInternal(Expression expression) line 125
    RelationalSqlTranslatingExpressionVisitor.Translate(Expression expression) line 120
    RelationalQueryableMethodTranslatingExpressionVisitor.TranslateExpression(Expression expression) line 1208
    RelationalQueryableMethodTranslatingExpressionVisitor.TranslateLambdaExpression(ShapedQueryExpression shapedQueryExpression, LambdaExpression lambdaExpression) line 1220
    RelationalQueryableMethodTranslatingExpressionVisitor.TranslateWhere(ShapedQueryExpression source, LambdaExpression predicate) line 1195
    QueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression) line 489
    MethodCallExpression.Accept(ExpressionVisitor visitor)
    ExpressionVisitor.Visit(Expression node)
    QueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression) line 120
    MethodCallExpression.Accept(ExpressionVisitor visitor)
    ExpressionVisitor.Visit(Expression node)
    QueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression) line 120
    MethodCallExpression.Accept(ExpressionVisitor visitor)
    ExpressionVisitor.Visit(Expression node)
    QueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression) line 120
    MethodCallExpression.Accept(ExpressionVisitor visitor)
    ExpressionVisitor.Visit(Expression node)
    QueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression) line 120
    MethodCallExpression.Accept(ExpressionVisitor visitor)
    ExpressionVisitor.Visit(Expression node)
    QueryableMethodTranslatingExpressionVisitor.TranslateSubquery(Expression expression) line 640
    RelationalSqlTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression) line 649
    MethodCallExpression.Accept(ExpressionVisitor visitor)
    ExpressionVisitor.Visit(Expression node)
    RelationalSqlTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression) line 849
    MethodCallExpression.Accept(ExpressionVisitor visitor)
    ExpressionVisitor.Visit(Expression node)
    RelationalSqlTranslatingExpressionVisitor.TranslateInternal(Expression expression) line 125
    RelationalSqlTranslatingExpressionVisitor.Translate(Expression expression) line 120
    RelationalProjectionBindingExpressionVisitor.Visit(Expression expression) line 240
    RelationalProjectionBindingExpressionVisitor.Translate(SelectExpression selectExpression, Expression expression) line 77
    RelationalQueryableMethodTranslatingExpressionVisitor.TranslateSelect(ShapedQueryExpression source, LambdaExpression selector) line 924
    QueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression) line 405
    MethodCallExpression.Accept(ExpressionVisitor visitor)
    ExpressionVisitor.Visit(Expression node)
    QueryCompilationContext.CreateQueryExecutor[TResult](Expression query) line 176
    Database.CompileQuery[TResult](Expression query, Boolean async) line 72
    QueryCompiler.CompileQueryCore[TResult](IDatabase database, Expression query, IModel model, Boolean async) line 114
    <>c__DisplayClass9_0`1.<Execute>b__0() line 98
    CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func`1 compiler) line 78
    QueryCompiler.Execute[TResult](Expression query) line 94
    EntityQueryProvider.Execute[TResult](Expression expression) line 81
    EntityQueryable`1.GetEnumerator() line 93
    List`1.ctor(IEnumerable`1 collection)

@smitpatel
Copy link
Contributor

@maumar

        [ConditionalTheory]
        [MemberData(nameof(IsAsyncData))]
        public virtual Task Complex_query_with_let_collection_projection_FirstOrDefault(bool async)
        {
            return AssertQuery(
                async,
                ss => from l1 in ss.Set<Level1>()
                      orderby l1.Id
                      let inner = (from l2 in l1.OneToMany_Optional1
                                   where l2.Name != "Foo"
                                   let innerL1s = from innerL1 in ss.Set<Level1>()
                                                  where innerL1.OneToMany_Optional1.Any(innerL2 => innerL2.Id == l2.Id)
                                                  select innerL1.Name
                                   select innerL1s.ToList()).FirstOrDefault()
                      select inner,
                assertOrder: true,
                elementAsserter: (e, a) => AssertCollection(e, a));
        }

Is in our current tests and works for all providers (Sqlite throws Apply not supported exception). The test looks awfully similar

@maumar
Copy link
Contributor Author

maumar commented Mar 23, 2021

@smitpatel the only difference is top level ToList (on inner which is already converted to ToList in the let). Issue happens when processing RelationalCollectionShaper that already has a nested RelationalCollectionShaper. The query that works only has one of these so we don't hit the issue.

Shaper for working query:

ProjectionBindingExpression: 0 == default(Nullable<int>) ? default(List<string>) : RelationalCollectionShaper:
    CollectionId: 0
    ParentIdentifier:new object[]
    { 
        (object)ProjectionBindingExpression: 1, 
        (object)ProjectionBindingExpression: 2 
    }
    OuterIdentifier:new object[]
    { 
        (object)ProjectionBindingExpression: 1, 
        (object)ProjectionBindingExpression: 2 
    }
    SelfIdentifier:new object[]{ (object)ProjectionBindingExpression: 4 }
    InnerShaper:ProjectionBindingExpression: 3
    Navigation: 

shaper for failing query:

RelationalCollectionShaper:
    CollectionId: 0
    ParentIdentifier:new object[]{ (object)ProjectionBindingExpression: 0 }
    OuterIdentifier:new object[]{ (object)ProjectionBindingExpression: 0 }
    SelfIdentifier:new object[]{ (object)ProjectionBindingExpression: 1 }
    InnerShaper:(IQueryable<string>)RelationalCollectionShaper:
        CollectionId: 1
        ParentIdentifier:new object[]{ (object)ProjectionBindingExpression: 1 }
        OuterIdentifier:new object[]{ (object)ProjectionBindingExpression: 1 }
        SelfIdentifier:new object[]{ (object)ProjectionBindingExpression: 3 }
        InnerShaper:ProjectionBindingExpression: 2
        Navigation: 
    Navigation: 

@smitpatel
Copy link
Contributor

Removing from triage to decide what do we want to do for 6.0 here
Notes from investigation

  • We have a code path which removes ToList calls on Queryable methods (essentially a subquery producing collection) since without a navigation to materializing into, we generate list objects anyway so ToList is redundant.
  • What above causes is, we never call ToList on something which is already a list in shaper.
  • In above query ToList is being applied on result of FirstOrDefault which is already of type ToList (intentional user specified double ToList), which fails our condition in point 1 so we get a ToList in shaper being called upon a list object.

In current latest code, we throw exception that parameter source is null. The root cause is the ToList call on the list object. While debugging I verified that we do have empty list object, I am not able to figure out why calling ToList over that is throwing parameter null exception.
Shaper

(queryContext, dataReader, resultContext, resultCoordinator) => 
{
    resultContext.Values == null ? 
    {
        int? namelessParameter{0};
        List<string> namelessParameter{1};
        namelessParameter{0} = dataReader.IsDBNull(4) ? default(int?) : (int?)dataReader.GetInt32(4);
        namelessParameter{1} = ShaperProcessingExpressionVisitor.InitializeCollection<string, List<string>>(
            collectionId: 0, 
            queryContext: queryContext, 
            dbDataReader: dataReader, 
            resultCoordinator: resultCoordinator, 
            parentIdentifier: Func<QueryContext, DbDataReader, object[]>, 
            outerIdentifier: Func<QueryContext, DbDataReader, object[]>, 
            clrCollectionAccessor: null);
        return resultContext.Values = new object[]
        { 
            (object)namelessParameter{0}, 
            namelessParameter{1} 
        };
    } : default(void);
    ShaperProcessingExpressionVisitor.PopulateCollection<List<string>, string, string>(
        collectionId: 0, 
        queryContext: queryContext, 
        dbDataReader: dataReader, 
        resultCoordinator: resultCoordinator, 
        parentIdentifier: Func<QueryContext, DbDataReader, object[]>, 
        outerIdentifier: Func<QueryContext, DbDataReader, object[]>, 
        selfIdentifier: Func<QueryContext, DbDataReader, object[]>, 
        parentIdentifierValueComparers: List<ValueComparer> { DefaultValueComparer<int>, DefaultValueComparer<int> }, 
        outerIdentifierValueComparers: List<ValueComparer> { DefaultValueComparer<int>, DefaultValueComparer<int> }, 
        selfIdentifierValueComparers: List<ValueComparer> { DefaultValueComparer<int> }, 
        innerShaper: Func<QueryContext, DbDataReader, ResultContext, SingleQueryResultCoordinator, string>);
    return IsTrue(resultCoordinator.ResultReady)
     ? (int?)(resultContext.Values[0]) == default(int?) ? default(List<string>) : (List<string>)(resultContext.Values[1])
        .ToList() : default(List<string>);
}

It is the ToList call in very last line. resultContext.Values[1] seems to have empty list while evaluating PopulateCollection function, not sure what goes wrong afterwards. It shouldn't get contaminated else we would see a lot more errors in the query.

Cost of figuring out what is wrong may not be worth iff we remove the redundant ToList. We already have test which works when user specify only 1 ToList (either inner or outer doesn't matter).

So decision to make

  • Do we want to investigate why shaper is failing?
  • Do we want to add code for removing redundant ToList?
  • Do we want to do anything for 6.0 since issue happens when user writes unnecessary ToList call.

@smitpatel smitpatel removed this from the 6.0.0 milestone Aug 30, 2021
@ajcvickers ajcvickers added this to the Backlog milestone Aug 31, 2021
@ajcvickers
Copy link
Member

Note from triage: backlogging to remove the redundant ToList, but as a low-priority fix.

@smitpatel smitpatel added the priority-bug Issues which requires API breaks and have bigger impact hence should be fixed earlier in the release label Nov 3, 2021
@ajcvickers ajcvickers modified the milestones: Backlog, 7.0.0 Nov 10, 2021
@smitpatel
Copy link
Contributor

Revisiting this -> Actually above output was bug in expression printer (already notified authorities).

The result of FirstOrDefault in the query is null since the enumerable is empty. And we call ToList on this result so ArgumentNullException is by design here.

@smitpatel smitpatel removed the priority-bug Issues which requires API breaks and have bigger impact hence should be fixed earlier in the release label May 19, 2022
@smitpatel smitpatel modified the milestones: 7.0.0, 6.0.0 May 19, 2022
@smitpatel smitpatel added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label May 19, 2022
smitpatel added a commit that referenced this issue May 19, 2022
smitpatel added a commit that referenced this issue May 19, 2022
ghost pushed a commit that referenced this issue May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported punted-for-6.0 type-bug
Projects
None yet
Development

No branches or pull requests

3 participants