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: Expand navigations together in OrderBy/ThenBy #18913

Merged
merged 1 commit into from
Nov 16, 2019
Merged

Conversation

smitpatel
Copy link
Contributor

Fixes #18904

@smitpatel smitpatel merged commit e4a0e41 into master Nov 16, 2019
@smitpatel smitpatel deleted the smit/issue18904 branch November 16, 2019 00:50
@psryan
Copy link

psryan commented Dec 18, 2019

I've hit a similar sounding issue to this in EF Core 3.1 after upgrading from Core 2.2. I've tried to test my repro case attached against the latest daily build unsuccessfully. Although this could be from my lack of experience with using daily builds. I still get the same issue outcome.

The issue I face is when using OrderBy with an aggregate (e.g. Max, Count) followed by ThenBy with a related entity property, I get "System.InvalidOperationException ... EF.Property called with wrong property name". I use dynamic ordering via System.Linq.Dynamic.Core, but have reproduced the issue using direct OrderBy().ThenBy() in the attached repro case.

             IList<Deal> Deals = ### context.Deal
               .Include(x => x.Customer)
               .Select(x => new Deal {
                  Id = x.Id,
                  SubmitDate = x.SubmitDate,
                  Customer = x.Customer,
                  MaxInstallDate = x.DealEquip.Max(de => de.InstallDate)
               })
               .OrderBy(x => x.MaxInstallDate).ThenBy(x => x.Customer.CustomerName)           // Having aggregate Field/Property first appears to cause issue when others added after it
               //.OrderBy("MaxInstallDate ASC, Customer.CustomerName ASC")                    // <<-- Dynamic OrderBy also encounters the same issue
               .ToList();

Can you please test this repro case attached is no longer an issue with this fix otherwise it may still be an existing problem.

CrashRepro OrderBy(Max)ThenBy(Related).zip

@smitpatel
Copy link
Contributor Author

@psryan - Please file a new issue with detailed repro code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Complex OrderBy+ThenBy throws InvalidOperationException
3 participants