Consider fit the rule of AnonymousTypes for EFCore2.1?

[yyjdelete]

EFCore2.1(in rc1 stage currently) add an new ability to do LINQ GroupBy translation, but only works with basic types, anonymous types and Tuple(with ctor).

But by default, System.Linq.Dynamic.Core use LinqProviderExtensions.IsLinqToObjects(IQueryable) to get createParameterCtor(which is false for EFCore), and there is no way to change that. And then the expression can not be recognized by EFCore as AnonymousTypes, and will be evaluated locally.

ctx.Set<Xxx>.GroupBy("new(Id)").Select("new(Key.Id as X, it.Sum(Id) as Y)").ToDynamicList();

warn: Microsoft.EntityFrameworkCore.Query[]
      The LINQ expression 'GroupBy(new {GroupId = [<generated>_2].Id}, [<generated>_2])' could not be translated and will be evaluated locally.
warn: Microsoft.EntityFrameworkCore.Query[]
      The LINQ expression 'Sum()' could not be translated and will be evaluated locally.

vs

public class X2
{
    public X2() { }
//remove the below ctor to see the difference
    public X2(int id)
    {
        Id = id;
    }
    public int Id { get; set; }
}
var exp = DynamicExpressionParser.ParseLambda<Group, X2>(null, true, "new (Id)");
ctx.Set<Xxx>.GroupBy(exp).Select("new(Key.Id as X, it.Sum(Id) as Y)").ToDynamicList();

[StefH]

So what you say is that is that in case of doing a GroupBy with EFCore2.1 rc1, the helper method LinqProviderExtensions.IsLinqToObjects(...) should actually return true instead of false ?

[yyjdelete]

Yes, createParameterCtor should be true, which is set with IsLinqToObjects. And maybe also for some other methods like Select.

[yyjdelete]

See https://github.com/yyjdelete/DynamicLinqWithEfCoreTest/tree/test165

[yyjdelete]

I'm not sure if it's(Only set `createParameterCtor` to true with Linq2Obj) still needed after #117(aebdd1b), now the same Expression as native AnonymousTypes does with `createParameterCtor` = true.

[byrnedo]

Hi, has there been any movement on this? I'm seeing that groupBy isn't getting translated on ef core 2.1

[StefH]

@byrnedo It's not translated at all ? Or just not optimized as the first post from this issue describes?

@yyjdelete I'll check the code to see if I can detect EFCore2.1

[byrnedo]

Sorry, meant that it says "could not be translated and will be evaluated locally".

The LINQ expression 'GroupBy(new {CustNo = [x].CustNo, Name = [cust].Name, Month = [y].Month, Flag = [x].Flag}, new JoinType() {x = [x], customer = [cust], y= [y]})' could not be translated and will be evaluated locally.

This was from the following group by string:

new (it.x.CustNo,it.customer.Name,it.y.Month,it.x.Flag)

I also happen to be doing a 'Sum' as well and that also gives the same warning.
@StefH

[yyjdelete]

@StefH
I think createParameterCtor can be true after #117(aebdd1b) for most QueryProvider which has natively support for AnonymousTypes, since now the same Expression as native AnonymousTypes does, but not sure if it will break some special provider that not support native AnonymousTypes.

[byrnedo]

@StefH Do you have an idea of if or when optimisation of group by will be working? I'm evaluating alternatives since it's pretty important that it happens on the database side.

[StefH]

I'm able to reproduce this issue with a sample console-app and I'll investigate.

[StefH]

@byrnedo There is not really an easy way of detecting if EFCore 2.1 is used, so for now I'll just update the ParsingConfig with a new property:

public bool EvaluateGroupByAtDatabase { get; set; }

See linked checkin / branch for more details.

[yyjdelete]

@StefH

1. Also consider make ParsingConfig.Default public for easy config?
2. It also needed for Select(not the lastest and see the difference)/SelectMany/GroupJoin/Join/Where and some others
   Maybe all method with selector or keySelector or predicate, not sure for some method like FirstOrDefault in an subquery.

//the the difference with sql
                var key = 1;
                var zz1 = ctx.Cars.Where(x => new { Id = x.Key } == new { Id = key }).ToList();
                var zz2 = ctx.Cars.Where(x => new XX1{ Id = x.Key } == new XX1 { Id = key }).ToList();

                var xx1 = ctx.Cars.Select(x => new { Id = x.Key }).GroupBy(x => x.Id).Select(x => new { Id = x.Key }).ToList();
                var xx2 = ctx.Cars.Select(x => new XX1 { Id = x.Key }).GroupBy(x => x.Id).Select(x => new { Id = x.Key }).ToList();

[StefH]

1. Good point. Do we want a Default and/or a DefaultEFCore21 static public ?
2. Correct, I realized this morning that I missed some code. But I need to double check if that this change will not break other code.

[StefH]

@yyjdelete Can you help me with point 1 ?

[StefH]

Issue 1 is updated.
Now working on 2 ...

[byrnedo]

@StefH I would love to help out if it would speed this issue along. Any pointers on what I could do which would be helpful?

[StefH]

Hello @byrnedo
Thanks for you help!

I did already start a new branch for this issue:
https://github.com/StefH/System.Linq.Dynamic.Core/tree/stef_AnonymousTypesforEFCore2.1
You can review this.

And maybe I can create a PR, and you can continue on that ? (I'm not 100% if that will work in github?)

[byrnedo]

@StefH Could I not fork and work on your branch and I can make a PR from that, and merge yours whenever/if you make changes?

[byrnedo]

Also, is it possible to run the tests on dotnetcore only?

[StefH]

Yes. You can use this branch.

Tests should run for multiple frameworks...

[byrnedo]

Got the tests to run.
I've looked through the changes and can't see anything that's a problem.
I think the EvaluateGroupByAtDatabase is fine as a way to toggle this.
Later you could make it default to true once it's been battle tested a bit.

I've been looking around to see if there's anything you missed but honestly I can't see anything.

[byrnedo]

@StefH could you go ahead and merge this?

[StefH]

@byrnedo I'll double check the code now, and maybe implement this switch also in all Select methods.

[StefH]

Code will be merged. And new NuGet shortly...

[byrnedo]

Thank you @StefH !!
🎆

[byrnedo]

@StefH : Just had a look at this running in production:

The LINQ expression '
GroupBy(new <>f__AnonymousType4`7(
    CustNo = [cust].CustNo, 
    Name = [cust].Name, 
    Year = [booking].Year, 
    Month = [booking].Month 
), new DomainEntity() { 
    customer = [cust], 
    booking = [booking]
    }
)' could not be translated and will be evaluated locally.

The line that does the group by looks as follows:

.GroupBy(parserConfig, $"new (it.cust.CustNo, it.cust.Name, it.booking.Year, it.booking.Month)", "it")

ParserConfig has the GroupByAtDatabase set to true.

[StefH]

Hello @byrnedo,

Can you also reproduce this same issue in this example app ?
See https://github.com/StefH/System.Linq.Dynamic.Core/blob/master/src-console/ConsoleAppEF2.1.1/Program.cs

[byrnedo]

Hey @StefH !

Yeah, just managed to recreate it now.

I added a Brands table (with 2 cols, BrandName and Rating :p ) in order to do a join.

It seems to happen if I do a join and project the result like this:

context.Cars.Join(
    context.Brands, 
    c => c.Brand, 
    b => b.BrandName,
  (c,b) => new {Brand = b, Car = c})
.GroupBy(...

It works if the projection was like this:

(c,b) => new {Brand = b.BrandName, Rating = b.Rating, Color = c.Color, Vin=c.Vin})

Is this something that a custom type provider would solve?
I'm not quite sure how that works...

EDIT:

Also doesn't translate if I project to a user defined class

c,b) => new MyGroup{Brand = b.BrandName, Rating = b.Rating, Color = c.Color, Vin=c.Vin}

EDIT 2:

I guess this mightn't have anything to do with this library and more to do with what efcore supports?

[byrnedo]

I've opened an issue with efcore as well: dotnet/efcore#13077

[yyjdelete]

For this, EvaluateGroupByAtDatabase is still missing from Join/GroupJoin

@byrnedo
I remember I had seen an comments in efcore's issue before I fire this issue, which say the current behavior to not mapping not-anonymous object is by design, but I forget the url.

In efcore, any expression maybe evaluated locally, and for compatibility, efcore keeps most behavior as clr does. But in clr, you can write an class like the below(and new object().Equals(new object()) == false without override Equals), and may lead to inconsistent behavior. But it's mostly impossible to check the behavior, so efcore chose to only mapping anonymous objects(Expression.New with members instead of Expression.MemberInit used by normal objects) and Tuple(which has an default comparator and consistent getter) to database.

        public class Xx
        {
            public int Yy
            {
                get => 1;//<-Not return the same with setter
                set { }
            }

            public override bool Equals(object obj)
            {
                return false;//<-No Equals or Equals not compare the properties
            }

            public override int GetHashCode()
            {
                return -1;
            }
        }

[StefH]

The EvaluateGroupByAtDatabase is indeeed missing from Join / GroupJoin. I'll need to add that for your test to work correct.

[byrnedo]

Ok so as I've understood, no fix in this project will get around my use case and the limitation is by design in efcore?

Edit: @yyjdelete was this the issue you were talking about?

[StefH]

Also commented on #199

@byrnedo and @yyjdelete

With code fixed (see linked example here https://github.com/StefH/System.Linq.Dynamic.Core/blob/EvaluateGroupByAtDatabase/src-console/ConsoleAppEF2.1/Program.cs), the sql statement is correct:

SELECT [c].[Key], [c].[Brand], [c].[Color], [c].[Vin], [c].[Year], [inner].[BrandType], [inner].[BrandName]
FROM [Cars] AS [c]
INNER JOIN [Brands] AS [inner] ON [c].[Brand] = [inner].[BrandType]
ORDER BY [c].[Year]

[byrnedo]

@StefH I wasn't very clear in the recreation post above. The code I'm running locally was translating the Join before, just not the subsequent GroupBy expression.

[StefH]

@byrnedo Maybe it's a best idea to just add the The EvaluateGroupByAtDatabase to all calls? In that way we are safe?

[byrnedo]

I'd say it might be ok without. This case of mine is by design as @yyjdelete suspected and confirmed now in dotnet/efcore#13077, so really I think this issue could be closed again.

[StefH]

Closing this one.
Continuing in #199