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 
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.
Generated predicate expression is re-visited by nav expansion to recursively handle filters (expand potential navigation expansions that were introduced in them).

This adds some complexity to the way navigations are expanded - before the newly constructed join always was guaranteed to be an entity. Now it can be a complex structure with multiple navigations already expanded.
  • Loading branch information
@maumar
maumar committed Aug 8, 2019
1 parent dfd323b commit c058f5b
Show file tree
Hide file tree
Showing 25 changed files with 582 additions and 286 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
26 changes: 25 additions & 1 deletion src/EFCore/Query/ExpressionPrinter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public class ExpressionPrinter : ExpressionVisitor
private readonly IndentedStringBuilder _stringBuilder;
private readonly Dictionary<ParameterExpression, string> _parametersInScope;
private readonly List<ParameterExpression> _namelessParameters;
private readonly List<ParameterExpression> _encounteredParameters;

private readonly Dictionary<ExpressionType, string> _binaryOperandMap = new Dictionary<ExpressionType, string>
{
Expand Down Expand Up @@ -47,6 +48,7 @@ public ExpressionPrinter()
_stringBuilder = new IndentedStringBuilder();
_parametersInScope = new Dictionary<ParameterExpression, string>();
_namelessParameters = new List<ParameterExpression>();
_encounteredParameters = new List<ParameterExpression>();
}

public virtual IndentedStringBuilder StringBuilder => _stringBuilder;
Expand Down Expand Up @@ -474,7 +476,17 @@ protected override Expression VisitLambda<T>(Expression<T> lambdaExpression)
_parametersInScope.Add(parameter, parameterName);
}

_stringBuilder.Append(parameter.Type.ShortDisplayName() + " " + parameterName);
var parameterIndex = _encounteredParameters.Count;
if (_encounteredParameters.Contains(parameter))
{
parameterIndex = _encounteredParameters.IndexOf(parameter);
}
else
{
_encounteredParameters.Add(parameter);
}

_stringBuilder.Append(parameter.Type.ShortDisplayName() + " " + parameterName + "{" + parameterIndex + "}");

if (parameter != lambdaExpression.Parameters.Last())
{
Expand Down Expand Up @@ -779,6 +791,18 @@ protected override Expression VisitParameter(ParameterExpression parameterExpres
Append(")");
}

var parameterIndex = _encounteredParameters.Count;
if (_encounteredParameters.Contains(parameterExpression))
{
parameterIndex = _encounteredParameters.IndexOf(parameterExpression);
}
else
{
_encounteredParameters.Add(parameterExpression);
}

_stringBuilder.Append("{" + parameterIndex + "}");

return parameterExpression;
}

Expand Down
25 changes: 11 additions & 14 deletions src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.ExpressionVisitors.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ protected Expression ExpandNavigation(
innerEntityReference.SetIncludePaths(innerIncludeTreeNode);
}

var innerParameter = Expression.Parameter(innerSource.SourceElementType, "i");
var innerParameter = Expression.Parameter(innerSource.Type.GetSequenceType(), "i");
Expression outerKey;
if (root is NavigationExpansionExpression innerNavigationExpansionExpression
&& innerNavigationExpansionExpression.CardinalityReducingGenericMethodInfo != null)
Expand All @@ -196,7 +196,7 @@ protected Expression ExpandNavigation(
: navigation.ForeignKey.PrincipalKey.Properties);
}

var innerKey = CreateKeyAccessExpression(innerParameter,
var innerKey = CreateKeyAccessExpression(innerSource.PendingSelector,
navigation.IsDependentToPrincipal()
? navigation.ForeignKey.PrincipalKey.Properties
: navigation.ForeignKey.Properties);
Expand All @@ -218,15 +218,12 @@ protected Expression ExpandNavigation(
{
// This is intentionally deferred to be applied to innerSource.Source
// Since outerKey's reference could change if a reference navigation is expanded afterwards
var correlationPredicate = _navigationExpandingExpressionVisitor.ExpandNavigationsInLambdaExpression(
innerSource,
Expression.Lambda(Expression.Equal(outerKey, innerKey), innerParameter));

var subquery = Expression.Call(
QueryableMethodProvider.WhereMethodInfo.MakeGenericMethod(innerSource.SourceElementType),
innerSource,
Expression.Quote(
Expression.Lambda(correlationPredicate, innerSource.CurrentParameter)));
QueryableMethodProvider.WhereMethodInfo.MakeGenericMethod(innerSource.Type.GetSequenceType()),
innerSource,
Expression.Quote(
Expression.Lambda(
Expression.Equal(outerKey, innerKey), innerParameter)));

return new MaterializeCollectionNavigationExpression(subquery, navigation);
}
Expand All @@ -241,8 +238,8 @@ protected Expression ExpandNavigation(
innerSource.CurrentParameter);

var resultSelectorOuterParameter = Expression.Parameter(_source.SourceElementType, "o");
var resultSelectorInnerParameter = Expression.Parameter(innerQueryableType, "i");
var resultType = TransparentIdentifierFactory.Create(_source.SourceElementType, innerQueryableType);
var resultSelectorInnerParameter = Expression.Parameter(innerSource.SourceElementType, "i");
var resultType = TransparentIdentifierFactory.Create(_source.SourceElementType, innerSource.SourceElementType);

var transparentIdentifierOuterMemberInfo = resultType.GetTypeInfo().GetDeclaredField("Outer");
var transparentIdentifierInnerMemberInfo = resultType.GetTypeInfo().GetDeclaredField("Inner");
Expand All @@ -269,11 +266,11 @@ protected Expression ExpandNavigation(
? QueryableMethodProvider.JoinMethodInfo
: QueryableExtensions.LeftJoinMethodInfo).MakeGenericMethod(
_source.SourceElementType,
innerQueryableType,
innerSource.SourceElementType,
outerKeySelector.ReturnType,
resultSelector.ReturnType),
_source.Source,
innerQueryable,
innerSource.Source,
outerKeySelector,
innerKeySelector,
resultSelector));
Expand Down
59 changes: 55 additions & 4 deletions src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.Expressions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace Microsoft.EntityFrameworkCore.Query.Internal
{
public partial class NavigationExpandingExpressionVisitor
{
public class NavigationExpansionExpression : Expression
public class NavigationExpansionExpression : Expression, IPrintable
{
private readonly List<(MethodInfo OrderingMethod, Expression KeySelector)> _pendingOrderings
= new List<(MethodInfo OrderingMethod, Expression KeySelector)>();
Expand Down Expand Up @@ -81,13 +81,26 @@ public virtual void ConvertToSingleResult(MethodInfo genericMethod)

protected override Expression VisitChildren(ExpressionVisitor visitor) => this;

public virtual void Print(ExpressionPrinter expressionPrinter)
{
expressionPrinter.Visit(Source);
expressionPrinter.StringBuilder.Append(".PendingSelector(");
expressionPrinter.Visit(PendingSelector);
expressionPrinter.StringBuilder.Append(")");

if (CardinalityReducingGenericMethodInfo != null)
{
expressionPrinter.StringBuilder.Append("." + CardinalityReducingGenericMethodInfo.Name + "()");
}
}

public override ExpressionType NodeType => ExpressionType.Extension;
public override Type Type => CardinalityReducingGenericMethodInfo == null
? typeof(IQueryable<>).MakeGenericType(PendingSelector.Type)
: PendingSelector.Type;
}

public class NavigationTreeExpression : NavigationTreeNode
public class NavigationTreeExpression : NavigationTreeNode, IPrintable
{
public NavigationTreeExpression(Expression value)
: base(null, null)
Expand All @@ -102,9 +115,27 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)
return this;
}
public override Type Type => Value.Type;

public override void Print(ExpressionPrinter expressionPrinter)
{
expressionPrinter.Append("NAVIGATION_TREE_EXPRESSION(");

var parent = Parent;
var currentParameter = CurrentParameter;
while (currentParameter == null)
{
currentParameter = parent.CurrentParameter;
parent = parent.Parent;
}

expressionPrinter.Visit(currentParameter);
expressionPrinter.Append(" | ");
expressionPrinter.Visit(Value);
expressionPrinter.Append(")");
}
}

public class EntityReference : Expression
public class EntityReference : Expression, IPrintable
{
public EntityReference(IEntityType entityType)
{
Expand Down Expand Up @@ -148,13 +179,26 @@ public virtual void MarkAsOptional()
IsOptional = true;
}

public virtual void Print(ExpressionPrinter expressionPrinter)
{
expressionPrinter.Append("ENTITY_REFERENCE(");
expressionPrinter.Append(EntityType.DisplayName());

if (IncludePaths.Count > 0)
{
expressionPrinter.Append(" | IncludePaths: " + string.Join(".", IncludePaths.Select(ip => ip.Key.Name)));
}

expressionPrinter.Append(")");
}

public virtual bool IsOptional { get; private set; }

public override ExpressionType NodeType => ExpressionType.Extension;
public override Type Type => EntityType.ClrType;
}

public class NavigationTreeNode : Expression
public class NavigationTreeNode : Expression, IPrintable
{
private NavigationTreeNode _parent;

Expand Down Expand Up @@ -203,6 +247,13 @@ public virtual Expression GetExpression()
? MakeMemberAccess(parentExperssion, parentExperssion.Type.GetTypeInfo().GetMember("Outer")[0])
: MakeMemberAccess(parentExperssion, parentExperssion.Type.GetTypeInfo().GetMember("Inner")[0]);
}

public virtual void Print(ExpressionPrinter expressionPrinter)
{
expressionPrinter.Append("NAVIGATION_TREE_NODE(");
expressionPrinter.Visit(CurrentParameter);
expressionPrinter.Append(")");
}
}

public class OwnedNavigationReference : Expression
Expand Down
70 changes: 62 additions & 8 deletions src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,38 +2,35 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Diagnostics;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using System.Runtime.InteropServices.ComTypes;
using Microsoft.EntityFrameworkCore.ChangeTracking;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Microsoft.EntityFrameworkCore.Query.Internal;

namespace Microsoft.EntityFrameworkCore.Query.Internal
{
public partial class NavigationExpandingExpressionVisitor : ExpressionVisitor
{
private readonly QueryCompilationContext _queryCompilationContext;
private readonly IModel _model;
private readonly bool _isTracking;
private readonly PendingSelectorExpandingExpressionVisitor _pendingSelectorExpandingExpressionVisitor;
private readonly SubqueryMemberPushdownExpressionVisitor _subqueryMemberPushdownExpressionVisitor;
private readonly ReducingExpressionVisitor _reducingExpressionVisitor;
private readonly ISet<string> _parameterNames = new HashSet<string>();
private readonly List<IEntityType> _appliedQueryFilters = new List<IEntityType>();

private static readonly MethodInfo _enumerableToListMethodInfo = typeof(Enumerable).GetTypeInfo()
.GetDeclaredMethods(nameof(Enumerable.ToList))
.Single(mi => mi.GetParameters().Length == 1);

public NavigationExpandingExpressionVisitor(QueryCompilationContext queryCompilationContext)
{
_queryCompilationContext = queryCompilationContext;
_model = queryCompilationContext.Model;
_isTracking = queryCompilationContext.IsTracking;
_pendingSelectorExpandingExpressionVisitor = new PendingSelectorExpandingExpressionVisitor(this);
Expand Down Expand Up @@ -74,7 +71,44 @@ protected override Expression VisitConstant(ConstantExpression constantExpressio
var currentTree = new NavigationTreeExpression(entityReference);
var parameterName = GetParameterName(entityType.ShortName()[0].ToString().ToLower());

return new NavigationExpansionExpression(constantExpression, currentTree, currentTree, parameterName);
var result = (Expression)new NavigationExpansionExpression(constantExpression, currentTree, currentTree, parameterName);

var rootEntityType = entityType.RootType();
var queryFilterAnnotation = rootEntityType.FindAnnotation("QueryFilter");
if (queryFilterAnnotation != null
&& !_queryCompilationContext.IgnoreQueryFilters
&& !_appliedQueryFilters.Contains(rootEntityType))
{
_appliedQueryFilters.Add(rootEntityType);
var filterPredicate = (LambdaExpression)queryFilterAnnotation.Value;

var parameterExtractingExpressionVisitor = new ParameterExtractingExpressionVisitor(
_queryCompilationContext.EvaluatableExpressionFilter,
_queryCompilationContext.ParameterValues,
_queryCompilationContext.ContextType,
_queryCompilationContext.Logger,
parameterize: false,
generateContextAccessors: true);

filterPredicate = (LambdaExpression)parameterExtractingExpressionVisitor.ExtractParameters(filterPredicate);
var sequenceType = result.Type.GetSequenceType();

// if we are constructing EntityQueryable of a derived type, we need to re-map filter predicate to the correct derived type
var filterPredicateParameter = filterPredicate.Parameters[0];
if (filterPredicateParameter.Type != sequenceType)
{
var newFilterPredicateParameter = Expression.Parameter(sequenceType, filterPredicateParameter.Name);
var newFilterPredicateBody = ReplacingExpressionVisitor.Replace(filterPredicateParameter, newFilterPredicateParameter, filterPredicate.Body);
filterPredicate = Expression.Lambda(newFilterPredicateBody, newFilterPredicateParameter);
}

var whereMethod = QueryableMethodProvider.WhereMethodInfo.MakeGenericMethod(result.Type.GetSequenceType());
var filteredResult = (Expression)Expression.Call(whereMethod, result, filterPredicate);

result = Visit(filteredResult);
}

return result;
}

return base.VisitConstant(constantExpression);
Expand Down Expand Up @@ -1180,11 +1214,31 @@ private Expression ExpandNavigationsInLambdaExpression(
NavigationExpansionExpression source,
LambdaExpression lambdaExpression)
{

var sikson = new Microsoft.EntityFrameworkCore.Query.ExpressionPrinter().Print(lambdaExpression.Body);

var lambdaBody = ReplacingExpressionVisitor.Replace(
lambdaExpression.Parameters[0],
source.PendingSelector,
lambdaExpression.Body);


var kupson = new Microsoft.EntityFrameworkCore.Query.ExpressionPrinter().Print(lambdaBody);

return ExpandNavigationsInExpression(source, lambdaBody);
}


private Expression ExpandNavigationsInLambdaExpression(
NavigationExpansionExpression source,
LambdaExpression lambdaExpression,
Expression expressionToReplace)
{
var lambdaBody = ReplacingExpressionVisitor.Replace(
expressionToReplace,
source.PendingSelector,
lambdaExpression.Body);

return ExpandNavigationsInExpression(source, lambdaBody);
}
}
Expand Down
9 changes: 9 additions & 0 deletions src/EFCore/Query/Internal/QueryMetadataExtractingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,15 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp

return innerQueryable;
}

if (genericMethodDefinition == EntityFrameworkQueryableExtensions.IgnoreQueryFiltersMethodInfo)
{
var innerQueryable = Visit(methodCallExpression.Arguments[0]);

_queryCompilationContext.IgnoreQueryFilters = true;

return innerQueryable;
}
}

return base.VisitMethodCall(methodCallExpression);
Expand Down
Loading

0 comments on commit c058f5b

Please sign in to comment.