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: Translate LINQ GroupBy to SQL GROUP BY where possible #12476

Closed
MarS0K opened this issue Jun 27, 2018 · 5 comments
Closed

Query: Translate LINQ GroupBy to SQL GROUP BY where possible #12476

MarS0K opened this issue Jun 27, 2018 · 5 comments
Assignees
Labels
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

@MarS0K
Copy link

MarS0K commented Jun 27, 2018

I want to group some entities by uppercase string property. But:

The LINQ expression 'GroupBy([artist].Name.ToUpper(), [artist])' could not be translated and will be evaluated locally.

Query:

using (var musicContext = new MusicContext())
{
    var artistNames = musicContext.Artists
        .GroupBy(artist => artist.Name.ToUpper())
        .Select(group => group.Key)
        .ToList();
}

Am I wrong?

Steps to reproduce

Model:

public class Artist
{
    [Key]
    [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
    public int ID { get; set; }

    [Required]
    public string Name { get; set; }
}

public class MusicContext : DbContext
{
    public DbSet<Artist> Artists { get; set; }
}

Generated SQL (if needed):

SELECT "artist"."Name"
FROM "Artists" AS "artist"
ORDER BY upper("artist"."Name")

Expected SQL (something like that):

SELECT upper("artist"."Name") AS "Key"
FROM "Artists" AS "artist"
GROUP BY upper("artist"."Name")

Further technical details

EF Core version: Microsoft.EntityFrameworkCore 2.1.0
Database Provider: Microsoft.EntityFrameworkCore.Sqlite

@MarS0K MarS0K changed the title Group by uppercase string is evaluated locally Query: Grouping by uppercase string is evaluated locally Jun 28, 2018
@ajcvickers
Copy link
Member

@smitpatel to de-dupe.

@smitpatel
Copy link
Member

Easy work-around is to use anonymous type for key selector instead of scalar property like that.

@smitpatel
Copy link
Member

Currently this does not get translated because we don't process keys other than EF property methods. We can remove that restrictions.

@MarS0K
Copy link
Author

MarS0K commented Jul 7, 2018

@smitpatel Ok, I got it. Pretty nice workaround. Thank you.
It's pretty simple if you create key selector expression like that:

.GroupBy(artist => artist.Name.ToUpper())

But if:

.GroupBy(ProcessGroupKey(m_groupKeyToken.DuplicateGroupKey))
public interface IDuplicateGroupKeyToken<T>
{
    Expression<Func<T, string>> DuplicateGroupKey { get; }
}
public class ArtistDuplicateGroupKeyToken : IDuplicateGroupKeyToken<Artist>
{
    public Expression<Func<Artist, string>> DuplicateGroupKey
        => artist => artist.Name;
}

private readonly IDuplicateGroupKeyToken<Artist> m_groupKeyToken;

private Expression<Func<T, string>> ProcessGroupKey(Expression<Func<T, string>> expression)
    => ExtendExpression(ProcessedGroupKeyExpression, expression);

private Expression<Func<string, string>> ProcessedGroupKeyExpression
    => groupKey => groupKey.Trim();

public Expression<Func<TContainer, TResult>> ExtendExpression<TContainer, TItem, TResult>(
    Expression<Func<TItem, TResult>> getResultExpression,
    Expression<Func<TContainer, TItem>> getItemExpression)
{
    var invokeExpression = Expression.Invoke(
        getResultExpression, // groupKey => groupKey.Trim();
        getItemExpression.Body // artist => artist.Name;
    );

    return Expression.Lambda<Func<TContainer, TResult>>(
        body: invokeExpression, // artist => artist.Name.Trim();
        parameters: getItemExpression.Parameters
    );
}

It's much harder.

@smitpatel
Copy link
Member

@MarS0K - That is true hence issue is still open. This is some enhancement which we would make in future release. I gave work-around till this is fixed in public release.

@divega divega added this to the 2.2.0 milestone Jul 11, 2018
@ajcvickers ajcvickers modified the milestones: 2.2.0, 3.0.0 Aug 3, 2018
@ajcvickers ajcvickers assigned smitpatel and unassigned maumar Jan 28, 2019
@ajcvickers ajcvickers changed the title Query: Grouping by uppercase string is evaluated locally Query: Translate LINQ GroupBy to SQL GROUP BY where possible Jun 28, 2019
@smitpatel smitpatel added the verify-fixed This issue is likely fixed in new query pipeline. label Jul 2, 2019
smitpatel added a commit that referenced this issue Jul 2, 2019
…fter GroupBy

Resolves #12826
Resolves #6658
Part of #15711
Resolves #15853
Resolves #12799
Resolves #12476
Resolves #11976

There are way too many existing issues are resolved by this PR. I haven't added regression test or verified each of them so I have put Verify-Fixed label on them for now.
smitpatel added a commit that referenced this issue Jul 2, 2019
…fter GroupBy

Resolves #12826
Resolves #6658
Part of #15711
Resolves #15853
Resolves #12799
Resolves #12476
Resolves #11976

There are way too many existing issues are resolved by this PR. I haven't added regression test or verified each of them so I have put Verify-Fixed label on them for now.
smitpatel added a commit that referenced this issue Jul 2, 2019
…fter GroupBy

Resolves #12826
Resolves #6658
Part of #15711
Resolves #15853
Resolves #12799
Resolves #12476
Resolves #11976

There are way too many existing issues are resolved by this PR. I haven't added regression test or verified each of them so I have put Verify-Fixed label on them for now.
@smitpatel smitpatel removed this from the 3.0.0 milestone Jul 2, 2019
@smitpatel smitpatel added this to the 3.0.0-preview7 milestone Jul 2, 2019
@smitpatel smitpatel added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed verify-fixed This issue is likely fixed in new query pipeline. labels Jul 2, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0-preview7, 3.0.0 Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

No branches or pull requests

5 participants