Skip to content

Commit

Permalink
Fix to #16231 - Query: Include on client projection gets ignored
Browse files Browse the repository at this point in the history
Problem was that when we search and apply include on the final projection we short circuit when we encounter member access. However, we should only do that if the member access is on top of navigation binding - in other cases, like client-side functions, we should peek inside and potentially apply includes on entities that are defined there.
  • Loading branch information
maumar authored and smitpatel committed Jun 28, 2019
1 parent fd4b5d6 commit ba4fbca
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,21 @@ namespace Microsoft.EntityFrameworkCore.Query.NavigationExpansion.Visitors
{
public class PendingSelectorIncludeRewriter : ExpressionVisitor
{
protected override Expression VisitMember(MemberExpression memberExpression) => memberExpression;
protected override Expression VisitMember(MemberExpression memberExpression)
{
if (memberExpression.Expression is NavigationBindingExpression navigationBindingExpression
&& navigationBindingExpression.EntityType.FindProperty(memberExpression.Member) != null)
{
return memberExpression;
}

var newExpression = Visit(memberExpression.Expression);

return newExpression != memberExpression.Expression
? Expression.MakeMemberAccess(newExpression, memberExpression.Member)
: memberExpression;
}

protected override Expression VisitInvocation(InvocationExpression invocationExpression) => invocationExpression;
protected override Expression VisitLambda<T>(Expression<T> lambdaExpression) => lambdaExpression;
protected override Expression VisitTypeBinary(TypeBinaryExpression typeBinaryExpression) => typeBinaryExpression;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,19 @@ public class PendingIncludeFindingVisitor : ExpressionVisitor
{
public virtual Dictionary<NavigationTreeNode, SourceMapping> PendingIncludes { get; } = new Dictionary<NavigationTreeNode, SourceMapping>();

protected override Expression VisitMember(MemberExpression memberExpression) => memberExpression;
protected override Expression VisitMember(MemberExpression memberExpression)
{
if (memberExpression.Expression is NavigationBindingExpression navigationBindingExpression
&& navigationBindingExpression.EntityType.FindProperty(memberExpression.Member) != null)
{
return memberExpression;
}

Visit(memberExpression.Expression);

return memberExpression;
}

protected override Expression VisitInvocation(InvocationExpression invocationExpression) => invocationExpression;
protected override Expression VisitLambda<T>(Expression<T> lambdaExpression) => lambdaExpression;
protected override Expression VisitTypeBinary(TypeBinaryExpression typeBinaryExpression) => typeBinaryExpression;
Expand Down
23 changes: 23 additions & 0 deletions test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7182,6 +7182,29 @@ public virtual Task Include_collection_with_Cast_to_base(bool isAsync)
new List<IExpectedInclude> { new ExpectedInclude<Gear>(e => e.Weapons, "Weapons") });
}

[ConditionalFact]
public virtual void Include_with_client_method_and_member_access_still_applies_includes()
{
using (var ctx = CreateContext())
{
var query = ctx.Gears
.Include(g => g.Tag)
.Select(g => new { g.Nickname, Client(g).FullName });

var result = query.ToList();
}
}

[ConditionalFact]
public virtual void Include_with_projection_of_unmapped_property_still_gets_applied()
{
using (var ctx = CreateContext())
{
var query = ctx.Gears.Include(g => g.Weapons).Select(g => g.IsMarcus);
var result = query.ToList();
}
}

[ConditionalFact(Skip = "Issue#16231")]
public virtual Task Multiple_includes_with_client_method_around_qsre_and_also_projecting_included_collection()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7691,6 +7691,29 @@ WHERE [g].[Discriminator] IN (N'Gear', N'Officer') AND ([g].[Discriminator] = N'
ORDER BY [g].[Nickname], [g].[SquadId], [w].[Id]");
}

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

AssertSql(
@"SELECT [g].[Nickname], [g].[SquadId], [g].[AssignedCityName], [g].[CityOrBirthName], [g].[Discriminator], [g].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[Rank], [t].[Id], [t].[GearNickName], [t].[GearSquadId], [t].[Note]
FROM [Gears] AS [g]
LEFT JOIN [Tags] AS [t] ON (([g].[Nickname] = [t].[GearNickName]) AND [t].[GearNickName] IS NOT NULL) AND (([g].[SquadId] = [t].[GearSquadId]) AND [t].[GearSquadId] IS NOT NULL)
WHERE [g].[Discriminator] IN (N'Gear', N'Officer')");
}

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

AssertSql(
@"SELECT [g].[Nickname], [g].[SquadId], [g].[AssignedCityName], [g].[CityOrBirthName], [g].[Discriminator], [g].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[Rank], [w].[Id], [w].[AmmunitionType], [w].[IsAutomatic], [w].[Name], [w].[OwnerFullName], [w].[SynergyWithId]
FROM [Gears] AS [g]
LEFT JOIN [Weapons] AS [w] ON [g].[FullName] = [w].[OwnerFullName]
WHERE [g].[Discriminator] IN (N'Gear', N'Officer')
ORDER BY [g].[Nickname], [g].[SquadId], [w].[Id]");
}

public override async Task Multiple_includes_with_client_method_around_qsre_and_also_projecting_included_collection()
{
await base.Multiple_includes_with_client_method_around_qsre_and_also_projecting_included_collection();
Expand Down

0 comments on commit ba4fbca

Please sign in to comment.