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.

Also small cleanup on ExpressionPrinter - adding IPrintable to some of the custom nodes created by nav expansion and removing rendering of connecting lines when printing the expression.
  • Loading branch information
maumar committed Aug 9, 2019
1 parent 31c0b5c commit bb5a176
Show file tree
Hide file tree
Showing 28 changed files with 546 additions and 432 deletions.
2 changes: 1 addition & 1 deletion src/EFCore/Internal/EntityFinder.cs
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
103 changes: 1 addition & 102 deletions src/EFCore/Internal/IndentedStringBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.IO;
using System.Text;
using JetBrains.Annotations;
Expand All @@ -18,11 +17,6 @@ namespace Microsoft.EntityFrameworkCore.Internal
public class IndentedStringBuilder
{
private const byte IndentSize = 4;

private readonly string _disconnectedIndent = new string(' ', IndentSize);
private readonly string _suspendedIndent = "|" + new string(' ', IndentSize - 1);
private readonly string _connectedIndent = "|" + new string('_', IndentSize - 2) + " ";

private byte _indent;
private bool _indentPending = true;

Expand Down Expand Up @@ -158,94 +152,19 @@ public virtual IndentedStringBuilder Clear()
return this;
}

private enum NodeConnectionState
{
Disconnected = 0,
Connected = 1,
Suspended = 2
}

private readonly List<NodeConnectionState> _nodeConnectionStates = new List<NodeConnectionState>();

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual IndentedStringBuilder IncrementIndent()
=> IncrementIndent(false);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual IndentedStringBuilder IncrementIndent(bool connectNode)
{
var state = connectNode ? NodeConnectionState.Connected : NodeConnectionState.Disconnected;
if (_indent == _nodeConnectionStates.Count)
{
_nodeConnectionStates.Add(state);
}
else
{
_nodeConnectionStates[_indent] = state;
}

_indent++;

return this;
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual void DisconnectCurrentNode()
{
if (_indent > 0
&& _nodeConnectionStates.Count >= _indent)
{
_nodeConnectionStates[_indent - 1] = NodeConnectionState.Disconnected;
}
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual void SuspendCurrentNode()
{
if (_indent > 0
&& _nodeConnectionStates.Count >= _indent
&& _nodeConnectionStates[_indent - 1] == NodeConnectionState.Connected)
{
_nodeConnectionStates[_indent - 1] = NodeConnectionState.Suspended;
}
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual void ReconnectCurrentNode()
{
if (_indent > 0
&& _nodeConnectionStates.Count >= _indent
&& _nodeConnectionStates[_indent - 1] == NodeConnectionState.Suspended)
{
_nodeConnectionStates[_indent - 1] = NodeConnectionState.Connected;
}
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
Expand All @@ -257,8 +176,6 @@ public virtual IndentedStringBuilder DecrementIndent()
if (_indent > 0)
{
_indent--;

_nodeConnectionStates.RemoveAt(_indent);
}

return this;
Expand All @@ -284,25 +201,7 @@ private void DoIndent()
{
if (_indentPending && (_indent > 0))
{
var indentString = string.Empty;
for (var i = 0; i < _nodeConnectionStates.Count; i++)
{
if (_nodeConnectionStates[i] == NodeConnectionState.Disconnected)
{
indentString += _disconnectedIndent;
}
else if (i == _nodeConnectionStates.Count - 1
&& _nodeConnectionStates[i] == NodeConnectionState.Connected)
{
indentString += _connectedIndent;
}
else
{
indentString += _suspendedIndent;
}
}

_stringBuilder.Append(indentString);
_stringBuilder.Append(new string(' ', _indent * IndentSize));
}

_indentPending = false;
Expand Down
99 changes: 49 additions & 50 deletions src/EFCore/Query/ExpressionPrinter.cs
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 All @@ -55,7 +57,7 @@ public ExpressionPrinter()

private int? CharacterLimit { get; set; }

private bool PrintConnections { get; set; }
private bool GenerateUniqueParameterIds { get; set; }

public virtual void VisitList<T>(
IReadOnlyList<T> items,
Expand Down Expand Up @@ -105,12 +107,6 @@ public virtual ExpressionPrinter IncrementIndent()
return this;
}

public virtual ExpressionPrinter IncrementIndent(bool connectNode)
{
_stringBuilder.IncrementIndent(connectNode);
return this;
}

public virtual ExpressionPrinter DecrementIndent()
{
_stringBuilder.DecrementIndent();
Expand All @@ -132,18 +128,32 @@ private void AppendLine([NotNull] string message)
}

public virtual string Print(
Expression expression,
bool removeFormatting = false,
int? characterLimit = null)
=> PrintCore(expression, removeFormatting, characterLimit, generateUniqueParameterIds: false);

public virtual string PrintDebug(
Expression expression,
bool removeFormatting = false,
int? characterLimit = null,
bool printConnections = true)
bool generateUniqueParameterIds = true)
=> PrintCore(expression, removeFormatting, characterLimit, generateUniqueParameterIds);

protected virtual string PrintCore(
Expression expression,
bool removeFormatting,
int? characterLimit,
bool generateUniqueParameterIds)
{
_stringBuilder.Clear();
_parametersInScope.Clear();
_namelessParameters.Clear();
_encounteredParameters.Clear();

RemoveFormatting = removeFormatting;
CharacterLimit = characterLimit;
PrintConnections = printConnections;
GenerateUniqueParameterIds = generateUniqueParameterIds;

Visit(expression);

Expand Down Expand Up @@ -318,12 +328,6 @@ protected override Expression VisitBinary(BinaryExpression binaryExpression)
protected override Expression VisitBlock(BlockExpression blockExpression)
{
AppendLine();

if (PrintConnections)
{
_stringBuilder.SuspendCurrentNode();
}

AppendLine("{");
_stringBuilder.IncrementIndent();

Expand Down Expand Up @@ -359,11 +363,6 @@ protected override Expression VisitBlock(BlockExpression blockExpression)
_stringBuilder.DecrementIndent();
Append("}");

if (PrintConnections)
{
_stringBuilder.ReconnectCurrentNode();
}

return blockExpression;
}

Expand All @@ -384,11 +383,6 @@ protected override Expression VisitConditional(ConditionalExpression conditional

protected override Expression VisitConstant(ConstantExpression constantExpression)
{
if (PrintConnections)
{
_stringBuilder.SuspendCurrentNode();
}

if (constantExpression.Value is IPrintable printable)
{
printable.Print(this);
Expand All @@ -402,11 +396,6 @@ protected override Expression VisitConstant(ConstantExpression constantExpressio
Print(constantExpression.Value);
}

if (PrintConnections)
{
_stringBuilder.ReconnectCurrentNode();
}

return constantExpression;
}

Expand Down Expand Up @@ -475,6 +464,20 @@ protected override Expression VisitLambda<T>(Expression<T> lambdaExpression)
}

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

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

if (parameter != lambdaExpression.Parameters.Last())
{
Expand Down Expand Up @@ -620,8 +623,7 @@ var argumentNames

if (!isSimpleMethodOrProperty)
{
var shouldPrintConnections = PrintConnections && !_nonConnectableMethods.Contains(methodCallExpression.Method.Name);
_stringBuilder.IncrementIndent(shouldPrintConnections);
_stringBuilder.IncrementIndent();
}

for (var i = 0; i < methodCallExpression.Arguments.Count; i++)
Expand All @@ -633,12 +635,6 @@ var argumentNames
_stringBuilder.Append(argumentNames[i] + ": ");
}

if (i == methodCallExpression.Arguments.Count - 1
&& !isSimpleMethodOrProperty)
{
_stringBuilder.DisconnectCurrentNode();
}

Visit(argument);

if (i < methodCallExpression.Arguments.Count - 1)
Expand Down Expand Up @@ -715,12 +711,6 @@ protected override Expression VisitNewArray(NewArrayExpression newArrayExpressio
var appendAction = isComplex ? (Action<string>)AppendLine : Append;

appendAction("new " + newArrayExpression.Type.GetElementType().ShortDisplayName() + "[]");

if (PrintConnections)
{
_stringBuilder.SuspendCurrentNode();
}

appendAction("{ ");

if (isComplex)
Expand All @@ -737,11 +727,6 @@ protected override Expression VisitNewArray(NewArrayExpression newArrayExpressio

Append("}");

if (PrintConnections)
{
_stringBuilder.ReconnectCurrentNode();
}

return newArrayExpression;
}

Expand Down Expand Up @@ -779,6 +764,21 @@ protected override Expression VisitParameter(ParameterExpression parameterExpres
Append(")");
}

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

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

return parameterExpression;
}

Expand Down Expand Up @@ -893,7 +893,6 @@ private void VisitArguments(IReadOnlyList<Expression> arguments, Action<string>
if (areConnected && i == arguments.Count - 1)
{
Append("");
_stringBuilder.DisconnectCurrentNode();
}

Visit(arguments[i]);
Expand Down
Loading

0 comments on commit bb5a176

Please sign in to comment.