Skip to content

Commit

Permalink
Fix to #15264 - QueryRewrite: incorporate query filters into nav rewrite
Browse files Browse the repository at this point in the history 
Also, fix to #13361 - Query: QueryFilters/Defining queries are not applied recursively

When nav rewrite constructs new EntityQueryable it now looks into EntityType for any query filter annotations and applies them on top. Those predicates are parameterized and the parameters created are injected into the context as part of query execution expression.

Query filters also fundamentally change how nav expansion creates new navigations - before navigations were being added one by one, so we could easily build the NavigationTree by a new node representing given INavigation.
With query filters, the newly created navigation may contain arbitrarily complex NavigationTree structure already (if the query filter itself has some navigations).
To support that, when we create new EntityQueryable for join or collection navigation, we need to visit it (creating NavigationExpansionExpression) and merge the resulting NavigationTree with the previous navigation tree.
  • Loading branch information
@maumar
maumar committed Jun 28, 2019
1 parent 4b98b55 commit 594b6ec
Show file tree
Hide file tree
Showing 31 changed files with 961 additions and 437 deletions.
2 changes: 1 addition & 1 deletion src/EFCore/Internal/EntityFinder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ private IQueryable<object[]> GetDatabaseValuesQuery(InternalEntityEntry entry)
keyValues[i] = keyValue;
}

return _queryRoot.AsNoTracking()//.IgnoreQueryFilters()
return _queryRoot.AsNoTracking().IgnoreQueryFilters()
.Where(BuildObjectLambda(properties, new ValueBuffer(keyValues)))
.Select(BuildProjection(entityType));
}
Expand Down
48 changes: 48 additions & 0 deletions src/EFCore/Query/NavigationExpansion/MaterializeCollectionNavigationExpression.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Linq.Expressions;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Query.Expressions.Internal;
using Microsoft.EntityFrameworkCore.Query.Internal;

namespace Microsoft.EntityFrameworkCore.Query.NavigationExpansion
{
public class MaterializeCollectionNavigationExpression : Expression, IPrintable
{
private Type _returnType;
public MaterializeCollectionNavigationExpression(Expression operand, INavigation navigation)
{
Operand = operand;
Navigation = navigation;
_returnType = navigation.ClrType;
}

public virtual Expression Operand { get; }
public virtual INavigation Navigation { get; }

public override ExpressionType NodeType => ExpressionType.Extension;
public override Type Type => _returnType;
public override bool CanReduce => false;

protected override Expression VisitChildren(ExpressionVisitor visitor)
{
var newOperand = visitor.Visit(Operand);

return Update(newOperand);
}

public virtual MaterializeCollectionNavigationExpression Update(Expression operand)
=> operand != Operand
? new MaterializeCollectionNavigationExpression(operand, Navigation)
: this;

public virtual void Print(ExpressionPrinter expressionPrinter)
{
expressionPrinter.StringBuilder.Append($"MATERIALIZE_COLLECTION({ Navigation }, ");
expressionPrinter.Visit(Operand);
expressionPrinter.StringBuilder.Append(")");
}
}
}
15 changes: 8 additions & 7 deletions src/EFCore/Query/NavigationExpansion/NavigationExpander.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,28 @@

using System.Linq.Expressions;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Query.NavigationExpansion.Visitors;
using Microsoft.EntityFrameworkCore.Query.Pipeline;
using Microsoft.EntityFrameworkCore.Utilities;

namespace Microsoft.EntityFrameworkCore.Query.NavigationExpansion
{
public class NavigationExpander
{
private IModel _model;
private readonly QueryCompilationContext _queryCompilationContext;

public NavigationExpander([NotNull] IModel model)
public NavigationExpander([NotNull] QueryCompilationContext queryCompilationContext)
{
Check.NotNull(model, nameof(model));
Check.NotNull(queryCompilationContext, nameof(queryCompilationContext));

_model = model;
_queryCompilationContext = queryCompilationContext;
}

public virtual Expression ExpandNavigations(Expression expression)
{
var newExpression = new NavigationExpandingVisitor(_model).Visit(expression);
newExpression = new NavigationExpansionReducingVisitor().Visit(newExpression);
var navigationExpandingVisitor = new NavigationExpandingVisitor(_queryCompilationContext);
var newExpression = navigationExpandingVisitor.Visit(expression);
newExpression = new NavigationExpansionReducingVisitor(navigationExpandingVisitor, _queryCompilationContext).Visit(newExpression);

return newExpression;
}
Expand Down
494 changes: 375 additions & 119 deletions src/EFCore/Query/NavigationExpansion/NavigationExpansionHelpers.cs
View file

Large diffs are not rendered by default.

57 changes: 57 additions & 0 deletions src/EFCore/Query/NavigationExpansion/NavigationTreeNode.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,56 @@ public static NavigationTreeNode Create(
return result;
}

private static void PrependFromMappings(NavigationTreeNode navigationTreeNode, List<List<string>> fromMappingsToPrepend)
{
var newFromMappings = new List<List<string>>();
foreach (var parentFromMapping in fromMappingsToPrepend)
{
foreach (var fromMapping in navigationTreeNode.FromMappings)
{
var newMapping = parentFromMapping.ToList();
newMapping.AddRange(fromMapping);
newFromMappings.Add(newMapping);
}
}

navigationTreeNode.FromMappings = newFromMappings;
foreach (var child in navigationTreeNode.Children)
{
PrependFromMappings(child, fromMappingsToPrepend);
}
}

public void AddChild([NotNull] NavigationTreeNode childNode, bool propagateFromMappings = true)
{
Check.NotNull(childNode, nameof(childNode));

// when adding the first child - propagate FromMappings from the parent
if (propagateFromMappings)
{
PrependFromMappings(childNode, FromMappings);
}

var existingChild = Children.Where(c => c.Navigation == childNode.Navigation).SingleOrDefault();
if (existingChild != null)
{
// if the child exisits, copy ToMappings, add new unique FromMappings and try adding it's children
// however for those children we don't need to re-propagate the mappings, since they are already in place
var newMappings = childNode.FromMappings.Where(m => !existingChild.FromMappings.Any(em => em.SequenceEqual(m)));
existingChild.ToMapping = childNode.ToMapping;
existingChild.FromMappings.AddRange(newMappings);
foreach (var grandChild in existingChild.Children)
{
existingChild.AddChild(grandChild, propagateFromMappings: false);
}
}
else
{
Children.Add(childNode);
childNode.Parent = this;
}
}

public List<NavigationTreeNode> Flatten()
{
var result = new List<NavigationTreeNode>();
Expand All @@ -150,5 +200,12 @@ public void MakeOptional()
{
Optional = true;
}

// TODO: hack - refactor this so that it's not needed
internal void SetNavigation(INavigation navigation)
{
Navigation = navigation;
PrependFromMappings(this, new List<List<string>> { new List<string> { navigation.Name } });
}
}
}
62 changes: 23 additions & 39 deletions src/EFCore/Query/NavigationExpansion/Visitors/CollectionNavigationRewritingVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Query.Internal;
using Microsoft.EntityFrameworkCore.Query.Pipeline;

namespace Microsoft.EntityFrameworkCore.Query.NavigationExpansion.Visitors
{
Expand All @@ -19,10 +20,17 @@ namespace Microsoft.EntityFrameworkCore.Query.NavigationExpansion.Visitors
public class CollectionNavigationRewritingVisitor : ExpressionVisitor
{
private readonly ParameterExpression _sourceParameter;
private readonly NavigationExpandingVisitor _navigationExpandingVisitor;
private readonly QueryCompilationContext _queryCompilationContext;

public CollectionNavigationRewritingVisitor(ParameterExpression sourceParameter)
public CollectionNavigationRewritingVisitor(
ParameterExpression sourceParameter,
NavigationExpandingVisitor navigationExpandingVisitor,
QueryCompilationContext queryCompilationContext)
{
_sourceParameter = sourceParameter;
_navigationExpandingVisitor = navigationExpandingVisitor;
_queryCompilationContext = queryCompilationContext;
}

protected override Expression VisitLambda<T>(Expression<T> lambdaExpression)
Expand Down Expand Up @@ -61,7 +69,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
&& methodCallExpression.Method.DeclaringType.IsGenericType
&& methodCallExpression.Method.DeclaringType.GetGenericTypeDefinition() == typeof(List<>))
{
var newCaller = RemoveMaterializeCollection(Visit(methodCallExpression.Object));
var newCaller = NavigationExpansionHelpers.RemoveMaterializeCollection(Visit(methodCallExpression.Object));
var newPredicate = Visit(methodCallExpression.Arguments[0]);

return Expression.Call(
Expand All @@ -79,7 +87,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
&& navigationBindingCaller.NavigationTreeNode.Navigation != null
&& navigationBindingCaller.NavigationTreeNode.Navigation.IsCollection())
{
var newCaller = RemoveMaterializeCollection(Visit(methodCallExpression.Object));
var newCaller = NavigationExpansionHelpers.RemoveMaterializeCollection(Visit(methodCallExpression.Object));
var newArgument = Visit(methodCallExpression.Arguments[0]);

var lambdaParameter = Expression.Parameter(newCaller.Type.GetSequenceType(), newCaller.Type.GetSequenceType().GenerateParameterName());
Expand All @@ -93,13 +101,13 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
lambda);
}

var newObject = RemoveMaterializeCollection(Visit(methodCallExpression.Object));
var newObject = NavigationExpansionHelpers.RemoveMaterializeCollection(Visit(methodCallExpression.Object));
var newArguments = new List<Expression>();

var argumentsChanged = false;
foreach (var argument in methodCallExpression.Arguments)
{
var newArgument = RemoveMaterializeCollection(Visit(argument));
var newArgument = NavigationExpansionHelpers.RemoveMaterializeCollection(Visit(argument));
newArguments.Add(newArgument);
if (newArgument != argument)
{
Expand All @@ -112,36 +120,12 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
: methodCallExpression;
}

private Expression RemoveMaterializeCollection(Expression expression)
{
if (expression is NavigationExpansionExpression navigationExpansionExpression
&& navigationExpansionExpression.State.MaterializeCollectionNavigation != null)
{
navigationExpansionExpression.State.MaterializeCollectionNavigation = null;

return new NavigationExpansionExpression(
navigationExpansionExpression.Operand,
navigationExpansionExpression.State,
navigationExpansionExpression.Operand.Type);
}

if (expression is NavigationExpansionRootExpression navigationExpansionRootExpression
&& navigationExpansionRootExpression.NavigationExpansion.State.MaterializeCollectionNavigation != null)
{
navigationExpansionRootExpression.NavigationExpansion.State.MaterializeCollectionNavigation = null;

var rewritten = new NavigationExpansionExpression(
navigationExpansionRootExpression.NavigationExpansion.Operand,
navigationExpansionRootExpression.NavigationExpansion.State,
navigationExpansionRootExpression.NavigationExpansion.Operand.Type);

return new NavigationExpansionRootExpression(rewritten, navigationExpansionRootExpression.Mapping);
}

return expression;
}

public static Expression CreateCollectionNavigationExpression(NavigationTreeNode navigationTreeNode, ParameterExpression rootParameter, SourceMapping sourceMapping)
public static NavigationExpansionExpression CreateCollectionNavigationExpression(
NavigationTreeNode navigationTreeNode,
ParameterExpression rootParameter,
SourceMapping sourceMapping,
NavigationExpandingVisitor navigationExpandingVisitor,
QueryCompilationContext queryCompilationContext)
{
var collectionEntityType = navigationTreeNode.Navigation.ForeignKey.DeclaringEntityType;
var entityQueryable = (Expression)NullAsyncQueryProvider.Instance.CreateEntityQueryableExpression(collectionEntityType.ClrType);
Expand Down Expand Up @@ -176,7 +160,7 @@ public static Expression CreateCollectionNavigationExpression(NavigationTreeNode
entityQueryable,
predicate);

var result = NavigationExpansionHelpers.CreateNavigationExpansionRoot(operand, collectionEntityType, navigationTreeNode.Navigation);
var result = NavigationExpansionHelpers.CreateNavigationExpansionRoot(operand, collectionEntityType, navigationTreeNode.Navigation, navigationExpandingVisitor, queryCompilationContext);

// this is needed for cases like: root.Include(r => r.Collection).ThenInclude(c => c.Reference).Select(r => r.Collection)
// result should be elements of the collection navigation with their 'Reference' included
Expand All @@ -196,8 +180,8 @@ protected override Expression VisitExtension(Expression extensionExpression)
&& lastNavigation.IsCollection())
{
return lastNavigation.ForeignKey.IsOwnership
? NavigationExpansionHelpers.CreateNavigationExpansionRoot(navigationBindingExpression, lastNavigation.GetTargetType(), lastNavigation)
: CreateCollectionNavigationExpression(navigationBindingExpression.NavigationTreeNode, navigationBindingExpression.RootParameter, navigationBindingExpression.SourceMapping);
? NavigationExpansionHelpers.CreateNavigationExpansionRoot(navigationBindingExpression, lastNavigation.GetTargetType(), lastNavigation, _navigationExpandingVisitor, _queryCompilationContext)
: CreateCollectionNavigationExpression(navigationBindingExpression.NavigationTreeNode, navigationBindingExpression.RootParameter, navigationBindingExpression.SourceMapping, _navigationExpandingVisitor, _queryCompilationContext);
}
else
{
Expand All @@ -210,7 +194,7 @@ protected override Expression VisitExtension(Expression extensionExpression)

protected override Expression VisitMember(MemberExpression memberExpression)
{
var newExpression = RemoveMaterializeCollection(Visit(memberExpression.Expression));
var newExpression = NavigationExpansionHelpers.RemoveMaterializeCollection(Visit(memberExpression.Expression));
if (newExpression != memberExpression.Expression)
{
if (memberExpression.Member.Name == nameof(List<int>.Count))
Expand Down
12 changes: 11 additions & 1 deletion src/EFCore/Query/NavigationExpansion/Visitors/IncludeApplyingVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,21 @@
using System.Linq;
using System.Linq.Expressions;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Query.Pipeline;

namespace Microsoft.EntityFrameworkCore.Query.NavigationExpansion.Visitors
{
public class PendingSelectorIncludeRewriter : ExpressionVisitor
{
private readonly NavigationExpandingVisitor _navigationExpandingVisitor;
private readonly QueryCompilationContext _queryCompilationContext;

public PendingSelectorIncludeRewriter(NavigationExpandingVisitor navigationExpandingVisitor, QueryCompilationContext queryCompilationContext)
{
_navigationExpandingVisitor = navigationExpandingVisitor;
_queryCompilationContext = queryCompilationContext;
}

protected override Expression VisitMember(MemberExpression memberExpression) => memberExpression;
protected override Expression VisitInvocation(InvocationExpression invocationExpression) => invocationExpression;
protected override Expression VisitLambda<T>(Expression<T> lambdaExpression) => lambdaExpression;
Expand Down Expand Up @@ -88,7 +98,7 @@ private IncludeExpression CreateIncludeReferenceCall(Expression caller, Navigati

private IncludeExpression CreateIncludeCollectionCall(Expression caller, NavigationTreeNode node, ParameterExpression rootParameter, SourceMapping sourceMapping)
{
var included = CollectionNavigationRewritingVisitor.CreateCollectionNavigationExpression(node, rootParameter, sourceMapping);
var included = CollectionNavigationRewritingVisitor.CreateCollectionNavigationExpression(node, rootParameter, sourceMapping, _navigationExpandingVisitor, _queryCompilationContext);

return new IncludeExpression(caller, included, node.Navigation);
}
Expand Down
Loading

0 comments on commit 594b6ec

Please sign in to comment.