Skip to content

Commit

Permalink
Query: Improvements to Relational GroupBy translation for composition
Browse files Browse the repository at this point in the history 
Part of #10012
  • Loading branch information
@smitpatel
smitpatel committed Mar 10, 2018
1 parent 7f75d7d commit 76df74f
Show file tree
Hide file tree
Showing 18 changed files with 472 additions and 452 deletions.
2 changes: 0 additions & 2 deletions ...FCore.Relational/Query/ExpressionVisitors/Internal/ResultTransformingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,9 @@ public class ResultTransformingExpressionVisitor<TResult> : ExpressionVisitorBas
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public ResultTransformingExpressionVisitor(
[NotNull] IQuerySource outerQuerySource,
[NotNull] RelationalQueryCompilationContext relationalQueryCompilationContext,
bool throwOnNullResult)
{
Check.NotNull(outerQuerySource, nameof(outerQuerySource));
Check.NotNull(relationalQueryCompilationContext, nameof(relationalQueryCompilationContext));

_relationalQueryCompilationContext = relationalQueryCompilationContext;
Expand Down
363 changes: 70 additions & 293 deletions src/EFCore.Relational/Query/ExpressionVisitors/RelationalProjectionExpressionVisitor.cs
View file

Large diffs are not rendered by default.

86 changes: 42 additions & 44 deletions src/EFCore.Relational/Query/ExpressionVisitors/SqlTranslatingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ protected override Expression VisitBinary(BinaryExpression expression)
switch (expression.NodeType)
{
case ExpressionType.Coalesce:
{
var left = Visit(expression.Left);
var right = Visit(expression.Right);

Expand All @@ -157,31 +156,25 @@ protected override Expression VisitBinary(BinaryExpression expression)
&& right.Type != typeof(Expression[])
? expression.Update(left, expression.Conversion, right)
: null;
}

case ExpressionType.Equal:
case ExpressionType.NotEqual:
{
var structuralComparisonExpression
= UnfoldStructuralComparison(
expression.NodeType,
ProcessComparisonExpression(expression));

return structuralComparisonExpression;
}

case ExpressionType.GreaterThan:
case ExpressionType.GreaterThanOrEqual:
case ExpressionType.LessThan:
case ExpressionType.LessThanOrEqual:
{
return ProcessComparisonExpression(expression);
}

case ExpressionType.AndAlso:
{
var left = Visit(expression.Left);
var right = Visit(expression.Right);
left = Visit(expression.Left);
right = Visit(expression.Right);

if (expression == _topLevelPredicate)
{
Expand Down Expand Up @@ -209,7 +202,6 @@ var structuralComparisonExpression
return left != null && right != null
? Expression.AndAlso(left, right)
: null;
}

case ExpressionType.OrElse:
case ExpressionType.Add:
Expand All @@ -219,7 +211,6 @@ var structuralComparisonExpression
case ExpressionType.Modulo:
case ExpressionType.And:
case ExpressionType.Or:
{
var leftExpression = Visit(expression.Left);
var rightExpression = Visit(expression.Right);

Expand All @@ -232,7 +223,6 @@ var structuralComparisonExpression
expression.IsLiftedToNull,
expression.Method)
: null;
}
}

return null;
Expand Down Expand Up @@ -794,30 +784,27 @@ protected override Expression VisitUnary(UnaryExpression expression)
switch (expression.NodeType)
{
case ExpressionType.Negate:
{
var operand = Visit(expression.Operand);
if (operand != null)
{
return Expression.Negate(operand);
}

break;
}

case ExpressionType.Not:
{
var operand = Visit(expression.Operand);
operand = Visit(expression.Operand);
if (operand != null)
{
return Expression.Not(operand);
}

break;
}

case ExpressionType.Convert:
{
var isTopLevelProjection = _isTopLevelProjection;
_isTopLevelProjection = false;
var operand = Visit(expression.Operand);
operand = Visit(expression.Operand);
_isTopLevelProjection = isTopLevelProjection;

if (operand != null)
Expand All @@ -832,7 +819,7 @@ protected override Expression VisitUnary(UnaryExpression expression)
}

break;
}

}

return null;
Expand Down Expand Up @@ -939,20 +926,42 @@ var subQueryModelVisitor
= (RelationalQueryModelVisitor)_queryModelVisitor.QueryCompilationContext
.CreateQueryModelVisitor(_queryModelVisitor);

if (expression.QueryModel.MainFromClause.FromExpression is QuerySourceReferenceExpression groupQsre
&& groupQsre.Type.IsGrouping())
{
var targetExpression = _queryModelVisitor.QueryCompilationContext.QuerySourceMapping
.GetExpression(groupQsre.ReferencedQuerySource);

if (targetExpression.Type == typeof(ValueBuffer))
{
var outerSelectExpression = _targetSelectExpression.Clone();
subQueryModelVisitor.AddQuery(subQueryModel.MainFromClause, outerSelectExpression);

_queryModelVisitor.QueryCompilationContext.AddOrUpdateMapping(
groupQsre.ReferencedQuerySource,
Expression.Parameter(
typeof(IEnumerable<>).MakeGenericType(typeof(ValueBuffer))));

subQueryModelVisitor.VisitSubQueryModel(subQueryModel);

_queryModelVisitor.QueryCompilationContext.AddOrUpdateMapping(
groupQsre.ReferencedQuerySource,
targetExpression);

if (outerSelectExpression.Projection.Count == 1)
{
return outerSelectExpression.Projection.Single();
}
}
}

var queriesProjectionCountMapping
= _queryModelVisitor.Queries
.ToDictionary(k => k, s => s.Projection.Count);

var queryModelMapping = new Dictionary<QueryModel, QueryModel>();
subQueryModel.PopulateQueryModelMapping(queryModelMapping);

var groupByTranslation = expression.QueryModel.MainFromClause.FromExpression.Type.IsGrouping();
if (groupByTranslation)
{
var outerSelectExpression = _targetSelectExpression.Clone();
subQueryModelVisitor.AddQuery(subQueryModel.MainFromClause, outerSelectExpression);
}

subQueryModelVisitor.VisitSubQueryModel(subQueryModel);

if (subQueryModelVisitor.IsLiftable)
Expand All @@ -968,12 +977,6 @@ var queriesProjectionCountMapping

_queryModelVisitor.LiftInjectedParameters(subQueryModelVisitor);

if (groupByTranslation
&& selectExpression.Projection.Count == 1)
{
return selectExpression.Projection.Single();
}

return selectExpression;
}

Expand Down Expand Up @@ -1041,7 +1044,6 @@ protected override Expression VisitExtension(Expression expression)
switch (expression)
{
case StringCompareExpression stringCompare:
{
var newLeft = Visit(stringCompare.Left);
var newRight = Visit(stringCompare.Right);
if (newLeft == null
Expand All @@ -1054,9 +1056,8 @@ protected override Expression VisitExtension(Expression expression)
|| newRight != stringCompare.Right
? new StringCompareExpression(stringCompare.Operator, newLeft, newRight)
: expression;
}

case ExplicitCastExpression explicitCast:
{
var newOperand = Visit(explicitCast.Operand);
if (newOperand == null)
{
Expand All @@ -1066,9 +1067,8 @@ protected override Expression VisitExtension(Expression expression)
return newOperand != explicitCast.Operand
? new ExplicitCastExpression(newOperand, explicitCast.Type)
: expression;
}

case NullConditionalExpression nullConditionalExpression:
{
var newAccessOperation = Visit(nullConditionalExpression.AccessOperation);
if (newAccessOperation == null)
{
Expand All @@ -1081,17 +1081,15 @@ protected override Expression VisitExtension(Expression expression)
}

return new NullableExpression(newAccessOperation);
}

case NullSafeEqualExpression nullSafeEqualExpression:
{
var equalityExpression
= new NullCompensatedExpression(nullSafeEqualExpression.EqualExpression);

return Visit(equalityExpression);
}

case NullCompensatedExpression nullCompensatedExpression:
{
var newOperand = Visit(nullCompensatedExpression.Operand);
newOperand = Visit(nullCompensatedExpression.Operand);
if (newOperand == null)
{
return null;
Expand All @@ -1100,7 +1098,7 @@ var equalityExpression
return newOperand != nullCompensatedExpression.Operand
? new NullCompensatedExpression(newOperand)
: nullCompensatedExpression;
}

case DiscriminatorPredicateExpression discriminatorPredicateExpression:
return new DiscriminatorPredicateExpression(
base.VisitExtension(expression), discriminatorPredicateExpression.QuerySource);
Expand Down
6 changes: 5 additions & 1 deletion src/EFCore.Relational/Query/Expressions/SelectExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,8 @@ public virtual bool IsIdentityQuery()
&& Offset == null
&& Projection.Count == 0
&& OrderBy.Count == 0
&& GroupBy.Count == 0
// GroupBy is intentionally ommitted because GroupBy does not require a pushdown.
//&& GroupBy.Count == 0
&& Tables.Count == 1;

/// <summary>
Expand Down Expand Up @@ -882,6 +883,9 @@ public virtual void AddToGroupBy([NotNull] Expression[] groupingExpressions)
{
Check.NotNull(groupingExpressions, nameof(groupingExpressions));

// Skip/Take will cause PushDown prior to calling this method so it is safe to clear ordering if any.
// Ordering before GroupBy Aggregate has no effect.
_orderBy.Clear();
_groupBy.AddRange(groupingExpressions);
}

Expand Down
Loading

0 comments on commit 76df74f

Please sign in to comment.