-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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 with GroupBy or GroupJoin throws exception #17068
Comments
#12560 (comment) is a good example of why it's dangerous to implement implicit client evaluation of GroupBy. Requiring users to do explicit client evaluation with AsEnumerable forces them to think and understand the distinctions and perf impact, etc. Reiterating previous thoughts, the more I think of it, the more I think we should aim to avoid implicit client evaluation for any scenario where client evaluation is (significantly) more expensive than server evaluation, when possible. In retrospect it actually seems like quite a mistake for LINQ to have GroupBy returning groupings rather than a reduced/aggregated scalar, given that it tends to follow SQL in operator naming and the general prevalence of SQL. |
Linking all other bugs from backlog which requires client side GroupBy. If we decide to fix this, then please consider all the scenarios in linked closed bugs. |
If we decide to close this, also close related issues which are blocked on this one. And also clean up tests in codebase. |
Note from team meeting: @smitpatel will discuss with team in a design meeting. |
We discussed this issue in a team meeting in detailed and I have read all above comments thoroughly. Following are our conclusions
GroupJoin operator
GroupJoin <-> GroupBy equivalence AsAsyncEnumerable usage WithClientEvaluation operator
With all of above, I believe that it addresses all the points raised in discussion here. |
Closing this issue as non-needed since sub-issues are created to track individual translation patterns. |
i'm arriving late to the party i guess but the loss of server side evaluation from entity framework on groupby is disgusting and gamebreaking. something as simple as this does NOT work. whoever is sleeping over there on the ef core team needs to wake up and start doing their jobs. MyTable
.GroupBy(x => x.Id)
.OrderBy(x => x.Key) InvalidOperationException: The LINQ expression 'DbSet<MyTable>()
.GroupBy(x => x.Id)
.OrderBy(x => x.Key)' 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 'AsEnumerable', 'AsAsyncEnumerable', 'ToList', or 'ToListAsync'. seriously..................... it's 20 stinking 24 and STILL no resolution to these issues that have been brought up since 2017ish. c'mon man. |
@pvg8v6g we've done quite a lot of work on closing the gap on GroupBy functionality in recent releases, but there are indeed some problematic corners remaining. This specific querying pattern - ordering after a final GroupBy (without an aggregate function) - is something that IIRC hasn't actually been requested by anyone, which is one reason why it is unimplemented; it also was never server-evaluated, GroupBy without an aggregate function isn't supported by SQL. As a workaround, the translation for final GroupBy without aggregate function already contains an ORDER BY on the key: _ = await context.Blogs.GroupBy(b => b.Name).ToListAsync(); SELECT [b].[Name], [b].[Id], [b].[DateTime]
FROM [Blogs] AS [b]
ORDER BY [b].[Name] So just doing GroupBy without the OrderBy will happen to do what you're looking for. Otherwise, I'd recommend simply explicitly switching to client evaluation: _ = context.Blogs
.AsEnumerable()
.GroupBy(b => b.Name)
.OrderBy(b => b.Key)
.ToList(); |
This is not an answer. I chose a VERY simple query to show things are still broken not because that’s what I was trying to do. I am doing VERY complicated queries on a daily basis and do NOT want to bring in tons and tons of data to memory to do a group by. Grouping is an SQL feature and should be translatable server side. These things used to be possible back in entity framework 6 and that functionality was lost moving to ef core. I should be able to do groupby ANYWHERE in the query with any number of statements after and it be translated to SQL. Bringing them into memory is the opposite of what we need. That’s the whole point of groupby on iqueryable… why not just remove the function off of that class and just leave it on ienumerable if you’re not going to support the functionality…. I’m only upset because we LOST functionality and we would love for it to be returned. |
@pvg8v6g you seem to have some incorrect assumptions there - I'd very much recommend looking at the actual SQL that the old EF6 generated here. This is the LINQ query you want EF translate: MyTable
.GroupBy(x => x.Id)
.OrderBy(x => x.Key) This query by design brings the entire table data to the client - that's simply what it's supposed to do; there's no filtering here (no Where), no aggregation (Sum, Average...) and no projection to prevent all columns from being fetched - you're querying all rows and all columns from the database, and just using LINQ to reorganize the data in groups. So you'd be "bringing in tons and tons of data to memory" no matter what. Further, LINQ GroupBy cannot be translated to SQL GROUP BY unless you use Select() to apply an aggregate function: _ = await context.Blogs
.GroupBy(b => b.Category)
.Select(g => new { g.Key, MaxId = g.Max(b => b.Id) })
.ToListAsync(); This can translate to SQL GROUP BY because of the aggregate function (Max); but without the Select() it cannot. EF does support queries where the last operator is a GroupBy, but these don't translate to SQL GROUP BY (not possible) - an ordering is applied (as I wrote above) and the grouping is performed client-side. Again, this doesn't imply any more data being transfered to the client - it merely performs the grouping client-side rather than server-side. To summarize, for your original query, you can place an AsEnumerable() before the OrderBy - or before the GroupBy - without impacting query performance in a meaningful way. |
If you run into issue with GroupBy/GroupJoin and arrive here, then please take time to read #17068 (comment) fully to direct your feedback in specific issue for your specific scenario.
Read our documentation about this here
Group By
Linq
GroupBy
operator allows to createIGrouping<TKey, TElement>
whereTKey
&TElement
could be arbitrary types. Further,IGrouping
implementsIEnumerable<TElement>
which means you can compose over using any queryable operator after grouping.In contrast SQL
GROUP BY
keyword is very restrictive. In SQL, you can only use scalar values inGROUP BY
. Most database also allows only referencing grouping key columns or aggregate applied over any other column. Hence selecting a column which is not in key or does not have aggregate applied is not possible.Due to this mismatch, not all linq
GroupBy
can be translated to SQLGROUP BY
. In EF Core 3.0 we have support for SQLGROUP BY
but not for all linqGroupBy
. There are few bugs/limitation in this translation for which we have various issues filed, mainly for scenarios where Distinct or a predicate is applied before aggregate operation.Another GroupBy scenario which can be translated to server is selecting first element of each group which is being tracked by #13805
Most other linq
GroupBy
cannot be evaluated on server and must be evaluated on client side. This issue tracks about allowing EF Core to do this client side grouping implicitly. Absence of this feature means users would have to manually callAsEnumerable
before GroupBy to explicitly falling into client side grouping.GroupJoin
Linq
GroupJoin
is a queryable operator which does not have equivalent SQL keyword. The biggest use of GroupJoin for an ORM is to generate Left join on server side as mentioned here.Customers.GroupJoin(Orders,...)
would createIEnumerable<Order>
for eachCustomer
. As with GroupBy, you can compose over this in Linq but there is no translation. Again, GroupJoin requires client side grouping to generate correct result. This issue also tracks adding implicit support for it.In recent discussion it came up that, allowing this may be pit of failure (with escape hatch at the bottom
).
The text was updated successfully, but these errors were encountered: