Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Translate GroupBy followed by FirstOrDefault over group #12088

Closed
Tracked by #24106
smitpatel opened this issue May 21, 2018 · 30 comments · Fixed by #25495
Closed
Tracked by #24106

Translate GroupBy followed by FirstOrDefault over group #12088

smitpatel opened this issue May 21, 2018 · 30 comments · Fixed by #25495
Assignees
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. ef6-parity type-enhancement
Milestone

Comments

@smitpatel
Copy link
Member

No description provided.

@RainingNight
Copy link

??

@hannan-sultan
Copy link

Is there any update? Any alternative or planned fix? @ajcvickers @shanselman

@collinbarrett
Copy link

Is there a workaround for this that would allow for database-evaluated DistinctBy() logic? Seems like this Issue blocks database evaluation of this.

@madhav5589
Copy link

Any workaround while it's being fixed? I am in the process of migrating .NET Core from 2.1 to 3.1 and EF Core from 2.1 to 3.1.

@suadev
Copy link

suadev commented Mar 15, 2020

Another breaking change for migrating to ef core 3.1 and nobody knows when it will be supported :( @smitpatel @ajcvickers

@smitpatel
Copy link
Member Author

@suadev - It was getting client eval'ed before. It is part of same breaking change disabling client eval. The perf was bad for this case. If your group had 100 element each and you are selecting only first, we still got data for all 100 elements to client side to compute the first.

@reathh
Copy link

reathh commented May 7, 2020

Very important feature IMHO.
My use case is this:

await _db.RetailerRates
                .GroupBy(r => r.Price)
                .Select(g => g.OrderByDescending(r => r.CreatedAt).First())
                .ToListAsync();

@ajcvickers ajcvickers removed this from the Backlog milestone Jun 1, 2020
@ajcvickers ajcvickers added this to the Backlog milestone Jun 10, 2020
@GSPP
Copy link

GSPP commented Jun 29, 2020

This query pattern shows up quite a bit in my under-way 1000 table LINQ to SQL migration project.

Example use case: You have a list of blogs. For each blog you want to display a single highlighted post (the most upvoted or most recent one). So you might group by blog and take the first item from each group.

This can be translated with ROW_NUMBER ... WHERE r = 1 or with CROSS APPLY (SELECT TOP 1 ...). Both variants have different query plan shapes and performance characteristics. It would be nice if the shape could be selected by writing the LINQ query in a particular way.

This query could result in the ROW_NUMBER shape:

ctx.BlogPosts
.GroupBy(bp => bp.Blog)
.Select(g => g.OrderByDescending(bp => bp.CreatedAt).FirstOrDefault())
.ToList();

This query could result in the CROSS APPLY shape:

ctx.BlogPosts
.GroupBy(bp => bp.Blog)
.SelectMany(g => g.OrderByDescending(bp => bp.CreatedAt).Take(1).DefaultIfEmpty())
.ToList();

But any way to support this would be better than no way so that our migration can proceed.


The ROW_NUMBER plan leads SQL Server to sort all rows which could be billions. The CROSS APPLY lends itself to a loop join plan that very selectively queries an index. ROW_NUMBER is great if the fraction of rows selected is near 100%. CROSS APPLY is great when picking very few items.

Depending on the scenario the performance difference can be significant. SQL Server is unable to automatically switch between theses forms.

@lenniebriscoe
Copy link

lenniebriscoe commented Jul 14, 2020

@smitpatel @ajcvickers Is there any update on this issue or work around you could suggest for my case #21619 please?

If I distil my use-case it comes down to a simple multi-column group-by statement and a projection:

            var groupedOrganisationMentions = await collection
                .GroupBy(m => new {m.LocationId, m.PublicationId, m.PracticeAreaId})
                .Select(am => new
                {
                    Key = am.Key,
                    Items = am.ToList()
                }).ToListAsync(cancellationToken: token).ConfigureAwait(false);

The Items projection just doesn't want to work.

@brandonsmith86
Copy link

I've got a similar use case that seems to be related to this issue. I use code like this in several places:

// This works fine
query.Select(price => new Customer {
   Name = price.Payer.Name,
   Code = price.Payer.Code,
   City = price.Payer.City,
   ParentCode = price.Payer.ParentCode,
   ParentLevel = CustomerLevel.Corporate,
   CustomerLevel = CustomerLevel.Payer
}).Distinct().ToListAsync();

That works fine. The query variable could potentially have a variety of expressions and joins with no issue. As soon as I add a call to OrderBy, it will not evaluate. I've tried several workarounds that I've found around the interwebz, and nothing seems to resolve it.

// This throws error
// query is of type IQueryable<Price>
query.Select(price => new Customer {
   Name = price.Payer.Name,
   Code = price.Payer.Code,
   City = price.Payer.City,
   ParentCode = price.Payer.ParentCode,
   ParentLevel = CustomerLevel.Corporate,
   CustomerLevel = CustomerLevel.Payer
}).Distinct().OrderBy(cust => cust.Name).ToListAsync();

Also, placement of the OrderBy does not seem to matter.

// This also throws error
// query is of type IQueryable<Price>
query
   .OrderBy(price => price.payer.Name)
   .Select(price => new Customer {
      Name = price.Payer.Name,
      Code = price.Payer.Code,
      City = price.Payer.City,
      ParentCode = price.Payer.ParentCode,
      ParentLevel = CustomerLevel.Corporate,
      CustomerLevel = CustomerLevel.Payer
   }).Distinct().ToListAsync();

@NetMage
Copy link

NetMage commented Feb 25, 2021

@smitpatel

@suadev - It was getting client eval'ed before. It is part of same breaking change disabling client eval. The perf was bad for this case. If your group had 100 element each and you are selecting only first, we still got data for all 100 elements to client side to compute the first.

I don't think you can assume that - I believe this worked fine in EF 6 / LINQ to SQL so I wouldn't assume was using EF Core 2.x (I would never assume that - it was barely functional.)

@ajcvickers

@stevozilik EF6 runs on .NET Core, so even if you can't use EF Core due to this, it should not stop you migrating to .NET Core.

Will EF 6 be supported on .Net 6?

@smitpatel
Copy link
Member Author

@NetMage - The comment from suadev indicated that they ran into another breaking change when upgrading to EF Core 3.1. So yes it is fair to assume it is coming from earlier version of EF Core. When upgrading from EF6/LINQ to SQL, the concept of running into breaking change has no meaning. Changing from one product to another product will require code changes and there are no "breaking changes" in such change.

smitpatel added a commit that referenced this issue Apr 16, 2021
- ClientProjections is alternate when ProjectionMember binding is not possible using indexes
- ClientProjections is separate from SelectExpression.Projection where former holds complex client side mapped projections and later holds only scalars
- What it enables
  - Single result subquery is not joined right away
  - pendingCollections are removed
  - Ability to bind with above subquery translations after projection has converted to index based binding (enabling pending selector future change)
  - Ability to bind client projections smoothly like ProjectionMember binding so suppose translations post-client eval where it is supported
  - Grouping.FirstOrDefault can add subquery projection which applies separately so we can combine aggregate/non-aggregate projection on grouping
- Introduce CollectionResultExpression which holds info on how to create collection for a subquery
- ApplyProjection converts projectionMember/Index based bindings to actual scalar projection and updates shaper in same pass
- Unique collectionId are assigned by shaper
- Change in flow to projection to let sqlTranslator determines translatibility before processing for client side
- Regression in collection over single result subquery
  - Earlier we applied single result right away copying over pending collection to outer which allowed single result subquery to convert apply to join
  - Now all client projections apply at same time so pending collection (equivalent subquery) is applied inside single result subquery so we fail to convert apply to join

Ground work for #20291, #12088, #13805
Resolves #23571
smitpatel added a commit that referenced this issue Apr 16, 2021
- ClientProjections is alternate when ProjectionMember binding is not possible using indexes
- ClientProjections is separate from SelectExpression.Projection where former holds complex client side mapped projections and later holds only scalars
- What it enables
  - Single result subquery is not joined right away
  - pendingCollections are removed
  - Ability to bind with above subquery translations after projection has converted to index based binding (enabling pending selector future change)
  - Ability to bind client projections smoothly like ProjectionMember binding so suppose translations post-client eval where it is supported
  - Grouping.FirstOrDefault can add subquery projection which applies separately so we can combine aggregate/non-aggregate projection on grouping
- Introduce CollectionResultExpression which holds info on how to create collection for a subquery
- ApplyProjection converts projectionMember/Index based bindings to actual scalar projection and updates shaper in same pass
- Unique collectionId are assigned by shaper
- Change in flow to projection to let sqlTranslator determines translatibility before processing for client side
- Regression in collection over single result subquery
  - Earlier we applied single result right away copying over pending collection to outer which allowed single result subquery to convert apply to join
  - Now all client projections apply at same time so pending collection (equivalent subquery) is applied inside single result subquery so we fail to convert apply to join

Ground work for #20291, #12088, #13805
Resolves #23571
@Ruud-cb
Copy link

Ruud-cb commented Apr 20, 2021

Wondering what all of you have been using as a workaround up until this is fixed? Last time i got away with it by caching all the results and doing to query in the app instead of linq-to-sql. This time I really have to do it like @reathh shows.

So if anyone got a workaround, that would be great.

Very important feature IMHO.
My use case is this:

await _db.RetailerRates
                .GroupBy(r => r.Price)
                .Select(g => g.OrderByDescending(r => r.CreatedAt).First())
                .ToListAsync();

@DK8ALREC
Copy link

I got away with something like this before:

await _db.RetailerRates
  .GroupBy(x => x.Price)
  .Select(x => new { CreatedAt = x.Max(y => y.CreatedAt), Price = x.Key})
  .Join(_db.RetailerRates, x => x, x => new { x.CreatedAt, x.Price }, (_, retailerRate) => retailerRate);
  .ToListAsync();

Disclaimer: This solution depends on the uniqueness of your Price/CreatedAt combination. If not unique, then you might end up with multiple entities for each Price/CreatedAt combination. You could do some extra filtering after materializing the query to accomodate for this 🙂

@LeaFrock
Copy link

@Ruud-cb Try to use the codes below,

await _db.RetailerRates
                .Select(r => r.Price)
                .Distinct()
                .SelectMany(p => _db.RetailerRates.Where(r => r.Price == p).OrderByDescending(r => r.CreatedAt).Take(1))
                .ToListAsync()

For SQL Server, it will generate the sql with partition & inner join. The result is the same as group by.

@DK8ALREC
Copy link

@Ruud-cb Try to use the codes below,

await _db.RetailerRates
                .Select(r => r.Price)
                .Distinct()
                .SelectMany(p => _db.RetailerRates.Where(r => r.Price == p).OrderByDescending(r => r.CreatedAt).Take(1))
                .ToListAsync()

For SQL Server, it will generate the sql with partition & inner join. The result is the same as group by.

I think .Distinct() will materialize all distinct prices?

@LeaFrock
Copy link

@DK8ALREC Yes, it works as same as group by. And for linq-to-sql, it will be generated as select distinct(r.Price) from [RetailerRates] as [r].

@DK8ALREC
Copy link

@DK8ALREC Yes, it works as same as group by. And for linq-to-sql, it will be generated as select distinct(r.Price) from [RetailerRates] as [r].

Yes, my point was that I think your approach would make 2 database queries, and it will materialize all distinct Prices before making the "final query", which could potentially be a lot of rows I guess.

@LeaFrock
Copy link

Yes, my point was that I think your approach would make 2 database queries, and it will materialize all distinct Prices before making the "final query", which could potentially be a lot of rows I guess.

No, it won't. You just need to take a try :)

smitpatel added a commit that referenced this issue Apr 20, 2021
- ClientProjections is alternate when ProjectionMember binding is not possible using indexes
- ClientProjections is separate from SelectExpression.Projection where former holds complex client side mapped projections and later holds only scalars
- What it enables
  - Single result subquery is not joined right away
  - pendingCollections are removed
  - Ability to bind with above subquery translations after projection has converted to index based binding (enabling pending selector future change)
  - Ability to bind client projections smoothly like ProjectionMember binding so suppose translations post-client eval where it is supported
  - Grouping.FirstOrDefault can add subquery projection which applies separately so we can combine aggregate/non-aggregate projection on grouping
- Introduce CollectionResultExpression which holds info on how to create collection for a subquery
- ApplyProjection converts projectionMember/Index based bindings to actual scalar projection and updates shaper in same pass
- Unique collectionId are assigned by shaper
- Change in flow to projection to let sqlTranslator determines translatibility before processing for client side
- Regression in collection over single result subquery
  - Earlier we applied single result right away copying over pending collection to outer which allowed single result subquery to convert apply to join
  - Now all client projections apply at same time so pending collection (equivalent subquery) is applied inside single result subquery so we fail to convert apply to join

Ground work for #20291, #12088, #13805
Resolves #23571
ghost pushed a commit that referenced this issue Apr 20, 2021
- ClientProjections is alternate when ProjectionMember binding is not possible using indexes
- ClientProjections is separate from SelectExpression.Projection where former holds complex client side mapped projections and later holds only scalars
- What it enables
  - Single result subquery is not joined right away
  - pendingCollections are removed
  - Ability to bind with above subquery translations after projection has converted to index based binding (enabling pending selector future change)
  - Ability to bind client projections smoothly like ProjectionMember binding so suppose translations post-client eval where it is supported
  - Grouping.FirstOrDefault can add subquery projection which applies separately so we can combine aggregate/non-aggregate projection on grouping
- Introduce CollectionResultExpression which holds info on how to create collection for a subquery
- ApplyProjection converts projectionMember/Index based bindings to actual scalar projection and updates shaper in same pass
- Unique collectionId are assigned by shaper
- Change in flow to projection to let sqlTranslator determines translatibility before processing for client side
- Regression in collection over single result subquery
  - Earlier we applied single result right away copying over pending collection to outer which allowed single result subquery to convert apply to join
  - Now all client projections apply at same time so pending collection (equivalent subquery) is applied inside single result subquery so we fail to convert apply to join

Ground work for #20291, #12088, #13805
Resolves #23571
smitpatel added a commit that referenced this issue Aug 12, 2021
- 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
@smitpatel smitpatel added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Aug 12, 2021
smitpatel added a commit that referenced this issue Aug 12, 2021
- 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
smitpatel added a commit that referenced this issue Aug 12, 2021
- 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
smitpatel added a commit that referenced this issue Aug 13, 2021
- 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
smitpatel added a commit that referenced this issue Aug 16, 2021
- 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
smitpatel added a commit that referenced this issue Aug 16, 2021
- 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
@ghost ghost closed this as completed in #25495 Aug 17, 2021
ghost pushed a commit that referenced this issue Aug 17, 2021
- 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
@ajcvickers ajcvickers modified the milestones: 6.0.0, 6.0.0-rc1 Aug 17, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0-rc1, 6.0.0 Nov 8, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. ef6-parity type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.