Skip to content

Commit

Permalink
Query: Allow final GroupBy entity type
Browse files Browse the repository at this point in the history 
Resolves #29199
  • Loading branch information
@smitpatel
smitpatel committed Sep 26, 2022
1 parent 5076987 commit 0d2eabe
Show file tree
Hide file tree
Showing 6 changed files with 118 additions and 10 deletions.
4 changes: 2 additions & 2 deletions src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -454,14 +454,14 @@ private static ShapedQueryExpression CreateShapedQueryExpression(IEntityType ent
{
// This could be group by entity type
if (remappedKeySelector is not EntityShaperExpression
{ ValueBufferExpression: ProjectionBindingExpression })
{ ValueBufferExpression: ProjectionBindingExpression pbe } ese)
{
// ValueBufferExpression can be JsonQuery, ProjectionBindingExpression, EntityProjection
// We only allow ProjectionBindingExpression which represents a regular entity
return null;
}

translatedKey = remappedKeySelector;
translatedKey = ese.Update(((SelectExpression)pbe.QueryExpression).GetProjection(pbe));
}

if (elementSelector != null)
Expand Down
49 changes: 48 additions & 1 deletion ...uery/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ protected override Expression VisitExtension(Expression extensionExpression)
switch (extensionExpression)
{
case RelationalEntityShaperExpression entityShaperExpression
when entityShaperExpression.ValueBufferExpression is ProjectionBindingExpression projectionBindingExpression:
when !_inline && entityShaperExpression.ValueBufferExpression is ProjectionBindingExpression projectionBindingExpression:
{
if (!_variableShaperMapping.TryGetValue(entityShaperExpression.ValueBufferExpression, out var accessor))
{
Expand Down Expand Up @@ -484,6 +484,53 @@ protected override Expression VisitExtension(Expression extensionExpression)
return accessor;
}

case RelationalEntityShaperExpression entityShaperExpression
when _inline && entityShaperExpression.ValueBufferExpression is ProjectionBindingExpression projectionBindingExpression:
{
if (GetProjectionIndex(projectionBindingExpression) is ValueTuple<int, List<(IProperty, int)>, string[]>
jsonProjectionIndex)
{
throw new InvalidOperationException();
//// json entity at the root
//var (jsonElementParameter, keyValuesParameter) = JsonShapingPreProcess(
// jsonProjectionIndex,
// entityShaperExpression.EntityType,
// isCollection: false);

//var shaperResult = CreateJsonShapers(
// entityShaperExpression.EntityType,
// entityShaperExpression.IsNullable,
// collection: false,
// jsonElementParameter,
// keyValuesParameter,
// parentEntityExpression: null,
// navigation: null);

//return Visit(shaperResult);
}
else
{
if (entityShaperExpression.EntityType.GetMappingStrategy() == RelationalAnnotationNames.TpcMappingStrategy)
{
var concreteTypes = entityShaperExpression.EntityType.GetDerivedTypesInclusive().Where(e => !e.IsAbstract())
.ToArray();
// Single concrete TPC entity type won't have discriminator column.
// We store the value here and inject it directly rather than reading from server.
if (concreteTypes.Length == 1)
{
_singleEntityTypeDiscriminatorValues[
(ProjectionBindingExpression)entityShaperExpression.ValueBufferExpression]
= concreteTypes[0].ShortName();
}
}

var entityMaterializationExpression = _parentVisitor.InjectEntityMaterializers(entityShaperExpression);
entityMaterializationExpression = Visit(entityMaterializationExpression);

return entityMaterializationExpression;
}
}

case CollectionResultExpression collectionResultExpression
when collectionResultExpression.Navigation is INavigation navigation
&& GetProjectionIndex(collectionResultExpression.ProjectionBindingExpression)
Expand Down
48 changes: 41 additions & 7 deletions src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -870,12 +870,13 @@ public Expression ApplyProjection(
var projectionBindingMap = new Dictionary<SqlExpression, ProjectionBindingExpression>();
var keySelector = AddGroupByKeySelectorToProjection(
this, newClientProjections, projectionBindingMap, groupByShaper.KeySelector);
var (keyIdentifier, keyIdentifierValueComparers) = GetIdentifierAccessor(projectionBindingMap, _identifier);
var (keyIdentifier, keyIdentifierValueComparers) = GetIdentifierAccessor(
this, newClientProjections, projectionBindingMap, _identifier);
_identifier.Clear();
_identifier.AddRange(_preGroupByIdentifier!);
_preGroupByIdentifier!.Clear();

static Expression AddGroupByKeySelectorToProjection(
Expression AddGroupByKeySelectorToProjection(
SelectExpression selectExpression,
List<Expression> clientProjectionList,
Dictionary<SqlExpression, ProjectionBindingExpression> projectionBindingMap,
Expand All @@ -884,20 +885,22 @@ static Expression AddGroupByKeySelectorToProjection(
switch (keySelector)
{
case SqlExpression sqlExpression:
{
var index = selectExpression.AddToProjection(sqlExpression);
var clientProjectionToAdd = Constant(index);
var existingIndex = clientProjectionList.FindIndex(
e => ExpressionEqualityComparer.Instance.Equals(e, clientProjectionToAdd));
if (existingIndex == -1)
{
clientProjectionList.Add(Constant(index));
clientProjectionList.Add(clientProjectionToAdd);
existingIndex = clientProjectionList.Count - 1;
}

var projectionBindingExpression = new ProjectionBindingExpression(
selectExpression, existingIndex, sqlExpression.Type.MakeNullable());
projectionBindingMap[sqlExpression] = projectionBindingExpression;
return projectionBindingExpression;
}

case NewExpression newExpression:
var newArguments = new Expression[newExpression.Arguments.Count];
Expand Down Expand Up @@ -936,21 +939,54 @@ static Expression AddGroupByKeySelectorToProjection(
AddGroupByKeySelectorToProjection(
selectExpression, clientProjectionList, projectionBindingMap, unaryExpression.Operand));

case EntityShaperExpression entityShaperExpression
when entityShaperExpression.ValueBufferExpression is EntityProjectionExpression entityProjectionExpression:
{
var clientProjectionToAdd = AddEntityProjection(entityProjectionExpression);
var existingIndex = clientProjectionList.FindIndex(
e => ExpressionEqualityComparer.Instance.Equals(e, clientProjectionToAdd));
if (existingIndex == -1)
{
clientProjectionList.Add(clientProjectionToAdd);
existingIndex = clientProjectionList.Count - 1;
}

return entityShaperExpression.Update(
new ProjectionBindingExpression(selectExpression, existingIndex, typeof(ValueBuffer)));
}

default:
throw new InvalidOperationException(
RelationalStrings.InvalidKeySelectorForGroupBy(keySelector, keySelector.GetType()));
}
}

static (Expression, IReadOnlyList<ValueComparer>) GetIdentifierAccessor(
SelectExpression selectExpression,
List<Expression> clientProjectionList,
Dictionary<SqlExpression, ProjectionBindingExpression> projectionBindingMap,
IEnumerable<(ColumnExpression Column, ValueComparer Comparer)> identifyingProjection)
{
var updatedExpressions = new List<Expression>();
var comparers = new List<ValueComparer>();
foreach (var (column, comparer) in identifyingProjection)
{
var projectionBindingExpression = projectionBindingMap[column];
if (!projectionBindingMap.TryGetValue(column, out var projectionBindingExpression))
{
var index = selectExpression.AddToProjection(column);
var clientProjectionToAdd = Constant(index);
var existingIndex = clientProjectionList.FindIndex(
e => ExpressionEqualityComparer.Instance.Equals(e, clientProjectionToAdd));
if (existingIndex == -1)
{
clientProjectionList.Add(clientProjectionToAdd);
existingIndex = clientProjectionList.Count - 1;
}

projectionBindingExpression = new ProjectionBindingExpression(
selectExpression, existingIndex, column.Type.MakeNullable());
}

updatedExpressions.Add(
projectionBindingExpression.Type.IsValueType
? Convert(projectionBindingExpression, typeof(object))
Expand Down Expand Up @@ -2005,9 +2041,7 @@ private static void PopulateGroupByTerms(
break;

case EntityShaperExpression entityShaperExpression
when entityShaperExpression.ValueBufferExpression is ProjectionBindingExpression projectionBindingExpression:
var entityProjectionExpression = (EntityProjectionExpression)((SelectExpression)projectionBindingExpression.QueryExpression)
.GetProjection(projectionBindingExpression);
when entityShaperExpression.ValueBufferExpression is EntityProjectionExpression entityProjectionExpression:
foreach (var property in GetAllPropertiesInHierarchy(entityProjectionExpression.EntityType))
{
PopulateGroupByTerms(entityProjectionExpression.BindProperty(property), groupByTerms, groupByAliases, name: null);
Expand Down
5 changes: 5 additions & 0 deletions test/EFCore.InMemory.FunctionalTests/Query/NorthwindGroupByQueryInMemoryTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ public override Task Final_GroupBy_property_entity(bool async)
() => base.Final_GroupBy_property_entity(async),
InMemoryStrings.NonComposedGroupByNotSupported);

public override Task Final_GroupBy_entity(bool async)
=> AssertTranslationFailedWithDetails(
() => base.Final_GroupBy_entity(async),
InMemoryStrings.NonComposedGroupByNotSupported);

public override Task Final_GroupBy_property_anonymous_type(bool async)
=> AssertTranslationFailedWithDetails(
() => base.Final_GroupBy_property_anonymous_type(async),
Expand Down
10 changes: 10 additions & 0 deletions test/EFCore.Specification.Tests/Query/NorthwindGroupByQueryTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2596,6 +2596,16 @@ public virtual Task Final_GroupBy_property_entity(bool async)
elementAsserter: (e, a) => AssertGrouping(e, a),
entryCount: 91);
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Final_GroupBy_entity(bool async)
=> AssertQuery(
async,
ss => ss.Set<Order>().Where(e => e.OrderID < 10500).GroupBy(c => c.Customer),
elementSorter: e => e.Key.CustomerID,
elementAsserter: (e, a) => AssertGrouping(e, a),
entryCount: 328);
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Final_GroupBy_property_anonymous_type(bool async)
Expand Down
12 changes: 12 additions & 0 deletions test/EFCore.SqlServer.FunctionalTests/Query/NorthwindGroupByQuerySqlServerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3142,6 +3142,18 @@ FROM [Customers] AS [c]
ORDER BY [c].[City]");
}

public override async Task Final_GroupBy_entity(bool async)
{
await base.Final_GroupBy_entity(async);

AssertSql(
@"SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region], [o].[OrderID], [o].[CustomerID], [o].[EmployeeID], [o].[OrderDate]
FROM [Orders] AS [o]
LEFT JOIN [Customers] AS [c] ON [o].[CustomerID] = [c].[CustomerID]
WHERE [o].[OrderID] < 10500
ORDER BY [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region]");
}

public override async Task Final_GroupBy_property_anonymous_type(bool async)
{
await base.Final_GroupBy_property_anonymous_type(async);
Expand Down

0 comments on commit 0d2eabe

Please sign in to comment.