Skip to content

Commit

Permalink
Fix to #9395 - Query: Apply CompositePredicateExpressionVisitor to jo…
Browse files Browse the repository at this point in the history
…in predicates

Some optimizations that are applied to predicates in SelectExpression can be safely applied to join predicates as well. Fix modifies CompositePredicateExpressionVisitor to also look into JoinExpression predicates and not only SelectExpression predicates.
Also changes the way we build join predicates themselves, when inheritance is concerned - now the discriminator predicate will be extracted out of the conditional expression, so that the comparisons are sargable.

Before:

((a is Derived) ? a.DerivedFK : null) == b.PK - which translated to CASE block (not sargable)

After:

a is Derived && a.DerivedFK == b.PK
  • Loading branch information
maumar committed Feb 22, 2018
1 parent 1fcdbea commit 1a88d02
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 144 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ public CompositePredicateExpressionVisitor(bool useRelationalNulls)
protected override Expression VisitExtension(Expression expression)
{
var selectExpression = expression as SelectExpression;

if (selectExpression?.Predicate != null)
{
var predicate = new EqualityPredicateInExpressionOptimizer().Visit(selectExpression.Predicate);
Expand All @@ -53,6 +52,10 @@ protected override Expression VisitExtension(Expression expression)

selectExpression.Predicate = predicate;
}
else if (expression is PredicateJoinExpressionBase joinExpression)
{
joinExpression.Predicate = new EqualityPredicateInExpressionOptimizer().Visit(joinExpression.Predicate);
}

return base.VisitExtension(expression);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1104,6 +1104,12 @@ var equalityExpression
? new NullCompensatedExpression(newOperand)
: nullCompensatedExpression;
}
case DiscriminatorPredicateExpression discriminatorPredicateExpression:
{
var newPredicate = base.VisitExtension(expression);

return new DiscriminatorPredicateExpression(newPredicate, discriminatorPredicateExpression.QuerySource);
}
default:
return base.VisitExtension(expression);
}
Expand Down
31 changes: 30 additions & 1 deletion src/EFCore.Relational/Query/RelationalQueryModelVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
using Remotion.Linq.Clauses;
using Remotion.Linq.Clauses.Expressions;
using Remotion.Linq.Clauses.ResultOperators;
using Remotion.Linq.Parsing;

namespace Microsoft.EntityFrameworkCore.Query
{
Expand Down Expand Up @@ -1363,7 +1364,7 @@ var discriminatorPredicate
Expression.Constant(concreteEntityType.Relational().DiscriminatorValue, discriminatorPropertyExpression.Type)))
.Aggregate((current, next) => Expression.OrElse(next, current));

return discriminatorPredicate;
return new DiscriminatorPredicateExpression(discriminatorPredicate, typeBinaryExpression.Expression.TryGetReferencedQuerySource());
}

return Expression.Constant(true, typeof(bool));
Expand Down Expand Up @@ -1568,6 +1569,9 @@ var predicate
return false;
}

var discriminatorPredicateOptimizer = new DiscriminatorPredicateOptimizingVisitor();
predicate = discriminatorPredicateOptimizer.Visit(predicate);

QueriesBySource.Remove(joinClause);

outerSelectExpression.RemoveRangeFromProjection(previousProjectionCount);
Expand Down Expand Up @@ -1700,6 +1704,9 @@ var predicate
Expression.Equal(joinClause.OuterKeySelector, joinClause.InnerKeySelector));
}

var discriminatorPredicateOptimizer = new DiscriminatorPredicateOptimizingVisitor();
predicate = discriminatorPredicateOptimizer.Visit(predicate);

QueriesBySource.Remove(joinClause);

outerSelectExpression.RemoveRangeFromProjection(previousProjectionCount);
Expand Down Expand Up @@ -1778,6 +1785,28 @@ var newShapedQueryMethod
return true;
}

private class DiscriminatorPredicateOptimizingVisitor : RelinqExpressionVisitor
{
protected override Expression VisitBinary(BinaryExpression binaryExpression)
{
if (binaryExpression.NodeType == ExpressionType.Equal
&& binaryExpression.Left.RemoveConvert() is ConditionalExpression conditionalExpression
&& conditionalExpression.Test is DiscriminatorPredicateExpression discriminatorPredicateExpression
&& conditionalExpression.IfFalse.IsNullConstantExpression())
{
return Expression.AndAlso(
discriminatorPredicateExpression.Reduce(),
Expression.Equal(
conditionalExpression.IfTrue.Type == binaryExpression.Right.Type
? conditionalExpression.IfTrue
: Expression.Convert(conditionalExpression.IfTrue, binaryExpression.Right.Type),
binaryExpression.Right));
}

return base.VisitBinary(binaryExpression);
}
}

private bool IsFlattenableGroupJoinDefaultIfEmpty(
[NotNull] GroupJoinClause groupJoinClause,
QueryModel queryModel,
Expand Down
Loading

0 comments on commit 1a88d02

Please sign in to comment.