-
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
Relational: Support translating GroupBy() to SQL #2341
Comments
I have these classes public class Blog
{
public int BlogId { get; set; }
public string Url { get; set; }
public List<Post> Posts { get; set; }
}
public class Post
{
public int PostId { get; set; }
public string Title { get; set; }
public string Content { get; set; }
public int BlogId { get; set; }
public Blog Blog { get; set; }
} If I use this code var query1 = from p in context.Posts
where p.BlogId == 1
group p by p.Title into g
select new
{
Title = g.Key,
Count = g.Count()
}; EF7 generates this SQL SELECT [p].[PostId], [p].[BlogId], [p].[Content], [p].[Title]
FROM [Post] AS [p]
WHERE [p].[BlogId] = 1 without grouping and projection and groups data on the client side If I use Group by with Inner Join with this code var query = from p in context.Posts
join b in context.Blogs on p.BlogId equals b.BlogId
where p.BlogId == 1
group p by p.Title into g
select new
{
Title = g.Key,
Count = g.Count()
}; I get this error
EF 7.0.0-beta6-13735 |
Consider these cases when translating SQL |
Just another test case:
Throws the following exception:
This is for a recursive tree-like structure. Here I'm just trying to count children. |
@anpete should this go to backlog for now? |
@rowanmiller Sounds good. |
Why it's go to backlog?! |
View/ Indexed view? |
Or a raw SQL query. It's not that we aren't going to support it, just that we don't have time to get this translating (and stabilize it) before the 7.0.0 release. This one will be one of the first things we tackle after that. |
What would be the recommended approach using SQL (FromSql)? |
@rowanmiller I was hoping this was a 1.1 milestone, but I don't see this in 1.2 either. Any idea when this will get pulled off the backlog? I know we have work-arounds in the mean time. |
@chris-oswald not yet, we are planning for the next release this week. |
It's good to at least see it is listed as item #2 in the Critical O/RM features section. Even if all situations cannot be accounted for - I hope we are going to at least get the simplest cases covered. And to be fair @chris-oswald there is literally nothing in the current 1.2 feature list - you scared me for a moment! |
Heads up: I have made a lot of headway in this area. Expect a PR soon 😸 |
This seems still not suppprted... I'm currently working on a project and facing the performance issue as it doesn't translate to select count(*), xxxx group by xxxx , it actually fetches all the data from DB and does group by locally which causes the performance issue, coz I have got a million records... var counts = (from t in _dbContext.VoteTickets |
I think you misunderstood me. I was advocating that you define the behavior
via unit test, and explained that many tools expect this behavior, and by
not supporting it, upgrading EF6 to EFCore will bring unexpected surprises.
…On Thu, Oct 12, 2017, 6:07 PM Smit Patel ***@***.***> wrote:
@jzabroski <https://github.com/jzabroski> - At present grouping on
constant does not throw. It would evaluate it on client. Change it to throw
an exception is breaking change.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2341 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAbT_baFfDgkb25tNZXnm1L3dQL5A73Gks5sro2ygaJpZM4E6HAK>
.
|
|
@mchenx I'm affraid you won't get any joy with EF Core anytime soon. I recommend doing the simple stuff in EF Core and anything complicated in dapper or similar. It really is WAY easier than even full EF. Yes there's hard coded strings, but with string interpolation, nameof and a data layer, you can really make it robust. |
@Ben-Pattinson, maybe you will be interested in this project I worked on EFSqlTranslator. It translates EFCore lambda into string and executes it on Dapper. |
@ethanli83, very nice! I must say, initially I was rather sceptical, having been badly burnt by EF's horrific SQL generation. Assuming you haven't cherry picked your examples of SQL generation, they are nice and readable, and moreover, sane. Very un-EF like. You'll get into trouble for stuff like that, the Union will be along in a moment to give you a good talking to :) Comprehensive examples too, keep up the good work! |
Thank you @Ben-Pattinson, I will take a look at Union!~ |
@Ben-Pattinson SQL generation is something we have tried really hard to improve in EF Core. Do you have some examples of where you feel we can improve? |
@anpete (sorry for the rambling reply) I appreciate the problems you have when you need to support the same linq syntax for many different types of query, but when personal projects are running rings around your official data layer implementation, something isn't right. Personally, all I ever wanted was to write sql in my data layer and have it compile time validated against my data layer. It's so much easier and flexible than translating between unrelated syntaxes. I've got that now with dapper, nameof, string interpolation and t4 templates. It would have been nice to get slicker intellisense support, but it's pretty usable. |
@anpete I am not @Ben-Pattinson but if you could describe what you do
differently in generating SQL, I am a rare combination of SQL Server expert
(going back to 6.5 days of Ron Soukup) and C# engineer who has built web
apps, desktop apps, as well as a proprietary ORM-like data layer.
Years ago, I explained to Diego that EF6 and lower was fundamentally flawed
in how it generated queries, and we (the company I was working for) even
logged a bug and workaround for the most fundamental flaw in the query
generator: the dreaded duplicate left join bug where the "forest" of
possible left join possibilities does not reduce to a single, unique tree.
Cheers,
John
…On Mon, Feb 26, 2018, 7:55 PM Andrew Peters ***@***.***> wrote:
@Ben-Pattinson <https://github.com/ben-pattinson> SQL generation is
something we have tried really hard to improve in EF Core. Do you have some
examples of where you feel we can improve?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2341 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAbT_dlkAEnarCjYtZo70QO0s2HNroufks5tY1KFgaJpZM4E6HAK>
.
|
Smit Patel , your hardline stance indicates a severe code smell. You are
stating a policy, whereas I'm talking about the basic mechanism. If you're
hardwiring sequences of collaborations between your Domain Object Hydration
Engine/Object Materializer and your Expression Transformation (Query
Transpiler) Engine, then you are likely going to regret it and never even
realize somebody told you you were coding it wrong on February 27th, 2018.
Decouple, decouple, decouple. Let your customers choose.
…On Tue, Feb 20, 2018, 6:45 PM Smit Patel ***@***.***> wrote:
upgrading EF6 to EFCore will bring unexpected surprises.
EF6 to EF Core is not in-place upgrade. EF Core is rewrite of EF6 and in
many cases have behavioral differences. Any application expecting the group
by constant to work same as EF6 is gonna fail regardless of it client eval
or throw. By doing client eval, there is higher probability of external
tools may just work.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2341 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAbT_bhKWA6HTDS4xfFb0VLWGRTm-NUkks5tW1kLgaJpZM4E6HAK>
.
|
@Ben-Pattinson Thanks for the feedback and glad to hear that you have come up with a solution that works for you. A couple of responses to the points you make: LINQ GroupBy is very different from SQL GROUP BY and so we need client eval here for many cases anyway. It was a shame we weren't able to get to GroupBy with aggregates translation until 2.1, but it is almost here. One reason for this prioritization was that we have the |
@anpete It's really great to finally see this working :-) But oddly enough I still receive the error message telling me it cannot translate to GROUP BY, even though the generated SQL right underneath most certainly uses GROUP BY! I assume this must be a known issue?
This is from this source C#
It returns instantly and definitely works as expected so I'm happy for now - just wanted to point this out. |
- Add support for translating OrderBy after GroupBy operator - Add support for `HAVING` clause in SQL which would be generated when translating predicate after GroupByAggregate Resolves #10870 - Make sure client eval warning is not issued when translating GroupByAggregate Resolves #11157 - GroupBy Aggregate works when element/result selector is DTO instead of anonymous type Resolves #11176 (KeySelector has to be client evaluated) - Make sure that SQL added to GROUP BY clause is not aliased Resolves #11218 - Translate GroupBy Constant/Parameter with aggregates Resolves #9969 Part of #10012 Part of #2341
- Add support for translating OrderBy after GroupBy operator - Add support for `HAVING` clause in SQL which would be generated when translating predicate after GroupByAggregate Resolves #10870 - Make sure client eval warning is not issued when translating GroupByAggregate Resolves #11157 - GroupBy Aggregate works when element/result selector is DTO instead of anonymous type Resolves #11176 (KeySelector has to be client evaluated) - Make sure that SQL added to GROUP BY clause is not aliased Resolves #11218 - Translate GroupBy Constant/Parameter with aggregates Resolves #9969 Part of #10012 Part of #2341
- Add support for translating OrderBy after GroupBy operator - Add support for `HAVING` clause in SQL which would be generated when translating predicate after GroupByAggregate Resolves #10870 - Make sure client eval warning is not issued when translating GroupByAggregate Resolves #11157 - GroupBy Aggregate works when element/result selector is DTO instead of anonymous type Resolves #11176 (KeySelector has to be client evaluated) - Make sure that SQL added to GROUP BY clause is not aliased Resolves #11218 - Translate GroupBy Constant/Parameter with aggregates Resolves #9969 Part of #10012 Part of #2341
For preview2 version, there are more patterns now being translated to server (including group by constant). I have updated first post to capture the details. |
Please let me know if I can solve the following in 2.1.1: I need to take the latest item from each group, but I receive warnings saying the query cannot be translated. Is there a workaround and is it going to be possible in the foreseeable future? Example:
|
Updated as of 3/14/2018 based on implementation introduced
LINQ's
GroupBy()
operators can sometimes be translated to SQL'sGROUP BY
clauses, in particular when aggregate functions are applied in the projection.Scope for 2.1 release
Our current intention is for the scope of the work in 2.1 to improve how LINQ's
GroupBy
is evaluated in this particular scenario:Grouping on a simple expression that references a single column or multiple columns (using an anonymous type) of the query root and then projecting any aggregate operation from (Count, Sum, Average, Min, Max) and/or any individual property that is part of the grouping key (this will be translated to GROUP BY in SQL)
What is supported in 2.1.0-preview2
Apart from what is supported in 2.1.0-preview1 (details below), we have also added some more patterns
Examples
And a few bugfixes - #11218 #11157 #11176
What is supported in 2.1.0-preview1
Scenarios that we are not planning to improve in the 2.1 release
1. Grouping on constants(available in 2.1.0-preview2)2. Grouping on an entity (e.g. a reference navigation property)
3. Projecting non-aggregate scalar subqueries after grouping, e.g.
FirstOrDefault()
4. Making groups of multiple entityTypes using anonymous types.
5. Using Key/Aggregate values after GroupBy in joins (#10012)
All the scenarios above present different variations depending on what happens after the
GroupBy
, e.g. is there an aggregate function or not, is the key mentioned in the projection or not, etc. These scenarios will still result in client evaluation.We would appreciate if customers that care about EF Core supporting any of those scenarios that are scoped out from 2.1 to create individual issues for them, up-vote them, and keep the discussion there.
The text was updated successfully, but these errors were encountered: