From ba4fbca613058e43800d704f186d702e31dd837f Mon Sep 17 00:00:00 2001 From: maumar Date: Tue, 25 Jun 2019 17:54:10 -0700 Subject: [PATCH] Fix to #16231 - Query: Include on client projection gets ignored 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. --- .../Visitors/IncludeApplyingVisitor.cs | 16 ++++++++++++- .../Visitors/PendingIncludeFindingVisitor.cs | 14 ++++++++++- .../Query/GearsOfWarQueryTestBase.cs | 23 +++++++++++++++++++ .../Query/GearsOfWarQuerySqlServerTest.cs | 23 +++++++++++++++++++ 4 files changed, 74 insertions(+), 2 deletions(-) diff --git a/src/EFCore/Query/NavigationExpansion/Visitors/IncludeApplyingVisitor.cs b/src/EFCore/Query/NavigationExpansion/Visitors/IncludeApplyingVisitor.cs index 318d123d207..d53afdf9544 100644 --- a/src/EFCore/Query/NavigationExpansion/Visitors/IncludeApplyingVisitor.cs +++ b/src/EFCore/Query/NavigationExpansion/Visitors/IncludeApplyingVisitor.cs @@ -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(Expression lambdaExpression) => lambdaExpression; protected override Expression VisitTypeBinary(TypeBinaryExpression typeBinaryExpression) => typeBinaryExpression; diff --git a/src/EFCore/Query/NavigationExpansion/Visitors/PendingIncludeFindingVisitor.cs b/src/EFCore/Query/NavigationExpansion/Visitors/PendingIncludeFindingVisitor.cs index ae4dd3e703b..ffcbde22396 100644 --- a/src/EFCore/Query/NavigationExpansion/Visitors/PendingIncludeFindingVisitor.cs +++ b/src/EFCore/Query/NavigationExpansion/Visitors/PendingIncludeFindingVisitor.cs @@ -12,7 +12,19 @@ public class PendingIncludeFindingVisitor : ExpressionVisitor { public virtual Dictionary PendingIncludes { get; } = new Dictionary(); - 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(Expression lambdaExpression) => lambdaExpression; protected override Expression VisitTypeBinary(TypeBinaryExpression typeBinaryExpression) => typeBinaryExpression; diff --git a/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs index 6a221687bc3..bae12e0d50a 100644 --- a/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs @@ -7182,6 +7182,29 @@ public virtual Task Include_collection_with_Cast_to_base(bool isAsync) new List { new ExpectedInclude(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() { diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs index 149a810c078..5fdf5d4b07e 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs @@ -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();