Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix to #15264 - QueryRewrite: incorporate query filters into nav rewrite #16327

Closed
wants to merge 1 commit into from
Closed

Fix to #15264 - QueryRewrite: incorporate query filters into nav rewrite #16327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/EFCore/Internal/EntityFinder.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop

Large diffs are not rendered by default.

57 changes: 57 additions & 0 deletions src/EFCore/Query/NavigationExpansion/NavigationTreeNode.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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