Skip to content

Commit

Permalink
Query: Add support for GroupBy patterns beyond aggregate
Browse files Browse the repository at this point in the history
- Allow expanding navigations after GroupBy operator applied before reducing it to non-grouping Fixes #22609
- Translate FirstOrDefault over grouping element Fixes #12088
- Add ability to select N element over grouping element Fixes #13805

Overall approach:
A grouping element (the range variable you get after applying GroupBy operator) is of type `IGrouping<TKey, TElement>` which implements `IEnumerable<TElement>`. Hence we treat this enumerable as if it is queryable during nav expansion phase. During translation phase we inject ShapedQueryExpression in place of the grouping element which is being enumerated. What this allows us is to expand navigation just like any other query root and translate a subquery similar to other subqueries to facilitate reusing same code for the tasks.
During translation phase in relational layer, since aggregate operation can be lifted into projection for SelectExpression containing SQL GROUP BY. This code path works in 2 ways, when translating we try to combine predicate/distinct into the aggregate operation (so in future when we support custom aggregate operators, we don't have to understand the shape of it to modify it later. When adding this scalar subquery to SelectExpression, we try to pattern match it to see if we can lift it. Further during lifting, we also lift any additional joins in the subquery (which implies there were some joins expanded on grouping element before aggregate) including the navigation expanded from owned navigations. A pending TODO is to de-dupe navigation expanded. It is not straight forward since aliases of table would have changed when previous was lifted.
Given every enumerable grouping element act as query root, every time we replace it inside a lambda expression, we need to create a copy of the root. Navigation expansion and individual queryableMethodTranslatingEV does this. So each root act and translate independently from each other.

Bug fixes:
- Fix a bug in identifying single result in InMemory to convert it to enumerable
- Null out _groupingParameter in InMemoryQueryExpression once the projection to reduce it has been applied
- Throw better error message when translating Min/Max over an entity type for InMemory
  • Loading branch information
smitpatel committed Aug 13, 2021
1 parent eb9a2b8 commit cfe84ba
Show file tree
Hide file tree
Showing 40 changed files with 3,796 additions and 1,126 deletions.
25 changes: 25 additions & 0 deletions src/EFCore.InMemory/Query/Internal/EntityProjectionExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,31 @@ public virtual void AddNavigationBinding(INavigation navigation, EntityShaperExp
: null;
}

/// <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 EntityProjectionExpression Clone()
{
var readExpressionMap = new Dictionary<IProperty, MethodCallExpression>();
foreach (var kvp in _readExpressionMap)
{
readExpressionMap.Add(kvp.Key, kvp.Value);
}
var entityProjectionExpression = new EntityProjectionExpression(EntityType, readExpressionMap);
foreach (var kvp in _navigationExpressionsCache)
{
entityProjectionExpression._navigationExpressionsCache[kvp.Key] = new EntityShaperExpression(
kvp.Value.EntityType,
((EntityProjectionExpression)kvp.Value.ValueBufferExpression).Clone(),
kvp.Value.IsNullable);
}

return entityProjectionExpression;
}

/// <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 Down

Large diffs are not rendered by default.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -173,5 +173,64 @@ protected override Expression VisitExtension(Expression extensionExpression)
: base.VisitExtension(extensionExpression);
}
}

private sealed class QueryExpressionReplacingExpressionVisitor : ExpressionVisitor
{
private readonly Expression _oldQuery;
private readonly Expression _newQuery;

public QueryExpressionReplacingExpressionVisitor(Expression oldQuery, Expression newQuery)
{
_oldQuery = oldQuery;
_newQuery = newQuery;
}

[return: NotNullIfNotNull("expression")]
public override Expression? Visit(Expression? expression)
{
return expression is ProjectionBindingExpression projectionBindingExpression
&& ReferenceEquals(projectionBindingExpression.QueryExpression, _oldQuery)
? projectionBindingExpression.ProjectionMember != null
? new ProjectionBindingExpression(
_newQuery, projectionBindingExpression.ProjectionMember!, projectionBindingExpression.Type)
: new ProjectionBindingExpression(
_newQuery, projectionBindingExpression.Index!.Value, projectionBindingExpression.Type)
: base.Visit(expression);
}
}

private sealed class CloningExpressionVisitor : ExpressionVisitor
{
[return: NotNullIfNotNull("expression")]
public override Expression? Visit(Expression? expression)
{
if (expression is InMemoryQueryExpression inMemoryQueryExpression)
{
var clonedInMemoryQueryExpression = new InMemoryQueryExpression(
inMemoryQueryExpression.ServerQueryExpression, inMemoryQueryExpression._valueBufferParameter)
{
_groupingParameter = inMemoryQueryExpression._groupingParameter,
_singleResultMethodInfo = inMemoryQueryExpression._singleResultMethodInfo,
_scalarServerQuery = inMemoryQueryExpression._scalarServerQuery
};

clonedInMemoryQueryExpression._clientProjections.AddRange(inMemoryQueryExpression._clientProjections.Select(e => Visit(e)));
clonedInMemoryQueryExpression._projectionMappingExpressions.AddRange(inMemoryQueryExpression._projectionMappingExpressions);
foreach (var item in inMemoryQueryExpression._projectionMapping)
{
clonedInMemoryQueryExpression._projectionMapping[item.Key] = Visit(item.Value);
}

return clonedInMemoryQueryExpression;
}

if (expression is EntityProjectionExpression entityProjectionExpression)
{
return entityProjectionExpression.Clone();
}

return base.Visit(expression);
}
}
}
}
58 changes: 50 additions & 8 deletions src/EFCore.InMemory/Query/Internal/InMemoryQueryExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,20 @@ private static readonly ConstructorInfo _resultEnumerableConstructor
private MethodInfo? _singleResultMethodInfo;
private bool _scalarServerQuery;

private CloningExpressionVisitor? _cloningExpressionVisitor;

private Dictionary<ProjectionMember, Expression> _projectionMapping = new();
private readonly List<Expression> _clientProjections = new();
private readonly List<Expression> _projectionMappingExpressions = new();

private InMemoryQueryExpression(
Expression serverQueryExpression,
ParameterExpression valueBufferParameter)
{
ServerQueryExpression = serverQueryExpression;
_valueBufferParameter = valueBufferParameter;
}

/// <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 Down Expand Up @@ -333,15 +343,17 @@ public virtual void ApplyProjection()
ServerQueryExpression,
selectorLambda);

_groupingParameter = null;

if (_singleResultMethodInfo != null)
{
ServerQueryExpression = Call(
_singleResultMethodInfo.MakeGenericMethod(CurrentParameter.Type),
ServerQueryExpression);

_singleResultMethodInfo = null;

ConvertToEnumerable();

_singleResultMethodInfo = null;
}
}

Expand Down Expand Up @@ -540,7 +552,7 @@ public virtual void ApplyDistinct()
/// 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 InMemoryGroupByShaperExpression ApplyGrouping(
public virtual GroupByShaperExpression ApplyGrouping(
Expression groupingKey,
Expression shaperExpression,
bool defaultElementSelector)
Expand Down Expand Up @@ -583,11 +595,15 @@ public virtual InMemoryGroupByShaperExpression ApplyGrouping(
keySelector,
selector);

return new InMemoryGroupByShaperExpression(
var clonedInMemoryQueryExpression = Clone();
clonedInMemoryQueryExpression.UpdateServerQueryExpression(_groupingParameter);
clonedInMemoryQueryExpression._groupingParameter = null;

return new GroupByShaperExpression(
groupingKey,
shaperExpression,
_groupingParameter,
_valueBufferParameter);
new ShapedQueryExpression(
clonedInMemoryQueryExpression,
new QueryExpressionReplacingExpressionVisitor(this, clonedInMemoryQueryExpression).Visit(shaperExpression)));
}

/// <summary>
Expand Down Expand Up @@ -711,6 +727,22 @@ public virtual EntityShaperExpression AddNavigationToWeakEntityType(
return entityShaper;
}

/// <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 ShapedQueryExpression Clone(Expression shaperExpression)
{
var clonedInMemoryQueryExpression = Clone();

return new ShapedQueryExpression(
clonedInMemoryQueryExpression,
new QueryExpressionReplacingExpressionVisitor(this, clonedInMemoryQueryExpression).Visit(shaperExpression));

}

/// <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 Down Expand Up @@ -811,6 +843,16 @@ void IPrintableExpression.Print(ExpressionPrinter expressionPrinter)
}
}

private InMemoryQueryExpression Clone()
{
if (_cloningExpressionVisitor == null)
{
_cloningExpressionVisitor = new();
}

return (InMemoryQueryExpression)_cloningExpressionVisitor.Visit(this);
}

private Expression GetGroupingKey(Expression key, List<Expression> groupingExpressions, Expression groupingKeyAccessExpression)
{
switch (key)
Expand Down Expand Up @@ -1061,7 +1103,7 @@ static Expression MakeNullable(Expression expression, bool nullable)

private void ConvertToEnumerable()
{
if (ServerQueryExpression.Type.TryGetSequenceType() == null)
if (_scalarServerQuery || _singleResultMethodInfo != null)
{
if (ServerQueryExpression.Type != typeof(ValueBuffer))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,24 @@ protected InMemoryQueryableMethodTranslatingExpressionVisitor(
protected override QueryableMethodTranslatingExpressionVisitor CreateSubqueryVisitor()
=> new InMemoryQueryableMethodTranslatingExpressionVisitor(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>
protected override Expression VisitExtension(Expression extensionExpression)
{
if (extensionExpression is GroupByShaperExpression groupByShaperExpression)
{
var shapedQueryExpression = groupByShaperExpression.GroupingEnumerable;
return ((InMemoryQueryExpression)shapedQueryExpression.QueryExpression)
.Clone(shapedQueryExpression.ShaperExpression);
}

return base.VisitExtension(extensionExpression);
}

/// <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 Down Expand Up @@ -1513,7 +1531,8 @@ private static Expression AccessField(
inMemoryQueryExpression.CurrentParameter)
: TranslateLambdaExpression(source, selector, preserveType: true);

if (selector == null)
if (selector == null
|| selector.Body is EntityProjectionExpression)
{
return null;
}
Expand Down
Loading

0 comments on commit cfe84ba

Please sign in to comment.