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

Query: GroupBy Where Aggregate translation to server #11711

Closed
smitpatel opened this issue Apr 17, 2018 · 5 comments · Fixed by #21907
Closed

Query: GroupBy Where Aggregate translation to server #11711

smitpatel opened this issue Apr 17, 2018 · 5 comments · Fixed by #21907
Assignees
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Milestone

Comments

@smitpatel
Copy link
Member

smitpatel commented Apr 17, 2018

Currently if there is predicate after grouping and before applying Aggregate operation we block the translation to server and do streaming GroupBy. Aggregate operator argument can take Case Block to do conditional aggregate and allow us to generate Group By clause in SQL.
Structure would be

COUNT(
    CASE 
        WHEN predicate 
        THEN projection
        ELSE NULL
    END
)
@AlekseyMartynov
Copy link

Hello @smitpatel
I recently stumbled upon this issue and found that there is a family of equivalent aggregates:

  1. .GroupBy(o => o.OrderDate)
    .Select(g => g.Count(i => i.OrderDate != null))
  2. .GroupBy(o => o.OrderDate)
    .Select(g => g.Where(i => i.OrderDate != null).Count())
  3. .GroupBy(o => o.OrderDate)
    .Select(g => g.Select(i => i.OrderDate != null ? 1 : 0).Sum())
  4. .GroupBy(o => o.OrderDate)
    .Select(g => g.Select(i => i.OrderDate != null ? (int?)1 : null).Sum())

(maybe more?)

Please support them all in the context of this feature.
This code from NHibernate seems relevant and useful.

@smitpatel
Copy link
Member Author

@AlekseyMartynov - Thanks for information. All of them are normalized into single form in our query parser so shouldn't be issue in supporting them all.

@sguryev
Copy link

sguryev commented Mar 11, 2019

Hi guys. What about the case with Ranking? Let's say I have a table with clients. And I need to group them by Country and select the top1 earning client in each country. So result should be:
Country-CustomerName-Earning

I use
GroupBy(c => c.Country).Select(cg => new {Country = cg.Key, cg.ThenByDescending(c => c.Earning).Name, Earning = cg.Max(t => t.Earning)})

This query will be evaluated locally. Do you have any advises to execute it on server? EF 6.2 can do it.

@mthalman
Copy link
Member

mthalman commented Nov 19, 2019

It's interesting to note that you can run into this issue just by using a navigation property in the GroupBy, seemingly because there will be an implicit predicate produced. A workaround in this case is to expose a foreign key property that can be grouped on instead of the navigation property.

Example:

await this.dbContext.Games
    .Select(g =>
        g.RoundScores
            .GroupBy(rs => rs.Player)
            .Select(o => o.Key)
            .Count()
    )
    .ToListAsync();
System.InvalidOperationException: 'The LINQ expression 'GroupBy<TransparentIdentifier<RoundScore, Player>, Player, RoundScore>(
    source: Join<RoundScore, Player, Nullable<Guid>, TransparentIdentifier<RoundScore, Player>>(
        outer: Where<RoundScore>(
            source: DbSet<RoundScore>, 
            predicate: (p2) => Property<Nullable<Guid>>(EntityShaperExpression: 
                EntityType: Game
                ValueBufferExpression: 
                    ProjectionBindingExpression: EmptyProjectionMember
                IsNullable: False
            , "Id") == Property<Nullable<Guid>>(p2, "GameId")), 
        inner: DbSet<Player>, 
        outerKeySelector: (p2) => Property<Nullable<Guid>>(p2, "PlayerId"), 
        innerKeySelector: (p3) => Property<Nullable<Guid>>(p3, "Id"), 
        resultSelector: (p2, p3) => new TransparentIdentifier<RoundScore, Player>(
            Outer = p2, 
            Inner = p3
        )), 
    keySelector: (ti) => ti.Inner, 
    elementSelector: (ti) => ti.Outer)' could not be translated. Either rewrite the query in a form that can be translated, or switch to client evaluation explicitly by inserting a call to either AsEnumerable(), AsAsyncEnumerable(), ToList(), or ToListAsync(). See https://go.microsoft.com/fwlink/?linkid=2101038 for more information.'

@smitpatel
Copy link
Member Author

@mthalman - That is incorrect assumption. GroupBy entityType is being tracked at #17653 You can look up into that issue discussion why GroupBy entity type is not straight fwd SQL GROUP BY.
The query you posted has Where before calling GroupBy so it has no relevance with this issue. If you group by property on navigation you are using (so group by scalar rather than entity type), the tree will be similar to yours with expression tree but it still translates to server.

This issue tracks about using Where on IGrouping parameter which is generated after applying GroupBy before applying aggregate operator.

smitpatel added a commit that referenced this issue Aug 3, 2020
smitpatel added a commit that referenced this issue Aug 3, 2020
@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 3, 2020
smitpatel added a commit that referenced this issue Aug 3, 2020
@smitpatel smitpatel modified the milestones: 5.0.0, 5.0.0-rc1 Aug 3, 2020
smitpatel added a commit that referenced this issue Aug 3, 2020
@ghost ghost closed this as completed in #21907 Aug 3, 2020
ghost pushed a commit that referenced this issue Aug 3, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0-rc1, 5.0.0 Nov 7, 2020
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. customer-reported type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants