Skip to content

Commit

Permalink
Fix to #6366 - 2 level expand not working correctly
Browse files Browse the repository at this point in the history
Problem happens for complex include scenarios like so:

ctx
    .Posts.Include(x => x.User).ThenInclude(x => x.Roles)
    .Include(x => x.VoteDefinition).ThenInclude(x => x.PossibleAnswers)
    .Skip(1)

We generate 3 queries for this case:

first one gets Posts and 1:1 navigations (User and Vote Definition)
SELECT [x].[Id], [x].[Name], [x].[UserId], [x].[VoteDefinitionId], [v].[Id], [v].[PostId], [u].[Id], [u].[Name]
FROM [Posts] AS [x]
LEFT JOIN [VoteDefinitions] AS [v] ON [v].[PostId] = [x].[Id]
INNER JOIN [Users] AS [u] ON [x].[UserId] = [u].[Id]
ORDER BY [v].[Id], [u].[Id]
OFFSET @__p_0 ROWS',N'@__p_0 int
second one copies the first one and joins it with Roles table to return associated Roles
SELECT [r].[Id], [r].[Name], [r].[UserId]
FROM [Roles] AS [r]
INNER JOIN (
    SELECT DISTINCT [t0].*
    FROM (
        SELECT [v].[Id], [u].[Id] AS [Id0]
        FROM [Posts] AS [x]
        LEFT JOIN [VoteDefinitions] AS [v] ON [v].[PostId] = [x].[Id]
        INNER JOIN [Users] AS [u] ON [x].[UserId] = [u].[Id]
        ORDER BY [v].[Id], [u].[Id]
        OFFSET @__p_0 ROWS
    ) AS [t0]
) AS [u0] ON [r].[UserId] = [u0].[Id]
ORDER BY [u0].[Id], [u0].[Id0]',N'@__p_0 int

third one is similar to the second one.

Problem is that in second query we are joining based on a wrong column (VoteDefinition's Id rather than User's Id). This is because we lose track of the information about which column is associated with which property.
Due to Skip() call in the original LINQ query, we generate SELECT * around the relevant column information. Later, when looking for which column to join, we peek into the SELECT expression and don't see any information there so we assume that we should join based on a property with the name we need. This works fine if there are no duplicate columns with the same name projected out (and uniquefied) from other joined tables.

Fix is, in case of a SELECT * query, to look for the relevant column information in the TablesExpressions/SelectExpressions that the SELECT * is projecting from.
  • Loading branch information
maumar committed Aug 27, 2016
1 parent e514b49 commit bd45e7d
Show file tree
Hide file tree
Showing 3 changed files with 174 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,14 @@ private static IEnumerable<Expression> ExtractProjections(TableExpressionBase ta

if (selectExpression != null)
{
return selectExpression.Projection.ToList();
if (selectExpression.IsProjectStar)
{
return selectExpression.Tables.SelectMany(ExtractProjections);
}
else
{
return selectExpression.Projection.ToList();
}
}

var joinExpression = tableExpression as JoinExpressionBase;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2473,6 +2473,78 @@ public virtual void Complex_multi_include_with_order_by_and_paging()
}
}

[ConditionalFact]
public virtual void Complex_multi_include_with_order_by_and_paging_joins_on_correct_key()
{
List<string> expected;

using (var context = CreateContext())
{
expected = context.LevelOne
.Include(e => e.OneToOne_Optional_FK).ThenInclude(e => e.OneToMany_Optional)
.Include(e => e.OneToOne_Required_FK).ThenInclude(e => e.OneToMany_Required)
.ToList()
.OrderBy(t => t.Name)
.Skip(0).Take(10)
.Select(e => e.Name)
.ToList();
}

ClearLog();

using (var context = CreateContext())
{
var query = context.LevelOne
.Include(e => e.OneToOne_Optional_FK).ThenInclude(e => e.OneToMany_Optional)
.Include(e => e.OneToOne_Required_FK).ThenInclude(e => e.OneToMany_Required)
.OrderBy(t => t.Name)
.Skip(0).Take(10);

var result = query.ToList();

Assert.Equal(expected.Count, result.Count);
for (var i = 0; i < result.Count; i++)
{
Assert.True(expected.Contains(result[i]?.Name));
}
}
}

[ConditionalFact]
public virtual void Complex_multi_include_with_order_by_and_paging_joins_on_correct_key2()
{
List<string> expected;

using (var context = CreateContext())
{
expected = context.LevelOne
.Include(e => e.OneToOne_Optional_FK.OneToOne_Required_FK).ThenInclude(e => e.OneToMany_Optional)
.ToList()
.OrderBy(t => t.Name)
.Skip(0).Take(10)
.Select(e => e.Name)
.ToList();
}

ClearLog();

using (var context = CreateContext())
{
var query = context.LevelOne
.Include(e => e.OneToOne_Optional_FK.OneToOne_Required_FK).ThenInclude(e => e.OneToMany_Optional)
.OrderBy(t => t.Name)
.Skip(0).Take(10);

var result = query.ToList();

Assert.Equal(expected.Count, result.Count);
for (var i = 0; i < result.Count; i++)
{
Assert.True(expected.Contains(result[i]?.Name));
}
}
}

[ConditionalFact]
public virtual void Correlated_subquery_doesnt_project_unnecessary_columns_in_top_level()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1288,7 +1288,7 @@ FROM [Level1] AS [e]
ORDER BY [e].[Name], [l].[Id], [l2].[Id]
OFFSET @__p_0 ROWS FETCH NEXT @__p_1 ROWS ONLY
) AS [t0]
) AS [l20] ON [l3].[OneToMany_Required_InverseId] = [l20].[Id]
) AS [l20] ON [l3].[OneToMany_Required_InverseId] = [l20].[Id0]
ORDER BY [l20].[Name], [l20].[Id], [l20].[Id0]
@__p_0: ?
Expand All @@ -1311,6 +1311,99 @@ OFFSET @__p_0 ROWS FETCH NEXT @__p_1 ROWS ONLY
}
}

public override void Complex_multi_include_with_order_by_and_paging_joins_on_correct_key()
{
base.Complex_multi_include_with_order_by_and_paging_joins_on_correct_key();

if (SupportsOffset)
{
Assert.Equal(
@"@__p_0: ?
@__p_1: ?
SELECT [e].[Id], [e].[Name], [e].[OneToMany_Optional_Self_InverseId], [e].[OneToMany_Required_Self_InverseId], [e].[OneToOne_Optional_SelfId], [l].[Id], [l].[Level1_Optional_Id], [l].[Level1_Required_Id], [l].[Name], [l].[OneToMany_Optional_InverseId], [l].[OneToMany_Optional_Self_InverseId], [l].[OneToMany_Required_InverseId], [l].[OneToMany_Required_Self_InverseId], [l].[OneToOne_Optional_PK_InverseId], [l].[OneToOne_Optional_SelfId], [l2].[Id], [l2].[Level1_Optional_Id], [l2].[Level1_Required_Id], [l2].[Name], [l2].[OneToMany_Optional_InverseId], [l2].[OneToMany_Optional_Self_InverseId], [l2].[OneToMany_Required_InverseId], [l2].[OneToMany_Required_Self_InverseId], [l2].[OneToOne_Optional_PK_InverseId], [l2].[OneToOne_Optional_SelfId]
FROM [Level1] AS [e]
LEFT JOIN [Level2] AS [l] ON [l].[Level1_Optional_Id] = [e].[Id]
LEFT JOIN [Level2] AS [l2] ON [l2].[Level1_Required_Id] = [e].[Id]
ORDER BY [e].[Name], [l].[Id], [l2].[Id]
OFFSET @__p_0 ROWS FETCH NEXT @__p_1 ROWS ONLY
@__p_0: ?
@__p_1: ?
SELECT [l3].[Id], [l3].[Level2_Optional_Id], [l3].[Level2_Required_Id], [l3].[Name], [l3].[OneToMany_Optional_InverseId], [l3].[OneToMany_Optional_Self_InverseId], [l3].[OneToMany_Required_InverseId], [l3].[OneToMany_Required_Self_InverseId], [l3].[OneToOne_Optional_PK_InverseId], [l3].[OneToOne_Optional_SelfId]
FROM [Level3] AS [l3]
INNER JOIN (
SELECT DISTINCT [t0].*
FROM (
SELECT [e].[Name], [l].[Id], [l2].[Id] AS [Id0]
FROM [Level1] AS [e]
LEFT JOIN [Level2] AS [l] ON [l].[Level1_Optional_Id] = [e].[Id]
LEFT JOIN [Level2] AS [l2] ON [l2].[Level1_Required_Id] = [e].[Id]
ORDER BY [e].[Name], [l].[Id], [l2].[Id]
OFFSET @__p_0 ROWS FETCH NEXT @__p_1 ROWS ONLY
) AS [t0]
) AS [l20] ON [l3].[OneToMany_Required_InverseId] = [l20].[Id0]
ORDER BY [l20].[Name], [l20].[Id], [l20].[Id0]
@__p_0: ?
@__p_1: ?
SELECT [l0].[Id], [l0].[Level2_Optional_Id], [l0].[Level2_Required_Id], [l0].[Name], [l0].[OneToMany_Optional_InverseId], [l0].[OneToMany_Optional_Self_InverseId], [l0].[OneToMany_Required_InverseId], [l0].[OneToMany_Required_Self_InverseId], [l0].[OneToOne_Optional_PK_InverseId], [l0].[OneToOne_Optional_SelfId]
FROM [Level3] AS [l0]
INNER JOIN (
SELECT DISTINCT [t].*
FROM (
SELECT [e].[Name], [l].[Id]
FROM [Level1] AS [e]
LEFT JOIN [Level2] AS [l] ON [l].[Level1_Optional_Id] = [e].[Id]
ORDER BY [e].[Name], [l].[Id]
OFFSET @__p_0 ROWS FETCH NEXT @__p_1 ROWS ONLY
) AS [t]
) AS [l1] ON [l0].[OneToMany_Optional_InverseId] = [l1].[Id]
ORDER BY [l1].[Name], [l1].[Id]",
Sql);
}
}

public override void Complex_multi_include_with_order_by_and_paging_joins_on_correct_key2()
{
base.Complex_multi_include_with_order_by_and_paging_joins_on_correct_key2();

if (SupportsOffset)
{
Assert.Equal(
@"@__p_0: ?
@__p_1: ?
SELECT [e].[Id], [e].[Name], [e].[OneToMany_Optional_Self_InverseId], [e].[OneToMany_Required_Self_InverseId], [e].[OneToOne_Optional_SelfId], [l].[Id], [l].[Level1_Optional_Id], [l].[Level1_Required_Id], [l].[Name], [l].[OneToMany_Optional_InverseId], [l].[OneToMany_Optional_Self_InverseId], [l].[OneToMany_Required_InverseId], [l].[OneToMany_Required_Self_InverseId], [l].[OneToOne_Optional_PK_InverseId], [l].[OneToOne_Optional_SelfId], [l0].[Id], [l0].[Level2_Optional_Id], [l0].[Level2_Required_Id], [l0].[Name], [l0].[OneToMany_Optional_InverseId], [l0].[OneToMany_Optional_Self_InverseId], [l0].[OneToMany_Required_InverseId], [l0].[OneToMany_Required_Self_InverseId], [l0].[OneToOne_Optional_PK_InverseId], [l0].[OneToOne_Optional_SelfId]
FROM [Level1] AS [e]
LEFT JOIN [Level2] AS [l] ON [l].[Level1_Optional_Id] = [e].[Id]
LEFT JOIN [Level3] AS [l0] ON [l0].[Level2_Required_Id] = [l].[Id]
ORDER BY [e].[Name], [l0].[Id]
OFFSET @__p_0 ROWS FETCH NEXT @__p_1 ROWS ONLY
@__p_0: ?
@__p_1: ?
SELECT [l1].[Id], [l1].[Level3_Optional_Id], [l1].[Level3_Required_Id], [l1].[Name], [l1].[OneToMany_Optional_InverseId], [l1].[OneToMany_Optional_Self_InverseId], [l1].[OneToMany_Required_InverseId], [l1].[OneToMany_Required_Self_InverseId], [l1].[OneToOne_Optional_PK_InverseId], [l1].[OneToOne_Optional_SelfId]
FROM [Level4] AS [l1]
INNER JOIN (
SELECT DISTINCT [t].*
FROM (
SELECT [e].[Name], [l0].[Id]
FROM [Level1] AS [e]
LEFT JOIN [Level2] AS [l] ON [l].[Level1_Optional_Id] = [e].[Id]
LEFT JOIN [Level3] AS [l0] ON [l0].[Level2_Required_Id] = [l].[Id]
ORDER BY [e].[Name], [l0].[Id]
OFFSET @__p_0 ROWS FETCH NEXT @__p_1 ROWS ONLY
) AS [t]
) AS [l00] ON [l1].[OneToMany_Optional_InverseId] = [l00].[Id]
ORDER BY [l00].[Name], [l00].[Id]",
Sql);
}
}

public override void Correlated_subquery_doesnt_project_unnecessary_columns_in_top_level()
{
base.Correlated_subquery_doesnt_project_unnecessary_columns_in_top_level();
Expand Down

0 comments on commit bd45e7d

Please sign in to comment.