Skip to content

Commit

Permalink
Fix to #1833 - Support filtered Include
Browse files Browse the repository at this point in the history 
Allows for additional operations to be specified inside Include/ThenInclude expression when the navigation is a collection:
- Where,
- OrderBy(Descending)/ThenBy(Descending),
- Skip,
- Take.

Those additional operations are treated like any other within the query, so translation restrictions apply.
Collections included using new filter operations are considered to be loaded.
Applying multiple filtered includes on the same navigation is not supported.
  • Loading branch information
@maumar
maumar committed Mar 19, 2020
1 parent 2972c62 commit 44bd275
Show file tree
Hide file tree
Showing 12 changed files with 736 additions and 12 deletions.
16 changes: 16 additions & 0 deletions src/EFCore/Properties/CoreStrings.Designer.cs
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions src/EFCore/Properties/CoreStrings.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1290,6 +1290,12 @@
<data name="IncludeOnNonEntity" xml:space="preserve">
<value>Include has been used on non entity queryable.</value>
</data>
<data name="MultipleFilteredIncludesOnSameNavigation" xml:space="preserve">
<value>Multiple filtered include operations on the same navigation '{navigationName}' are not supported.</value>
</data>
<data name="FilteredIncludeOperationNotSupported" xml:space="preserve">
<value>Operation '{operationName}' is not supported inside Include method.</value>
</data>
<data name="CannotConvertQueryableToEnumerableMethod" xml:space="preserve">
<value>Unable to convert queryable method to enumerable method.</value>
</data>
Expand Down
23 changes: 18 additions & 5 deletions src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.ExpressionVisitors.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ protected Expression ExpandNavigation(
return expansion;
}

var filterExpression = default(LambdaExpression);
var targetType = navigation.TargetEntityType;
if (targetType.HasDefiningNavigation()
|| targetType.IsOwned())
Expand All @@ -159,16 +160,21 @@ protected Expression ExpandNavigation(
ownedEntityReference.MarkAsOptional();
if (entityReference.IncludePaths.ContainsKey(navigation))
{
ownedEntityReference.SetIncludePaths(entityReference.IncludePaths[navigation]);
var innerIncludeTreeNode = entityReference.IncludePaths[navigation];
filterExpression = innerIncludeTreeNode.FilterExpression;
ownedEntityReference.SetIncludePaths(innerIncludeTreeNode);
}

var ownedExpansion = new OwnedNavigationReference(root, navigation, ownedEntityReference);
if (navigation.IsCollection)
{
var elementType = ownedExpansion.Type.TryGetSequenceType();
var subquery = Expression.Call(
QueryableMethods.AsQueryable.MakeGenericMethod(elementType),
ownedExpansion);

var subquery = filterExpression != null
? ReplacingExpressionVisitor.Replace(filterExpression.Parameters[0], ownedExpansion, filterExpression.Body)
: Expression.Call(
QueryableMethods.AsQueryable.MakeGenericMethod(elementType),
ownedExpansion);

return new MaterializeCollectionNavigationExpression(subquery, navigation);
}
Expand All @@ -179,9 +185,11 @@ protected Expression ExpandNavigation(

var innerQueryable = new QueryRootExpression(targetType);
var innerSource = (NavigationExpansionExpression)_navigationExpandingExpressionVisitor.Visit(innerQueryable);

if (entityReference.IncludePaths.ContainsKey(navigation))
{
var innerIncludeTreeNode = entityReference.IncludePaths[navigation];
filterExpression = innerIncludeTreeNode.FilterExpression;
var innerEntityReference = (EntityReference)((NavigationTreeExpression)innerSource.PendingSelector).Value;
innerEntityReference.SetIncludePaths(innerIncludeTreeNode);
}
Expand Down Expand Up @@ -248,13 +256,18 @@ protected Expression ExpandNavigation(
Expression.Equal(outerKey, innerKey))
: Expression.Equal(outerKey, innerKey);

var subquery = Expression.Call(
var subquery = (Expression)Expression.Call(
QueryableMethods.Where.MakeGenericMethod(innerSourceSequenceType),
innerSource,
Expression.Quote(
Expression.Lambda(
predicateBody, innerParameter)));

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

return new MaterializeCollectionNavigationExpression(subquery, navigation);
}

Expand Down
10 changes: 9 additions & 1 deletion src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.Expressions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ protected class IncludeTreeNode : Dictionary<INavigation, IncludeTreeNode>
{
private EntityReference _entityReference;

public virtual LambdaExpression FilterExpression { get; set; }

public IncludeTreeNode(IEntityType entityType, EntityReference entityReference)
{
EntityType = entityType;
Expand All @@ -92,10 +94,16 @@ public IncludeTreeNode(IEntityType entityType, EntityReference entityReference)

public virtual IEntityType EntityType { get; private set; }

public virtual IncludeTreeNode AddNavigation(INavigation navigation)
public virtual IncludeTreeNode AddNavigation(INavigation navigation, bool withFilter)
{
if (TryGetValue(navigation, out var existingValue))
{
if (existingValue.FilterExpression != null
&& withFilter)
{
throw new NotSupportedException(CoreStrings.MultipleFilteredIncludesOnSameNavigation(navigation.Name));
}

return existingValue;
}

Expand Down
53 changes: 47 additions & 6 deletions src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,19 @@ private static readonly PropertyInfo _queryContextContextPropertyInfo
{ QueryableMethods.LastWithPredicate, QueryableMethods.LastWithoutPredicate },
{ QueryableMethods.LastOrDefaultWithPredicate, QueryableMethods.LastOrDefaultWithoutPredicate }
};

private static readonly List<MethodInfo> _supportedFilteredIncludeOperations = new List<MethodInfo>
{
QueryableMethods.Where,
QueryableMethods.OrderBy,
QueryableMethods.OrderByDescending,
QueryableMethods.ThenBy,
QueryableMethods.ThenByDescending,
QueryableMethods.Skip,
QueryableMethods.Take,
QueryableMethods.AsQueryable
};

private readonly QueryTranslationPreprocessor _queryTranslationPreprocessor;
private readonly QueryCompilationContext _queryCompilationContext;
private readonly PendingSelectorExpandingExpressionVisitor _pendingSelectorExpandingExpressionVisitor;
Expand Down Expand Up @@ -767,7 +780,7 @@ private NavigationExpansionExpression ProcessInclude(NavigationExpansionExpressi
var currentNode = includeTreeNodes.Dequeue();
foreach (var navigation in FindNavigations(currentNode.EntityType, navigationName))
{
var addedNode = currentNode.AddNavigation(navigation);
var addedNode = currentNode.AddNavigation(navigation, withFilter: false);
// This is to add eager Loaded navigations when owner type is included.
PopulateEagerLoadedNavigations(addedNode);
includeTreeNodes.Enqueue(addedNode);
Expand All @@ -786,7 +799,7 @@ private NavigationExpansionExpression ProcessInclude(NavigationExpansionExpressi
? entityReference.LastIncludeTreeNode
: entityReference.IncludePaths;
var includeLambda = expression.UnwrapLambdaFromQuote();
var lastIncludeTree = PopulateIncludeTree(currentIncludeTreeNode, includeLambda.Body);
var lastIncludeTree = PopulateIncludeTree(currentIncludeTreeNode, includeLambda.Body, withFilter: false);
if (lastIncludeTree == null)
{
throw new InvalidOperationException(CoreStrings.InvalidLambdaExpressionInsideInclude);
Expand Down Expand Up @@ -1445,12 +1458,12 @@ var outboundNavigations

foreach (var navigation in outboundNavigations)
{
var addedIncludeTreeNode = includeTreeNode.AddNavigation(navigation);
var addedIncludeTreeNode = includeTreeNode.AddNavigation(navigation, withFilter: false);
PopulateEagerLoadedNavigations(addedIncludeTreeNode);
}
}

private IncludeTreeNode PopulateIncludeTree(IncludeTreeNode includeTreeNode, Expression expression)
private IncludeTreeNode PopulateIncludeTree(IncludeTreeNode includeTreeNode, Expression expression, bool withFilter)
{
switch (expression)
{
Expand All @@ -1459,7 +1472,7 @@ private IncludeTreeNode PopulateIncludeTree(IncludeTreeNode includeTreeNode, Exp

case MemberExpression memberExpression:
var innerExpression = memberExpression.Expression.UnwrapTypeConversion(out var convertedType);
var innerIncludeTreeNode = PopulateIncludeTree(includeTreeNode, innerExpression);
var innerIncludeTreeNode = PopulateIncludeTree(includeTreeNode, innerExpression, withFilter: false);
var entityType = innerIncludeTreeNode.EntityType;
if (convertedType != null)
{
Expand All @@ -1473,13 +1486,41 @@ private IncludeTreeNode PopulateIncludeTree(IncludeTreeNode includeTreeNode, Exp
var navigation = entityType.FindNavigation(memberExpression.Member);
if (navigation != null)
{
var addedNode = innerIncludeTreeNode.AddNavigation(navigation);
var addedNode = innerIncludeTreeNode.AddNavigation(navigation, withFilter);
if (withFilter)
{
var prm = Expression.Parameter(expression.Type);
addedNode.FilterExpression = Expression.Lambda(prm, prm);
}

// This is to add eager Loaded navigations when owner type is included.
PopulateEagerLoadedNavigations(addedNode);

return addedNode;
}

break;

case MethodCallExpression methodCallExpression
when methodCallExpression.Method.DeclaringType == typeof(Queryable):
{
if (!methodCallExpression.Method.IsGenericMethod
|| !_supportedFilteredIncludeOperations.Contains(methodCallExpression.Method.GetGenericMethodDefinition()))
{
throw new NotSupportedException(CoreStrings.FilteredIncludeOperationNotSupported(methodCallExpression.Method.Name));
}

var result = PopulateIncludeTree(includeTreeNode, methodCallExpression.Arguments[0], withFilter: true);

var arguments = new List<Expression>();
arguments.Add(result.FilterExpression.Body);
arguments.AddRange(methodCallExpression.Arguments.Skip(1));
result.FilterExpression = Expression.Lambda(
methodCallExpression.Update(methodCallExpression.Object, arguments),
result.FilterExpression.Parameters);

return result;
}
}

return null;
Expand Down
Loading

0 comments on commit 44bd275

Please sign in to comment.