Skip to content

Commit

Permalink
Refactored to reduce in-place, all tests pass
Browse files Browse the repository at this point in the history 
Instead of having one pass to add wrapper nodes and another to remove
them, wrapped nodes are now reduced within the first place.

Lots of various other refactors and changes to make all tests pass.
  • Loading branch information
@roji
roji committed Jun 4, 2019
1 parent edeb215 commit cb40004
Show file tree
Hide file tree
Showing 9 changed files with 541 additions and 542 deletions.
13 changes: 5 additions & 8 deletions src/EFCore/Query/Expressions/Internal/NullConditionalExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,16 +110,13 @@ var operation
/// </returns>
/// <param name="visitor">An instance of <see cref="T:System.Func`2" />.</param>
protected override Expression VisitChildren(ExpressionVisitor visitor)
{
var newCaller = visitor.Visit(Caller);
var newAccessOperation = visitor.Visit(AccessOperation);
=> Update(visitor.Visit(Caller), visitor.Visit(AccessOperation));

return newCaller != Caller
|| newAccessOperation != AccessOperation
&& !(ExpressionEqualityComparer.Instance.Equals((newAccessOperation as NullConditionalExpression)?.AccessOperation, AccessOperation))
public virtual Expression Update(Expression newCaller, Expression newAccessOperation)
=> newCaller != Caller || newAccessOperation != AccessOperation
&& !ExpressionEqualityComparer.Instance.Equals((newAccessOperation as NullConditionalExpression)?.AccessOperation, AccessOperation)
? new NullConditionalExpression(newCaller, newAccessOperation)
: (this);
}
: this;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
149 changes: 0 additions & 149 deletions src/EFCore/Query/NavigationExpansion/Visitors/NavigationExpandingVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,154 +220,5 @@ protected override Expression VisitMember(MemberExpression memberExpression)
? ProcessMemberPushdown(newExpression, navigationExpansionExpression, efProperty: false, memberExpression.Member, propertyName: null, memberExpression.Type)
: memberExpression.Update(newExpression);
}

protected override Expression VisitBinary(BinaryExpression binaryExpression)
{
var leftConstantNull = binaryExpression.Left.IsNullConstantExpression();
var rightConstantNull = binaryExpression.Right.IsNullConstantExpression();

// collection comparison must be optimized out before we visit the left and right
// otherwise collections would be rewriteen and harder to identify
if (binaryExpression.NodeType == ExpressionType.Equal
|| binaryExpression.NodeType == ExpressionType.NotEqual)
{
var leftParent = default(Expression);
var leftNavigation = default(INavigation);
var rightParent = default(Expression);
var rightNavigation = default(INavigation);

// TODO: this is hacky and won't work for weak entity types
// also, add support for EF.Property and maybe convert node around the navigation
if (binaryExpression.Left is MemberExpression leftMember
&& leftMember.Type.TryGetSequenceType() is Type leftSequenceType
&& leftSequenceType != null
&& _model.FindEntityType(leftMember.Expression.Type) is IEntityType leftParentEntityType)
{
leftNavigation = leftParentEntityType.FindNavigation(leftMember.Member.Name);
if (leftNavigation != null)
{
leftParent = leftMember.Expression;
}
}

if (binaryExpression.Right is MemberExpression rightMember
&& rightMember.Type.TryGetSequenceType() is Type rightSequenceType
&& rightSequenceType != null
&& _model.FindEntityType(rightMember.Expression.Type) is IEntityType rightParentEntityType)
{
rightNavigation = rightParentEntityType.FindNavigation(rightMember.Member.Name);
if (rightNavigation != null)
{
rightParent = rightMember.Expression;
}
}

if (leftNavigation != null
&& leftNavigation.IsCollection()
&& leftNavigation == rightNavigation)
{
var rewritten = Expression.MakeBinary(binaryExpression.NodeType, leftParent, rightParent);

return Visit(rewritten);
}

if (leftNavigation != null
&& leftNavigation.IsCollection()
&& rightConstantNull)
{
var rewritten = Expression.MakeBinary(binaryExpression.NodeType, leftParent, Expression.Constant(null));

return Visit(rewritten);
}

if (rightNavigation != null
&& rightNavigation.IsCollection()
&& leftConstantNull)
{
var rewritten = Expression.MakeBinary(binaryExpression.NodeType, Expression.Constant(null), rightParent);

return Visit(rewritten);
}
}

var newLeft = Visit(binaryExpression.Left);
var newRight = Visit(binaryExpression.Right);

if (binaryExpression.NodeType == ExpressionType.Equal
|| binaryExpression.NodeType == ExpressionType.NotEqual)
{
var leftNavigationExpansionExpression = newLeft as NavigationExpansionExpression;
var rightNavigationExpansionExpression = newRight as NavigationExpansionExpression;
var leftNavigationBindingExpression = default(NavigationBindingExpression);
var rightNavigationBindingExpression = default(NavigationBindingExpression);

if (leftNavigationExpansionExpression?.State.PendingCardinalityReducingOperator != null)
{
leftNavigationBindingExpression = leftNavigationExpansionExpression.State.PendingSelector.Body as NavigationBindingExpression;
}

if (rightNavigationExpansionExpression?.State.PendingCardinalityReducingOperator != null)
{
rightNavigationBindingExpression = rightNavigationExpansionExpression.State.PendingSelector.Body as NavigationBindingExpression;
}

if (leftNavigationBindingExpression != null
&& rightConstantNull)
{
var comparisonArgumentsResult = CreateNullComparisonArguments(leftNavigationBindingExpression, leftNavigationExpansionExpression);
newLeft = comparisonArgumentsResult.navigationExpression;
newRight = comparisonArgumentsResult.nullKeyExpression;
}

if (rightNavigationBindingExpression != null
&& leftConstantNull)
{
var comparisonArgumentsResult = CreateNullComparisonArguments(rightNavigationBindingExpression, rightNavigationExpansionExpression);
newLeft = comparisonArgumentsResult.nullKeyExpression;
newRight = comparisonArgumentsResult.navigationExpression;
}

var result = binaryExpression.NodeType == ExpressionType.Equal
? Expression.Equal(newLeft, newRight)
: Expression.NotEqual(newLeft, newRight);

return result;
}

return binaryExpression.Update(newLeft, binaryExpression.Conversion, newRight);
}

private (NavigationExpansionExpression navigationExpression, Expression nullKeyExpression) CreateNullComparisonArguments(
NavigationBindingExpression navigationBindingExpression,
NavigationExpansionExpression navigationExpansionExpression)
{
var navigationKeyAccessExpression = NavigationExpansionHelpers.CreateKeyAccessExpression(
navigationBindingExpression,
navigationBindingExpression.EntityType.FindPrimaryKey().Properties,
addNullCheck: true);

var nullKeyExpression = NavigationExpansionHelpers.CreateNullKeyExpression(
navigationKeyAccessExpression.Type,
navigationBindingExpression.EntityType.FindPrimaryKey().Properties.Count);

var newNavigationExpansionExpressionState = new NavigationExpansionExpressionState(
navigationExpansionExpression.State.CurrentParameter,
navigationExpansionExpression.State.SourceMappings,
Expression.Lambda(navigationKeyAccessExpression, navigationExpansionExpression.State.PendingSelector.Parameters[0]),
applyPendingSelector: true,
navigationExpansionExpression.State.PendingOrderings,
navigationExpansionExpression.State.PendingIncludeChain,
navigationExpansionExpression.State.PendingCardinalityReducingOperator,
navigationExpansionExpression.State.PendingTags,
navigationExpansionExpression.State.CustomRootMappings,
navigationExpansionExpression.State.MaterializeCollectionNavigation);

var navigationExpression = new NavigationExpansionExpression(
navigationExpansionExpression.Operand,
newNavigationExpansionExpressionState,
navigationKeyAccessExpression.Type);

return (navigationExpression, nullKeyExpression);
}
}
}
Loading

0 comments on commit cb40004

Please sign in to comment.