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.

Only one filter is allowed per navigation. In cases where same navigation is included multiple times (e.g. Include(A).ThenInclude(A_B).Include(A).ThenInclude(A_C)) filter should only be applied once.
Alternatively the same exact filter should be applied to all.
  • Loading branch information
maumar committed Mar 23, 2020
1 parent ac46335 commit d9f78c4
Show file tree
Hide file tree
Showing 13 changed files with 1,105 additions and 11 deletions.
16 changes: 12 additions & 4 deletions src/EFCore/Properties/CoreStrings.Designer.cs

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

7 changes: 5 additions & 2 deletions src/EFCore/Properties/CoreStrings.resx
Expand Up @@ -747,8 +747,8 @@
<data name="AmbiguousForeignKeyPropertyCandidates" xml:space="preserve">
<value>Both relationships between '{firstDependentToPrincipalNavigationSpecification}' and '{firstPrincipalToDependentNavigationSpecification}' and between '{secondDependentToPrincipalNavigationSpecification}' and '{secondPrincipalToDependentNavigationSpecification}' could use {foreignKeyProperties} as the foreign key. To resolve this configure the foreign key properties explicitly on at least one of the relationships.</value>
</data>
<data name="InvalidIncludeLambdaExpression" xml:space="preserve">
<value>The {methodName} property lambda expression '{includeLambdaExpression}' is invalid. The expression should represent a property access: 't =&gt; t.MyProperty'. To target navigations declared on derived types, specify an explicitly typed lambda parameter of the target type, E.g. '(Derived d) =&gt; d.MyProperty'. For more information on including related data, see http://go.microsoft.com/fwlink/?LinkID=746393.</value>
<data name="InvalidIncludeExpression" xml:space="preserve">
<value>The expression '{expression}' is invalid inside Include operation. The expression should represent a property access: 't =&gt; t.MyProperty'. To target navigations declared on derived types, specify an explicitly typed lambda parameter of the target type, E.g. '(Derived d) =&gt; d.MyProperty'. Collection navigation access can be filtered by composing Where, OrderBy(Descending), ThenBy(Descending), Skip or Take operations. For more information on including related data, see http://go.microsoft.com/fwlink/?LinkID=746393.</value>
</data>
<data name="AbstractLeafEntityType" xml:space="preserve">
<value>The corresponding CLR type for entity type '{entityType}' is not instantiable and there is no derived entity type in the model that corresponds to a concrete CLR type.</value>
Expand Down Expand Up @@ -1290,6 +1290,9 @@
<data name="IncludeOnNonEntity" xml:space="preserve">
<value>Include has been used on non entity queryable.</value>
</data>
<data name="MultipleFilteredIncludesOnSameNavigation" xml:space="preserve">
<value>Different filters '{filter1}' and '{filter2}' have been applied on the same included navigation. Only one unique filter per navigation is allowed. For more information on including related data, see http://go.microsoft.com/fwlink/?LinkID=746393.</value>
</data>
<data name="CannotConvertQueryableToEnumerableMethod" xml:space="preserve">
<value>Unable to convert queryable method to enumerable method.</value>
</data>
Expand Down
Expand Up @@ -181,9 +181,8 @@ private Expression TryExpandNavigation(Expression root, MemberIdentity memberIde
var innerSource = (NavigationExpansionExpression)_navigationExpandingExpressionVisitor.Visit(innerQueryable);
if (entityReference.IncludePaths.ContainsKey(navigation))
{
var innerIncludeTreeNode = entityReference.IncludePaths[navigation];
var innerEntityReference = (EntityReference)((NavigationTreeExpression)innerSource.PendingSelector).Value;
innerEntityReference.SetIncludePaths(innerIncludeTreeNode);
innerEntityReference.SetIncludePaths(entityReference.IncludePaths[navigation]);
}

var innerSourceSequenceType = innerSource.Type.GetSequenceType();
Expand Down Expand Up @@ -532,6 +531,20 @@ private Expression ExpandIncludesHelper(Expression root, EntityReference entityR
included = ExpandIncludesHelper(included, innerEntityReference);
}

if (included is MaterializeCollectionNavigationExpression materializeCollectionNavigation)
{
var filterExpression = entityReference.IncludePaths[navigation].FilterExpression;
if (filterExpression != null)
{
var subquery = ReplacingExpressionVisitor.Replace(
filterExpression.Parameters[0],
materializeCollectionNavigation.Subquery,
filterExpression.Body);

included = materializeCollectionNavigation.Update(subquery);
}
}

result = new IncludeExpression(result, included, navigation);
}

Expand Down
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 Down
Expand Up @@ -32,6 +32,19 @@ public partial class NavigationExpandingExpressionVisitor : ExpressionVisitor
{ 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 @@ -786,19 +799,92 @@ private NavigationExpansionExpression ProcessInclude(NavigationExpansionExpressi
? entityReference.LastIncludeTreeNode
: entityReference.IncludePaths;
var includeLambda = expression.UnwrapLambdaFromQuote();
var lastIncludeTree = PopulateIncludeTree(currentIncludeTreeNode, includeLambda.Body);

var (result, filterExpression) = ExtractIncludeFilter(includeLambda.Body, includeLambda.Body);
var lastIncludeTree = PopulateIncludeTree(currentIncludeTreeNode, result);
if (lastIncludeTree == null)
{
throw new InvalidOperationException(CoreStrings.InvalidLambdaExpressionInsideInclude);
}

if (filterExpression != null)
{
if (lastIncludeTree.FilterExpression != null
&& !ExpressionEqualityComparer.Instance.Equals(filterExpression, lastIncludeTree.FilterExpression))
{
throw new InvalidOperationException(
CoreStrings.MultipleFilteredIncludesOnSameNavigation(
FormatFilter(filterExpression.Body).Print(),
FormatFilter(lastIncludeTree.FilterExpression.Body).Print()));
}

lastIncludeTree.FilterExpression = filterExpression;
}

entityReference.SetLastInclude(lastIncludeTree);
}

return source;
}

throw new InvalidOperationException(CoreStrings.IncludeOnNonEntity);

(Expression result, LambdaExpression filterExpression) ExtractIncludeFilter(Expression currentExpression, Expression includeExpression)
{
if (currentExpression is MemberExpression)
{
return (currentExpression, default(LambdaExpression));
}

if (currentExpression is MethodCallExpression methodCallExpression)
{
if (!methodCallExpression.Method.IsGenericMethod
|| !_supportedFilteredIncludeOperations.Contains(methodCallExpression.Method.GetGenericMethodDefinition()))
{
throw new InvalidOperationException(CoreStrings.InvalidIncludeExpression(includeExpression));
}

var (result, filterExpression) = ExtractIncludeFilter(methodCallExpression.Arguments[0], includeExpression);
if (filterExpression == null)
{
var prm = Expression.Parameter(result.Type);
filterExpression = Expression.Lambda(prm, prm);
}

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

return (result, filterExpression);
}

throw new InvalidOperationException(CoreStrings.InvalidIncludeExpression(includeExpression));
}

Expression FormatFilter(Expression expression)
{
if (expression is MethodCallExpression methodCallExpression
&& methodCallExpression.Method.IsGenericMethod
&& _supportedFilteredIncludeOperations.Contains(methodCallExpression.Method.GetGenericMethodDefinition()))
{
if (methodCallExpression.Method.GetGenericMethodDefinition() == QueryableMethods.AsQueryable)
{
return Expression.Parameter(expression.Type, "navigation");
}

var arguments = new List<Expression>();
var source = FormatFilter(methodCallExpression.Arguments[0]);
arguments.Add(source);
arguments.AddRange(methodCallExpression.Arguments.Skip(1));

return methodCallExpression.Update(methodCallExpression.Object, arguments);
}

return expression;
}
}

private NavigationExpansionExpression ProcessJoin(
Expand Down Expand Up @@ -1474,8 +1560,10 @@ private IncludeTreeNode PopulateIncludeTree(IncludeTreeNode includeTreeNode, Exp
if (navigation != null)
{
var addedNode = innerIncludeTreeNode.AddNavigation(navigation);

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

return addedNode;
}

Expand Down

0 comments on commit d9f78c4

Please sign in to comment.