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 24, 2020
1 parent c9f9c33 commit 21b9a35
Show file tree
Hide file tree
Showing 13 changed files with 1,104 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
Original file line number Diff line number Diff line change
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 use cast, e.g. 't =&gt; ((Derived)t).MyProperty' or 'as' operator, e.g. 't =&gt; (t as Derived).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
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,8 @@ protected Expression ExpandNavigation(
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
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 Down
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 @@ -768,6 +781,7 @@ private NavigationExpansionExpression ProcessInclude(NavigationExpansionExpressi
foreach (var navigation in FindNavigations(currentNode.EntityType, navigationName))
{
var addedNode = currentNode.AddNavigation(navigation);

// This is to add eager Loaded navigations when owner type is included.
PopulateEagerLoadedNavigations(addedNode);
includeTreeNodes.Enqueue(addedNode);
Expand All @@ -786,19 +800,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);

static (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));
}

static 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 +1561,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
Loading

5 comments on commit 21b9a35

@cgountanis
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a bit noobish on this process, when will this get rolled into the next SDK? Before 5.0 I hope, maybe 3.1.4?

@ErikEJ
Copy link
Contributor

@ErikEJ ErikEJ commented on 21b9a35 Mar 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5.0 (already in preview)

@Moumit
Copy link

@Moumit Moumit commented on 21b9a35 Aug 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this commit can be merged to support projects on .net framework? on 3.1..... legacy ..

@roji
Copy link
Member

@roji roji commented on 21b9a35 Aug 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Moumit unfortnuately not. We only backport important bug fixes into patch releases of existing versions (like 3.1). Backporting a big feature like this would be quite risky and could destabilize 3.1.

You can read more about our release planning in the docs.

@Moumit
Copy link

@Moumit Moumit commented on 21b9a35 Aug 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood @roji .. The problem is due to shared hosting server still does not support .net core .. we can not move to .net 5 or later so we unable to use these features and that creating many limitations

Please sign in to comment.