Skip to content

Commit

Permalink
Query: Put client method to fetch join entity after filtered include …
Browse files Browse the repository at this point in the history 
…expression

Part of #19003

The issue was that we put the client method to fetch join entity directly after navigation hence the methods on filtered include threw client eval error.
Fix: We generate a join with both join/target entity and apply all filtered incldue expression by remapping them. At the end we apply client projection selector
  • Loading branch information
@smitpatel
smitpatel committed Jul 5, 2020
1 parent dff6a1a commit b2fb18f
Show file tree
Hide file tree
Showing 5 changed files with 141 additions and 59 deletions.
144 changes: 113 additions & 31 deletions src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.ExpressionVisitors.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -676,32 +676,6 @@ private Expression ExpandIncludesHelper(Expression root, EntityReference entityR
_ => throw new InvalidOperationException(CoreStrings.UnhandledNavigationBase(navigationBase.GetType()))
};

if (_queryStateManager
&& navigationBase is ISkipNavigation
&& included is MaterializeCollectionNavigationExpression materializeCollectionNavigationExpression
&& materializeCollectionNavigationExpression.Subquery is MethodCallExpression joinMethodCallExpression
&& joinMethodCallExpression.Method.IsGenericMethod
&& joinMethodCallExpression.Method.GetGenericMethodDefinition() == QueryableMethods.Join
&& joinMethodCallExpression.Arguments[4] is UnaryExpression unaryExpression
&& unaryExpression.NodeType == ExpressionType.Quote
&& unaryExpression.Operand is LambdaExpression resultSelectorLambda
&& resultSelectorLambda.Body == resultSelectorLambda.Parameters[1])
{
var joinParameter = resultSelectorLambda.Parameters[0];
var targetParameter = resultSelectorLambda.Parameters[1];
var newResultSelector = Expression.Quote(
Expression.Lambda(
Expression.Call(
_fetchJoinEntityMethodInfo.MakeGenericMethod(joinParameter.Type, targetParameter.Type),
joinParameter,
targetParameter),
joinParameter,
targetParameter));

included = materializeCollectionNavigationExpression.Update(
joinMethodCallExpression.Update(null, joinMethodCallExpression.Arguments.Take(4).Append(newResultSelector)));
}

// Collection will expand it's includes when reducing the navigationExpansionExpression
if (!navigationBase.IsCollection)
{
Expand All @@ -712,16 +686,89 @@ private Expression ExpandIncludesHelper(Expression root, EntityReference entityR

if (included is MaterializeCollectionNavigationExpression materializeCollectionNavigation)
{
var subquery = materializeCollectionNavigation.Subquery;
var filterExpression = entityReference.IncludePaths[navigationBase].FilterExpression;
if (filterExpression != null)
if (_queryStateManager
&& navigationBase is ISkipNavigation
&& subquery is MethodCallExpression joinMethodCallExpression
&& joinMethodCallExpression.Method.IsGenericMethod
&& joinMethodCallExpression.Method.GetGenericMethodDefinition() == QueryableMethods.Join
&& joinMethodCallExpression.Arguments[4] is UnaryExpression unaryExpression
&& unaryExpression.NodeType == ExpressionType.Quote
&& unaryExpression.Operand is LambdaExpression resultSelectorLambda
&& resultSelectorLambda.Body == resultSelectorLambda.Parameters[1])
{
var subquery = ReplacingExpressionVisitor.Replace(
filterExpression.Parameters[0],
materializeCollectionNavigation.Subquery,
filterExpression.Body);
var joinParameter = resultSelectorLambda.Parameters[0];
var targetParameter = resultSelectorLambda.Parameters[1];
if (filterExpression == null)
{
var newResultSelector = Expression.Quote(
Expression.Lambda(
Expression.Call(
_fetchJoinEntityMethodInfo.MakeGenericMethod(joinParameter.Type, targetParameter.Type),
joinParameter,
targetParameter),
joinParameter,
targetParameter));

subquery = joinMethodCallExpression.Update(
null, joinMethodCallExpression.Arguments.Take(4).Append(newResultSelector));
}
else
{
var resultType = TransparentIdentifierFactory.Create(joinParameter.Type, targetParameter.Type);

var transparentIdentifierOuterMemberInfo = resultType.GetTypeInfo().GetDeclaredField("Outer");
var transparentIdentifierInnerMemberInfo = resultType.GetTypeInfo().GetDeclaredField("Inner");

var newResultSelector = Expression.Quote(
Expression.Lambda(
Expression.New(
resultType.GetConstructors().Single(),
new[] { joinParameter, targetParameter },
transparentIdentifierOuterMemberInfo,
transparentIdentifierInnerMemberInfo),
joinParameter,
targetParameter));

var joinTypeParameters = joinMethodCallExpression.Method.GetGenericArguments();
joinTypeParameters[3] = resultType;
subquery = Expression.Call(
QueryableMethods.Join.MakeGenericMethod(joinTypeParameters),
joinMethodCallExpression.Arguments.Take(4).Append(newResultSelector));

var transparentIdentifierParameter = Expression.Parameter(resultType);
var transparentIdentifierInnerAccessor = Expression.MakeMemberAccess(
transparentIdentifierParameter, transparentIdentifierInnerMemberInfo);

subquery = RemapFilterExpressionForJoinEntity(
filterExpression.Parameters[0],
filterExpression.Body,
subquery,
transparentIdentifierParameter,
transparentIdentifierInnerAccessor);

var selector = Expression.Quote(
Expression.Lambda(
Expression.Call(
_fetchJoinEntityMethodInfo.MakeGenericMethod(joinParameter.Type, targetParameter.Type),
Expression.MakeMemberAccess(transparentIdentifierParameter, transparentIdentifierOuterMemberInfo),
transparentIdentifierInnerAccessor),
transparentIdentifierParameter));

subquery = Expression.Call(
QueryableMethods.Select.MakeGenericMethod(resultType, targetParameter.Type),
subquery,
selector);
}

included = materializeCollectionNavigation.Update(subquery);
}
else if (filterExpression != null)
{
subquery = ReplacingExpressionVisitor.Replace(filterExpression.Parameters[0], subquery, filterExpression.Body);
included = materializeCollectionNavigation.Update(subquery);
}
}

result = new IncludeExpression(result, included, navigationBase);
Expand All @@ -730,7 +777,42 @@ private Expression ExpandIncludesHelper(Expression root, EntityReference entityR
return result;
}

#pragma warning disable IDE0060 // Remove unused parameter
private static TTarget FetchJoinEntity<TJoin, TTarget>(TJoin joinEntity, TTarget targetEntity) => targetEntity;
#pragma warning restore IDE0060 // Remove unused parameter

private static Expression RemapFilterExpressionForJoinEntity(
ParameterExpression filterParameter,
Expression filterExpressionBody,
Expression subquery,
ParameterExpression transparentIdentifierParameter,
Expression transparentIdentifierInnerAccessor)
{
if (filterExpressionBody == filterParameter)
{
return subquery;
}

var methodCallExpression = (MethodCallExpression)filterExpressionBody;
var arguments = methodCallExpression.Arguments.ToArray();
arguments[0] = RemapFilterExpressionForJoinEntity(
filterParameter, arguments[0], subquery, transparentIdentifierParameter, transparentIdentifierInnerAccessor);
var genericParameters = methodCallExpression.Method.GetGenericArguments();
genericParameters[0] = transparentIdentifierParameter.Type;
var method = methodCallExpression.Method.GetGenericMethodDefinition().MakeGenericMethod(genericParameters);

if (arguments.Length == 2
&& arguments[1].GetLambdaOrNull() is LambdaExpression lambdaExpression)
{
arguments[1] = Expression.Quote(
Expression.Lambda(
ReplacingExpressionVisitor.Replace(
lambdaExpression.Parameters[0], transparentIdentifierInnerAccessor, lambdaExpression.Body),
transparentIdentifierParameter));
}

return Expression.Call(method, arguments);
}
}

/// <summary>
Expand Down
6 changes: 3 additions & 3 deletions ...FCore.Relational.Specification.Tests/Query/ManyToManyNoTrackingQueryRelationalTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,12 +255,12 @@ public virtual Task Filtered_include_on_skip_navigation_then_filtered_include_on
{
return AssertQuery(
async,
ss => ss.Set<EntityTwo>().Include(e => e.OneSkip.Where(i => i.Id > 15))
ss => ss.Set<EntityThree>().Include(e => e.OneSkipPayloadFull.Where(i => i.Id > 15))
.ThenInclude(e => e.Collection.Where(i => i.Id < 5))
.AsSplitQuery(),
elementAsserter: (e, a) => AssertInclude(e, a,
new ExpectedFilteredInclude<EntityTwo, EntityOne>(et => et.OneSkip, includeFilter: x => x.Where(i => i.Id > 15)),
new ExpectedFilteredInclude<EntityOne, EntityTwo>(et => et.Collection, "OneSkip", includeFilter: x => x.Where(i => i.Id < 5))));
new ExpectedFilteredInclude<EntityThree, EntityOne>(et => et.OneSkipPayloadFull, includeFilter: x => x.Where(i => i.Id > 15)),
new ExpectedFilteredInclude<EntityOne, EntityTwo>(et => et.Collection, "OneSkipPayloadFull", includeFilter: x => x.Where(i => i.Id < 5))));
}

[ConditionalTheory]
Expand Down
6 changes: 3 additions & 3 deletions test/EFCore.Relational.Specification.Tests/Query/ManyToManyQueryRelationalTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,12 +255,12 @@ public virtual Task Filtered_include_on_skip_navigation_then_filtered_include_on
{
return AssertQuery(
async,
ss => ss.Set<EntityTwo>().Include(e => e.OneSkip.Where(i => i.Id > 15))
ss => ss.Set<EntityThree>().Include(e => e.OneSkipPayloadFull.Where(i => i.Id > 15))
.ThenInclude(e => e.Collection.Where(i => i.Id < 5))
.AsSplitQuery(),
elementAsserter: (e, a) => AssertInclude(e, a,
new ExpectedFilteredInclude<EntityTwo, EntityOne>(et => et.OneSkip, includeFilter: x => x.Where(i => i.Id > 15)),
new ExpectedFilteredInclude<EntityOne, EntityTwo>(et => et.Collection, "OneSkip", includeFilter: x => x.Where(i => i.Id < 5))));
new ExpectedFilteredInclude<EntityThree, EntityOne>(et => et.OneSkipPayloadFull, includeFilter: x => x.Where(i => i.Id > 15)),
new ExpectedFilteredInclude<EntityOne, EntityTwo>(et => et.Collection, "OneSkipPayloadFull", includeFilter: x => x.Where(i => i.Id < 5))));
}

[ConditionalTheory]
Expand Down
6 changes: 3 additions & 3 deletions test/EFCore.Specification.Tests/Query/ManyToManyQueryTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -710,10 +710,10 @@ public virtual Task Filtered_include_on_skip_navigation_then_filtered_include_on
{
return AssertQuery(
async,
ss => ss.Set<EntityTwo>().Include(e => e.OneSkip.Where(i => i.Id > 15)).ThenInclude(e => e.Collection.Where(i => i.Id < 5)),
ss => ss.Set<EntityThree>().Include(e => e.OneSkipPayloadFull.Where(i => i.Id > 15)).ThenInclude(e => e.Collection.Where(i => i.Id < 5)),
elementAsserter: (e, a) => AssertInclude(e, a,
new ExpectedFilteredInclude<EntityTwo, EntityOne>(et => et.OneSkip, includeFilter: x => x.Where(i => i.Id > 15)),
new ExpectedFilteredInclude<EntityOne, EntityTwo>(et => et.Collection, "OneSkip", includeFilter: x => x.Where(i => i.Id < 5))),
new ExpectedFilteredInclude<EntityThree, EntityOne>(et => et.OneSkipPayloadFull, includeFilter: x => x.Where(i => i.Id > 15)),
new ExpectedFilteredInclude<EntityOne, EntityTwo>(et => et.Collection, "OneSkipPayloadFull", includeFilter: x => x.Where(i => i.Id < 5))),
entryCount: 0);
}

Expand Down
38 changes: 19 additions & 19 deletions test/EFCore.SqlServer.FunctionalTests/Query/ManyToManyNoTrackingQuerySqlServerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -916,20 +916,20 @@ public override async Task Filtered_include_on_skip_navigation_then_filtered_inc
await base.Filtered_include_on_skip_navigation_then_filtered_include_on_navigation(async);

AssertSql(
@"SELECT [e].[Id], [e].[CollectionInverseId], [e].[Name], [e].[ReferenceInverseId], [t0].[Id], [t0].[Name], [t0].[OneId], [t0].[TwoId], [t0].[Id0], [t0].[CollectionInverseId], [t0].[Name0], [t0].[ReferenceInverseId]
FROM [EntityTwos] AS [e]
@"SELECT [e].[Id], [e].[CollectionInverseId], [e].[Name], [e].[ReferenceInverseId], [t0].[Id], [t0].[Name], [t0].[OneId], [t0].[ThreeId], [t0].[Id0], [t0].[CollectionInverseId], [t0].[Name0], [t0].[ReferenceInverseId]
FROM [EntityThrees] AS [e]
LEFT JOIN (
SELECT [e0].[Id], [e0].[Name], [j].[OneId], [j].[TwoId], [t].[Id] AS [Id0], [t].[CollectionInverseId], [t].[Name] AS [Name0], [t].[ReferenceInverseId]
FROM [JoinOneToTwo] AS [j]
SELECT [e0].[Id], [e0].[Name], [j].[OneId], [j].[ThreeId], [t].[Id] AS [Id0], [t].[CollectionInverseId], [t].[Name] AS [Name0], [t].[ReferenceInverseId]
FROM [JoinOneToThreePayloadFull] AS [j]
INNER JOIN [EntityOnes] AS [e0] ON [j].[OneId] = [e0].[Id]
LEFT JOIN (
SELECT [e1].[Id], [e1].[CollectionInverseId], [e1].[Name], [e1].[ReferenceInverseId]
FROM [EntityTwos] AS [e1]
WHERE [e1].[Id] < 5
) AS [t] ON [e0].[Id] = [t].[CollectionInverseId]
WHERE [e0].[Id] > 15
) AS [t0] ON [e].[Id] = [t0].[TwoId]
ORDER BY [e].[Id], [t0].[OneId], [t0].[TwoId], [t0].[Id], [t0].[Id0]");
) AS [t0] ON [e].[Id] = [t0].[ThreeId]
ORDER BY [e].[Id], [t0].[OneId], [t0].[ThreeId], [t0].[Id], [t0].[Id0]");
}

public override async Task Filtered_include_on_navigation_then_filtered_include_on_skip_navigation(bool async)
Expand Down Expand Up @@ -1503,33 +1503,33 @@ public override async Task Filtered_include_on_skip_navigation_then_filtered_inc

AssertSql(
@"SELECT [e].[Id], [e].[CollectionInverseId], [e].[Name], [e].[ReferenceInverseId]
FROM [EntityTwos] AS [e]
FROM [EntityThrees] AS [e]
ORDER BY [e].[Id]",
//
@"SELECT [t].[Id], [t].[Name], [e].[Id], [t].[OneId], [t].[TwoId]
FROM [EntityTwos] AS [e]
@"SELECT [t].[Id], [t].[Name], [e].[Id], [t].[OneId], [t].[ThreeId]
FROM [EntityThrees] AS [e]
INNER JOIN (
SELECT [j].[OneId], [j].[TwoId], [e0].[Id], [e0].[Name]
FROM [JoinOneToTwo] AS [j]
SELECT [j].[OneId], [j].[ThreeId], [j].[Payload], [e0].[Id], [e0].[Name]
FROM [JoinOneToThreePayloadFull] AS [j]
INNER JOIN [EntityOnes] AS [e0] ON [j].[OneId] = [e0].[Id]
WHERE [e0].[Id] > 15
) AS [t] ON [e].[Id] = [t].[TwoId]
ORDER BY [e].[Id], [t].[OneId], [t].[TwoId], [t].[Id]",
) AS [t] ON [e].[Id] = [t].[ThreeId]
ORDER BY [e].[Id], [t].[OneId], [t].[ThreeId], [t].[Id]",
//
@"SELECT [t0].[Id], [t0].[CollectionInverseId], [t0].[Name], [t0].[ReferenceInverseId], [e].[Id], [t].[OneId], [t].[TwoId], [t].[Id]
FROM [EntityTwos] AS [e]
@"SELECT [t0].[Id], [t0].[CollectionInverseId], [t0].[Name], [t0].[ReferenceInverseId], [e].[Id], [t].[OneId], [t].[ThreeId], [t].[Id]
FROM [EntityThrees] AS [e]
INNER JOIN (
SELECT [j].[OneId], [j].[TwoId], [e0].[Id], [e0].[Name]
FROM [JoinOneToTwo] AS [j]
SELECT [j].[OneId], [j].[ThreeId], [j].[Payload], [e0].[Id], [e0].[Name]
FROM [JoinOneToThreePayloadFull] AS [j]
INNER JOIN [EntityOnes] AS [e0] ON [j].[OneId] = [e0].[Id]
WHERE [e0].[Id] > 15
) AS [t] ON [e].[Id] = [t].[TwoId]
) AS [t] ON [e].[Id] = [t].[ThreeId]
INNER JOIN (
SELECT [e1].[Id], [e1].[CollectionInverseId], [e1].[Name], [e1].[ReferenceInverseId]
FROM [EntityTwos] AS [e1]
WHERE [e1].[Id] < 5
) AS [t0] ON [t].[Id] = [t0].[CollectionInverseId]
ORDER BY [e].[Id], [t].[OneId], [t].[TwoId], [t].[Id]");
ORDER BY [e].[Id], [t].[OneId], [t].[ThreeId], [t].[Id]");
}

public override async Task Filtered_include_on_navigation_then_filtered_include_on_skip_navigation_split(bool async)
Expand Down

0 comments on commit b2fb18f

Please sign in to comment.