v1.1 - The multi-part identifier "x.Id" could not be bound

[ctolkien]

The following linq:

var foo = await _context.Things
                .Select(x => new ThingViewModel
                {
                    Name = x.Name,
                    Bars = x.Bars
                        .Take(5)
                        .Select(q => new ThingViewModel.BarViewModel {
                            OtherName = q.Name,
                    })
                }).ToListAsync();

Generates a Select N+1 of the following:

SELECT [t].[Name]
FROM (
    SELECT TOP(5) [q0].*
    FROM [Bars] AS [q0]
    WHERE [x].[Id] = [q0].[ThingId]
) AS [t]

This is invalid SQL, as [x] doesn't exist here. Removing the .Take(5) allows the query to proceed without issue (it is a select n+1, but I presume this is by design for now).

[ctolkien]

I should also flag, this is a regression from v1.0.

[smitpatel]

We do client eval in this case producing N+1 queries.
In the second query, ParameterInjection is faulty hence we generated incorrect SQL instead of having injected parameter.

[smitpatel]

Possible workarounds

1. Makes 2 queries but fetches all records from second table though only relevant columns. So if there are many Bars for each Thing it may perform poorly.

var foo1 = db.Things.Select(t => new {Id = t.Id, Name = t.Name}).ToList();
var bars =
    db.Set<Bar>()
        .Select(b => new {Id = b.ThingId, OtherName = b.Name})
        .GroupBy(t => t.Id)
        .Select(g => new {Id = g.Key, Bars = g.Take(5)})
        .ToList();

var foo = foo1.Join(bars, f1 => f1.Id, f2 => f2.Id,
    (f1, f2) => new { Name = f1.Name, Bars = f2.Bars }).ToList();

1. N+1 queries but only fetches required rows from Bars table. (Also fetches all columns)

var temp = db.Things.Select(t => new { t.Name, Bars = t.Bars.Take(5) }).ToList();
var foo = temp.Select(t => new {Name = t.Name, Bars = t.Bars.Select(b => new {OtherName = b.Name})}).ToList();

[divega]

@smitpatel could you please take a stab at writing a justification/risk assessment for taking the fix in 1.1.1? A comment here in the bug works for me. Thanks.

[smitpatel]

Justification: This is regression from 1.0. Due to changes in query pipeline we started binding with outer query even though it was a separate SelectExpression overzealously without verifying if it would generate valid scenario. The fix blocks additional binding which could generate invalid queries.

Risk: This is low risk. It makes queries to evaluate certain parts on client side instead of translating to server. And in most of those cases, the server side translation was producing invalid queries.

[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.

[smitpatel]

merged to 1.1.1 in b8fb258
in dev a457f2e

[ctolkien]

Thanks @smitpatel 👍