We're updating the issue view to help you get more done. 

Relinq parser doesn't perform some optimizations when accessing a property defined on a base type, rather than the exact type of the parameter

Description

class Program
{
static void Main(string[] args)
{
using (var ctx = new MyContext())

{ ctx.Database.EnsureDeleted(); ctx.Database.EnsureCreated(); }

using (var ctx = new MyContext())
{
var query = ctx.Set<Package>().Select(p => new PackageViewModel()

{ CreatedByUserName = p.CreatedBy.UserName, Id = p.Id }

);

//var param = Expression.Parameter(typeof(BaseViewModel), "p"); // optimized
var param = Expression.Parameter(typeof(PackageViewModel), "p"); // unoptimized

var exp = Expression.Lambda<Func<PackageViewModel, bool>>(
Expression.Equal(
Expression.Property(param, "CreatedByUserName"),
Expression.Constant("Ivan", typeof(String))
), param
);

var fullQuery = query.Where(exp);

var result = fullQuery.ToList();
}
}
}

public class MyContext : DbContext

{ public DbSet<Package> Packages
{ get; set; }

public DbSet<User> Users { get; set; }

protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)

{ optionsBuilder.UseSqlServer(@"Server=.;Database=RelinqRepro;Trusted_Connection=True;MultipleActiveResultSets=True"); }

}

public class Package

{ public Guid Id
{ get; set; }
public User CreatedBy { get; set; }
}

public class User { public int Id { get; set; }
public string UserName { get; set; }
}

public class BaseViewModel { public String CreatedByUserName { get; set; }
}

public class PackageViewModel : BaseViewModel{ public Guid Id { get; set; }

}

 

 

 

 

 

if param is typed as BaseViewModel, parser produces the following QM:

 

from Package p in DbSet<Package>
where [p].CreatedBy.UserName == "Ivan"
select new PackageViewModel

{ CreatedByUserName = [p].CreatedBy.UserName, Id = [p].Id }

 

however, if param is typed as PackageViewModel we get:

from Package p in DbSet<Package>
where new PackageViewModel{ CreatedByUserName = [p].CreatedBy.UserName, Id = [p].Id }

.CreatedByUserName == "Ivan"
select new PackageViewModel

{ CreatedByUserName = [p].CreatedBy.UserName, Id = [p].Id }

 

It should be safe to perform the same optimization for the second case, because CreatedByUserName is not virtual.

 


See also https://github.com/aspnet/EntityFrameworkCore/issues/12230

Activity

Show:
Michael Ketting
June 6, 2018, 6:44 AM

Hi ,

I've given this a first tirage and it appears to be related to the way you set up the Reflection calls. re-linq is designed to work well with the pattern generated by the C# compiler, which would use the declaring type of the property when building the expression tree.

Thus, there's two options:

  1. this is easily solvable on the user side when setting up the expression tree by using the declaring type of the property instead of the derived one that just happens to be lying around.

  2. in re-linq, this change would probably be in the TransparentIdentifierRemovingExpressionVisitor.

I would also like to re-classify this as a feature request and since there's a user-code option, it would be something to consider for the next feature release.

For completeness' sake: Sample tests comaptible with re-linq domain for quick reference purposes:

Boštjan Markežič
May 17, 2020, 2:21 AM

Hi Michael,

recently we encountered the same issue in NHibernate ( and unfortunately the first option is not possible as the expression is generated by an external library (OData). I do understand that re-linq is designed to work well with expressions generated by the C# compiler, but it would be nice to have the ability to override the default behavior of TransparentIdentifierRemovingExpressionVisitor in order to support such cases. Adding a setting when creating a QueryParser would be good enough for us.

Michael Ketting
May 17, 2020, 1:48 PM

Hi Bostjan,

I've marked this for consideration when we're working on the next round of code changes, but that's not going to be until after the summer.

I'm not 100% sure about your suggestion regarding QueryParser. The change isn't really contentious for me, it should be perfectly non-breaking. Right now, we don't have the option for injecting a derived version of the TransparentIdentifierRemovingExpressionVisitor, which would also be much more involved process.

Since this is a fairly simple modifaction of the expression tree, you could try to just rewrite the property nodes in a pre-processing step.

Assignee

Unassigned

Reporter

User known

Components

Fix versions

Priority

Normal
Configure