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 4, 2020
1 parent dff6a1a commit a822d04
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 40 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

0 comments on commit a822d04

Please sign in to comment.