Building Filter and/or OrderBy via Expression on Intermediate type throws InvalidOpException

[johnhmuller]

I need to build dynamic OrderBy's:

IQueryable<T> query = …
var parameter = Expression.Parameter(query.ElementType, "p");
var property = Expression.Property(parameter, "NameOfProperty");
var getProperty = Expression.Lambda(property, new [] { parameter });

var expression = Expression.Call(typeof(Queryable), nameof(Queryable.OrderBy), 
new [] { query.ElementType, typeof(TKey) }, 
new [] { query.Expression, Expression.Quote(getProperty) });

query = query.Provider.CreateQuery<T>(expression);

This obviously builds:

query = query.OrderBy(p => p.NameOfProperty);

The query above is the result of some Expression<Func<TEntity, T>> based projection (from entity type TEntity to T) where T is an intermediate type with public constructors and properties (I try to keep the anonymous type count down). On retrieving the results I get the exception attached - there is also a .Distinct - but I am 100% certain that the problem is with the OrderBy, as removing the OrderBy-building produces results, as well as using the simple OrderBy directly.

The very same code works on EF 6.

Exception.txt

System.InvalidOperationException: The LINQ expression 'OrderBy<ProcessGroup, long>(
    source: Distinct<ProcessGroup>(Select<TransparentIdentifier<TransparentIdentifier<TransparentIdentifier<Task, Process>, ProcessGroup>, string>, ProcessGroup>(
        source: SelectMany<TransparentIdentifier<TransparentIdentifier<Task, Process>, ProcessGroup>, string, TransparentIdentifier<TransparentIdentifier<TransparentIdentifier<Task, Process>, ProcessGroup>, string>>(
            source: Join<TransparentIdentifier<Task, Process>, ProcessGroup, Nullable<long>, TransparentIdentifier<TransparentIdentifier<Task, Process>, ProcessGroup>>(
                outer: Where<TransparentIdentifier<Task, Process>>(
                    source: Where<TransparentIdentifier<Task, Process>>(
                        source: Join<Task, Process, Nullable<long>, TransparentIdentifier<Task, Process>>(
                            outer: DbSet<Task>, 
                            inner: DbSet<Process>, 
                            outerKeySelector: (t) => Property<Nullable<long>>(t, "ProcessId"), 
                            innerKeySelector: (p) => Property<Nullable<long>>(p, "Id"), 
                            resultSelector: (o, i) => new TransparentIdentifier<Task, Process>(
                                Outer = o, 
                                Inner = i
                            )), 
                        predicate: (t) => t.Inner.IsActive && Count<Field>(Where<Field>(
                            source: DbSet<Field>, 
                            predicate: (f) => Property<Nullable<long>>(t.Outer, "Id") == Property<Nullable<long>>(f, "TaskId"))) > 0), 
                    predicate: (t) => (int)t.Outer.ConstructorType == (int)(Unhandled parameter: __ctor_0) && Any<ParticipantMember>(
                        source: DbSet<ParticipantMember>, 
                        predicate: (p0) => (Nullable<long>)p0.ParticipantId == t.Outer.ConstructorId && p0.UserId == (Unhandled parameter: __8__locals1_userId_1))), 
                inner: DbSet<ProcessGroup>, 
                outerKeySelector: (t) => Property<Nullable<long>>(t.Inner, "ProcessGroupId"), 
                innerKeySelector: (p1) => Property<Nullable<long>>(p1, "Id"), 
                resultSelector: (o, i) => new TransparentIdentifier<TransparentIdentifier<Task, Process>, ProcessGroup>(
                    Outer = o, 
                    Inner = i
                )), 
            collectionSelector: (t) => Select<ProcessGroupResource, string>(
                source: Where<ProcessGroupResource>(
                    source: DbSet<ProcessGroupResource>, 
                    predicate: (p2) => p2.LanguageId == (Unhandled parameter: __languageId_2) && t.Inner.Id == p2.ProcessGroupId), 
                selector: (p2) => p2.Narrative), 
            resultSelector: (t, c) => new TransparentIdentifier<TransparentIdentifier<TransparentIdentifier<Task, Process>, ProcessGroup>, string>(
                Outer = t, 
                Inner = c
            )), 
        selector: (ti) => new ProcessGroup{ 
            Description = ti.Inner, 
            Id = ti.Outer.Inner.Id, 
            Name = ti.Outer.Inner.Name, 
            UniqueId = ti.Outer.Inner.UniqueId, 
            PropertiesValue = null 
        }
    )), 
    keySelector: (e) => e.Id)' 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 either AsEnumerable(), AsAsyncEnumerable(), ToList(), or ToListAsync(). See https://go.microsoft.com/fwlink/?linkid=2101038 for more information.

Further technical details

EF Core version: 3.0.0 RTM
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET Core 3.0
Operating system: Windows 10 Enterprise (1909)
IDE: Visual Studio 2019 16.3.10

[johnhmuller]

Similar exception is thrown on applying Expression<Func<T, bool>> based filters on the intermediate query versus applying the Where directly on the fields. I found the gem called ExpressionPrinter and used it to print both query,Expression for both OrderBy and Where and the output is exactly the same?

[ajcvickers]

@johnhmuller The issue here is likely that Expression.Property(parameter, "NameOfProperty"); does not result in the same PropertyInfo as is generated when the compiler. You should be able to fix this by creating the PropertyInfo in a different way--pinging @smitpatel to show a code snippet for this.

Generally we put more emphasis on translating expression trees generated by the compiler. However, in this case we will look at making EF Core understand this alternate expression tree since it seems to frequently catch people out in a non-obvious way.

Also filed dotnet/EntityFramework.Docs#1956

[johnhmuller]

Awesome, thanks for the reply. Awaiting code snippet from @smitpatel :)

[fschlaef]

Got the same issue, here is a full repro :

using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Debug;
using System;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;

namespace InheritedClassThrows
{
    public static class LinqExtensions
    {
        public static IOrderedQueryable<TSource> OrderByFilter<TSource>(this IQueryable<TSource> query, string sortField)
        {
            if (!string.IsNullOrEmpty(sortField))
            {
                return query.OrderBy(sortField);
            }

            return (IOrderedQueryable<TSource>)query;
        }

        public static IOrderedQueryable<TSource> OrderBy<TSource>(this IQueryable<TSource> query, string propertyName)
        {
            var entityType = typeof(TSource);

            // Create x=>x.PropName
            var propertyInfo = entityType.GetProperty(propertyName);

            // If we try to order by a property that does not exist in the object return the list
            if (propertyInfo == null)
            {
                return (IOrderedQueryable<TSource>)query;
            }

            var arg = Expression.Parameter(entityType, "x");
            var property = Expression.Property(arg, propertyName);
            var selector = Expression.Lambda(property, new ParameterExpression[] { arg });

            // Get System.Linq.Queryable.OrderBy() method.
            var method = typeof(Queryable).GetMethods()
                 .Where(m => m.Name == "OrderBy" && m.IsGenericMethodDefinition)
                 .Where(m => m.GetParameters().ToList().Count == 2) // ensure selecting the right overload  
                 .Single();

            //The linq's OrderBy<TSource, TKey> has two generic types, which provided here
            MethodInfo genericMethod = method.MakeGenericMethod(entityType, propertyInfo.PropertyType);

            /* Call query.OrderBy(selector), with query and selector: x=> x.PropName
              Note that we pass the selector as Expression to the method and we don't compile it.
              By doing so EF can extract "order by" columns and generate SQL for it. */
            return (IOrderedQueryable<TSource>)genericMethod.Invoke(genericMethod, new object[] { query, selector });
        }
    }

    class Program
    {
        static void Main(string[] args)
        {
            using var context = new InheritedClassThrowsContext();

            context.Database.EnsureDeleted();
            context.Database.EnsureCreated();

            context.Customer.Add(new Customer() { Name = "Customer1", CreatedDate = DateTime.UtcNow });
            context.SaveChanges();

            var ok = context.Customer.Select(c => new Customer()
                {
                    Id = c.Id,
                    CreatedDate = c.CreatedDate,
                    Name = c.Name
                })
                .OrderByFilter("CreatedDate")
                .ToList();

            var crash = context.Customer.Select(c => new Customer2()
                {
                    Id = c.Id,
                    CreatedDate = c.CreatedDate,
                    Name = c.Name,
                    SomeGeneratedValue = "Test"
                })
                .OrderByFilter("Name")
                .ToList();
        }
    }

    public partial class InheritedClassThrowsContext : DbContext
    {
        public virtual DbSet<Customer> Customer { get; set; }

        private static readonly LoggerFactory Logger = new LoggerFactory(new[] { new DebugLoggerProvider() });

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            string connectionString = "Server=.;Database=InheritedClassThrows;Trusted_Connection=True;MultipleActiveResultSets=true";

            optionsBuilder.UseSqlServer(connectionString)
                .EnableSensitiveDataLogging()
                .UseLoggerFactory(Logger);

        }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Customer>(entity =>
            {
                entity.Property(e => e.Id).HasColumnName("id");

                entity.Property(e => e.CreatedDate).HasColumnName("createdDate");

                entity.Property(e => e.Name)
                    .IsRequired()
                    .HasColumnName("name")
                    .HasMaxLength(50);
            });
        }
    }

    public partial class Customer
    {
        public int Id { get; set; }
        public string Name { get; set; }
        public DateTime CreatedDate { get; set; }
    }

    public class Customer2 : Customer
    {
        public string SomeGeneratedValue { get; set; }
    }
}

[smitpatel]

        public static IOrderedQueryable<TSource> OrderBy<TSource>(this IQueryable<TSource> query, string propertyName)
        {
            var entityType = typeof(TSource);

            // Create x=>x.PropName
            var propertyInfo = entityType.GetProperty(propertyName);
            if (propertyInfo.DeclaringType != entityType)
            {
                propertyInfo = propertyInfo.DeclaringType.GetProperty(propertyName);
            }

            // If we try to order by a property that does not exist in the object return the list
            if (propertyInfo == null)
            {
                return (IOrderedQueryable<TSource>)query;
            }

            var arg = Expression.Parameter(entityType, "x");
            var property = Expression.MakeMemberAccess(arg, propertyInfo);
            var selector = Expression.Lambda(property, new ParameterExpression[] { arg });

            // Get System.Linq.Queryable.OrderBy() method.
            var method = typeof(Queryable).GetMethods()
                 .Where(m => m.Name == "OrderBy" && m.IsGenericMethodDefinition)
                 .Where(m => m.GetParameters().ToList().Count == 2) // ensure selecting the right overload
                 .Single();

            //The linq's OrderBy<TSource, TKey> has two generic types, which provided here
            MethodInfo genericMethod = method.MakeGenericMethod(entityType, propertyInfo.PropertyType);

            /* Call query.OrderBy(selector), with query and selector: x=> x.PropName
              Note that we pass the selector as Expression to the method and we don't compile it.
              By doing so EF can extract "order by" columns and generate SQL for it. */
            return (IOrderedQueryable<TSource>)genericMethod.Invoke(genericMethod, new object[] { query, selector });
        }

Updated above method.
Mainly, get actual propertyInfo from the class declaring the member & use Expression.MakeMemberAccess

[johnhmuller]

Hi,

@fschlaef , thanks a million for very decent repro steps that perfectly captured my scenario (I would not have thought that the inheritance would be part of the cause of the issue).

@smitpatel, thanks a million for the code sample - works perfectly! @ajcvickers, thanks for setting me up with a solution:)

P.S. Is this an "issue" in EF Core 3 (as it was working on EF6) or with .NET Core 3 (as I did not have to worry about .DeclaringType's in the past)?

And what do I do with this logged issue - close (as resolved)?

[fschlaef]

@smitpatel Perfect fix ! Amazing 👍

@johnhmuller My pleasure 😄

[smitpatel]

@johnhmuller - It is not issue in .NET Core as it gave same results in past too. I haven't looked at how EF6 deals with it so cannot comment on that. But for previous versions of EF Core, it worked since we only looked at the name rather than actual MemberInfo (which differs based on declaring type). While that approach worked for this scenario, it matched a lot of incorrect ones too causing bugs hence we had to change it. It did not become in 3.x release since compiler did not generate such tree and it always comes from dynamically created expression trees.

This issue is open since we want to look into if we can support this.

[johnhmuller]

@smitpatel - if I am the only one to have this issue, then please rather leave it as is (there is a proper solution here), as no one else might have run into this / have a need for this. I have already updated my code based on your solution, this will no longer prevent me from continuing my conversion from Full Framework to Core. Thanks again...

[smitpatel]

@johnhmuller - We had 4-5 reports of this so far. And the fix turned out to be 1 line change only so I submitted PR. 😄

[ajcvickers]

@smitpatel to prepare 3.1.x PR.

[JohnGalt1717]

This is a vital blocking bug that effectively renders oData $filter useless. It needs to be more than considered for fixing in 3.1.3.

[smitpatel]

@JohnGalt1717 - Please explain to me what is more than "we are considering for fixing it in next patch release". Sadly, I don't own a time machine.

[JohnGalt1717]

This is what you wrote in my bug report (no time machine required, literally you just had to remember what you wrote 15 minutes prior):

"Duplicate of #19087
Being considered for patching in 3.1.x"

It needs to be patched for 3.1.3 because it kills existing oData functionality that worked in .NET Core 2.2 and blocks upgrades while introducing bugs into your customers' code without them knowing. It shouldn't be "being considered" it should be "done and awaiting release." because you have hundreds of thousands of customers trapped into a buggy EF Core 3.1 upgrade that they didn't want because you forced the EOL of 2.2 when EF Core 3.1 clearly isn't even remotely close to ready to replace it or EF Core 2.1 for that matter.

Hence now you should be committing to fixing ALL of the queries that ran server side in .NET Core 2.2 that are broken in EF Core 3.1 (about 25% in our experience, and only discover-able at run-time) and doing so immediately and iteratively over the next few months so that people don't get hung out to dry without security patches on .net core 2.2 as we and so many others are now. Is it optimal? No. Is it risky on your part? Absolutely! But your entire approach is defective so now you have to scramble to make this right because you made a decision to inject bugs into your user's code that they can't discover without happening to hit it and then force them to upgrade on an untenable time-table.

As an example in this case, it works fine 90% of the time, but fails in a common case. If you don't happen to have code coverage in this specific case because it works in all other cases, EF Core causes run-time crashes. And that's true all over the place with endless bugs. (Again about 25% of our Linq queries that run server side now fail.)

[smitpatel]

@JohnGalt1717 - I remember pretty well what I wrote 15 mins before. My statement exactly meant that we are considering it for patching. (Passive voice construct, english). There is already PR out to port the fix to 3.1 branch. When upper management approves it, we will merge and it will get released as per patch release schedule.

As for rest of your post which is contradicting itself. We have received feedback from customers using EF Core 3.1, many of which does not share your views on the state of EF Core 3.1. There are few paper cuts which we are trying to fix based on how much risky it is for applications which are working already on EF Core 3.1. This is what is our current procedure and we will work based on our roadmap for 5.0 timeframe.

If EF Core 3.1 is not working for you, feel free to use other product.

[JohnGalt1717]

@smitpatel (And no, I'm not at all contradicting myself. I dare you to point to anywhere that I have.)

[Pilchie]

@JohnGalt1717 I've hidden your comment as abusive. We're working to build an inclusive and welcoming community, and the way that you are communicating doesn't follow that.

[JohnGalt1717]

@Pilchie Why isn't @smitpatel getting hidden as abusive? He just suggested I flush 4 years of development down the toilet because of your team's choices. That's true abuse (and VERY CERTAINLY not a welcoming community)

[Pilchie]

@smitpatel has not mounted any personal attacks against individuals or teams that I'm aware of.

[JohnGalt1717]

@Pilchie He just did against myself by telling me to flush 4 years of work and sod off.

[alexmurari]

@smitpatel @ajcvickers Noob question here: If patch 3.1.3 hasn't been released yet (I think), why this bug can't make to this milestone instead of 3.1.4 (apparently the solution is a one-liner)? Once an release is finished it is protected and untouchable? Just trying to understand how the team works and makes these choices. Some flexibility to ship a fix earlier wouldn't hurt anyone I think, but I don't know all the reasons behind these choices.

[JohnGalt1717]

@alexmurari That's effectively what I've been asking for: Be transparent, own the fact that there are major regressions even in the stuff they promised did work, and work fast to fix them and put the onus on EF Core team to get it right, not customers to work around it.

The problem is that EF is still brittle and they're terrified of breaking stuff when they fix other things so they're avoiding fixing stuff between major versions. Except they broke everyone's code horribly in EF Core 3.x, so accidentally breaking something that was working in previous .net core 3.x releases is just "stay on the older 3.x release until we fix it next month." whereas the stuff that they borked between EF Core 2.x and 3.x is blocking upgrades and putting companies at security risk.

It appears no one is doing the math, and no one wants to admit the mess they created so they're just sticking their heads in the sand and ignoring it.

What I've suggested should be happening is that .NET 3.2 should including EF Core 3.2 that is optional, and pulls down all of the fixes for everything from .NET 5, and iterates quickly as a short term supported release. I mean weekly production builds that fix all of this stuff.

[ajcvickers]

@alexmurari See the release planning process. More specifically, there is a patch release process for all of .NET Core. This allows patches in different parts of the stack to be validated against each other. Beyond this, patches may also need to be synchronized with .NET Framework, Visual Studio, Windows, and so on. All of this means that changes for a patch need to be merged significantly before the release date in order for this synchronization and validation to happen.

So that's the current situation. However, we are working across .NET to be able to ship faster with the same level of validation. Also, on the EF team, we have investigated what it would take to ship patches outside of the .NET Core process. We concluded that it is possible, but it does involve considerable extra work for the EF team, and adds complications to the general patch process. At this time we're going to stick with the .NET Core process.

[ms-saltation]

        public static IOrderedQueryable<TSource> OrderBy<TSource>(this IQueryable<TSource> query, string propertyName)
        {
            var entityType = typeof(TSource);

            // Create x=>x.PropName
            var propertyInfo = entityType.GetProperty(propertyName);
            if (propertyInfo.DeclaringType != entityType)
            {
                propertyInfo = propertyInfo.DeclaringType.GetProperty(propertyName);
            }

            // If we try to order by a property that does not exist in the object return the list
            if (propertyInfo == null)
            {
                return (IOrderedQueryable<TSource>)query;
            }

            var arg = Expression.Parameter(entityType, "x");
            var property = Expression.MakeMemberAccess(arg, propertyInfo);
            var selector = Expression.Lambda(property, new ParameterExpression[] { arg });

            // Get System.Linq.Queryable.OrderBy() method.
            var method = typeof(Queryable).GetMethods()
                 .Where(m => m.Name == "OrderBy" && m.IsGenericMethodDefinition)
                 .Where(m => m.GetParameters().ToList().Count == 2) // ensure selecting the right overload
                 .Single();

            //The linq's OrderBy<TSource, TKey> has two generic types, which provided here
            MethodInfo genericMethod = method.MakeGenericMethod(entityType, propertyInfo.PropertyType);

            /* Call query.OrderBy(selector), with query and selector: x=> x.PropName
              Note that we pass the selector as Expression to the method and we don't compile it.
              By doing so EF can extract "order by" columns and generate SQL for it. */
            return (IOrderedQueryable<TSource>)genericMethod.Invoke(genericMethod, new object[] { query, selector });
        }

Updated above method.
Mainly, get actual propertyInfo from the class declaring the member & use Expression.MakeMemberAccess

Hey, thanks for this snippet. Worsk fine so far.. how can i do this with case insensitive ordering ?

[rodrigol-insight]

Thanks a lot, this saved my day!