Query compilation error or data corruption for queries with GroupJoin wrapped in a subquery when the subset of columns is being projected

[shaulbehr]

This is a pretty specific corner case, but I've managed to repro it several times using different tables.

You need two tables, one with a non-mandatory FK to the other, e.g.

public class Salesperson
{
    public Guid SalespersonId {get; set;}
    public string Name {get; set;}
}
public class Customer
{
    public Guid CustomerId {get;set;}
    public Guid? SalespersonId {get;set;}
    public Salesperson Salesperson {get;set;}
}

Now you write a query to get the IDs of the top 100 Customers whose salesperson is named Fred:

var query = db.Customers
    .Where(q=>q.Salesperson.Name == "Fred")
    .Take(100)
    .Select(q=>q.SalespersonId)
    .ToList();

Result: a nasty error with sharp, pointy teeth:

System.ArgumentException : Expression of type 'Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.ValueBufferShaper' cannot be used for parameter of type 'Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.IShaper1[MyNamespace.Salesperson]' of method 'System.Collections.Generic.IEnumerable1[Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor+TransparentIdentifier2[Microsoft.EntityFrameworkCore.Storage.ValueBuffer,System.Collections.Generic.IEnumerable1[MyNamespace.Salesperson]]] _GroupJoin[ValueBuffer,BookTag,Nullable1,TransparentIdentifier2](Microsoft.EntityFrameworkCore.Query.RelationalQueryContext, System.Collections.Generic.IEnumerable1[Microsoft.EntityFrameworkCore.Storage.ValueBuffer], Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.IShaper1[Microsoft.EntityFrameworkCore.Storage.ValueBuffer], Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.IShaper1[MyNamespace.Salesperson], System.Func2[MyNamespace.Salesperson,System.Nullable1[System.Guid]], System.Func3[Microsoft.EntityFrameworkCore.Storage.ValueBuffer,System.Collections.Generic.IEnumerable1[MyNamespace.Salesperson],Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor+TransparentIdentifier2[Microsoft.EntityFrameworkCore.Storage.ValueBuffer,System.Collections.Generic.IEnumerable`1[MyNamespace.Salesperson]]], Microsoft.EntityFrameworkCore.Query.Internal.GroupJoinInclude, Microsoft.EntityFrameworkCore.Query.Internal.GroupJoinInclude)'

This error goes away if you change any of the following:

• Remove the "Where" clause
• Remove the "Take" clause
• Remove the final "Select" clause (i.e. you get back the whole original object)
• Move the "Select" to before the "Take"
• Make the SalespersonId mandatory

It's a perfect storm of a corner case, but it did, nonetheless, show up in my code, and it is a bug. I discovered it in EFCore 1.1.0, but even in the latest prerelease of 1.2.0 it is still a problem.

[divega]

@maumar this could be related to another 1.1.1 issue you are working on.

[maumar]

turns out to be unrelated to #7367
Problem is that we are not marking one of the groupjoin sources for materialization, because its wrapped in a subquery

[maumar]

Problem here is that our current mechanism to find the groupjoin sources that needs to be materialized only works on top level query. If the query has subqueries containing groupjoins, they will not be marked for materialization and cause the error.

The simple fix would be to introduce the visitor that walks the entire QueryModel and finds all the necessary query sources, however the problem with that approach is that we would regress perf in some scenarios - we would bring out even more data (which we currently don't do).

Better solution is to be smart about which sources we need - only mark for materialization sources that are part of the client side GroupJoin. If the GroupJoin is eliminated (flattened using SelectMany) - we don't need to materialize the main from clause. This effectively could address #6647. Problem is that there is significant risk associated with doing so, and it might not fit into 1.1.1

[maumar]

clearing milestone for re-triage

[maumar]

Moving to 1.1.1. again since this bug may cause data corruption in some cases

[rowanmiller]

We are OK with taking a minimal fix to this in 1.1.1. A more complete fix (better perf, cleaner SQL, etc.) will go into 2.0.

[maumar]

@divega
Justification to include this in 1.1.1 release:

Impact: High. Issue was reported by a customer and is a regression introduced in 1.1.0. It can also cause data corruption in specific cases.

Risk: Low/Medium. We already have the same logic that applied to GroupJoin clauses, but we were not doing it recursively. However, the fix incurs a performance hit - we will now yield more unnecessary data from database and materialize them as entities for the affected scenarios (groupjoins in a subquery). For simpler cases its just extra columns, more complicated ones can turn into N+1 queries.

[maumar]

Fixed in c99151d

[shaulbehr]

Great, thanks! Any idea when 1.1.1 will be released?

[maumar]

We don't have exact dates yet, but most likely sometime this month

[Eilon]

This patch bug is approved. Please use the normal code review process w/ a PR and make sure the fix is in the correct branch, then close the bug and mark it as done.