Efcore: Query: NotSupportedException while trying to use EF.Property

[roji]

While working on array property support for Npgsql (#9183), I ran across an exception while trying to access an entity's array property via EF.Property. Accessing the same property in the regular, strongly-typed way worked just fine. I'm not sure to what extent this problem is specific to the fact that the property is an array, the exception message seems to suggest otherwise.

The failing test is Length_on_EF_Property, compare to Length. The exception is:

 System.NotSupportedException : Cannot parse expression 'e' as it has an unsupported type. Only query sources (that is, expressions that implement IEnumerable) and query operators can be parsed.
---- System.ArgumentException : Parameter 'expression.Type' is a 'Microsoft.EntityFrameworkCore.Query.SomeEntity', which cannot be assigned to type 'System.Collections.IEnumerable'.

[roji]

/cc @smitpatel

[anpete]

@roji Can you post the stack trace?

[roji]

Here you go:

Failed   Microsoft.EntityFrameworkCore.Query.ArrayQueryTest.Length_on_EF_Property
Error Message:
 System.NotSupportedException : Cannot parse expression 'e' as it has an unsupported type. Only query sources (that is, expressions that implement IEnumerable) and query operators can be parsed.
---- System.ArgumentException : Parameter 'expression.Type' is a 'Microsoft.EntityFrameworkCore.Query.SomeEntity', which cannot be assigned to type 'System.Collections.IEnumerable'.
Parameter name: expression.Type
Stack Trace:
   at Remotion.Linq.Parsing.Structure.ExpressionTreeParser.ParseNonQueryOperatorExpression(Expression expression, String associatedIdentifier)
   at Remotion.Linq.Parsing.Structure.ExpressionTreeParser.ParseNode(Expression expression, String associatedIdentifier)
   at Remotion.Linq.Parsing.Structure.ExpressionTreeParser.ParseMethodCallExpression(MethodCallExpression methodCallExpression, String associatedIdentifier)
   at Remotion.Linq.Parsing.Structure.ExpressionTreeParser.ParseNode(Expression expression, String associatedIdentifier)
   at Remotion.Linq.Parsing.Structure.ExpressionTreeParser.ParseMethodCallExpression(MethodCallExpression methodCallExpression, String associatedIdentifier)
   at Remotion.Linq.Parsing.Structure.ExpressionTreeParser.ParseNode(Expression expression, String associatedIdentifier)
   at Remotion.Linq.Parsing.Structure.ExpressionTreeParser.ParseTree(Expression expressionTree)
   at Remotion.Linq.Parsing.Structure.QueryParser.GetParsedQuery(Expression expressionTreeRoot)
   at Remotion.Linq.Parsing.ExpressionVisitors.SubQueryFindingExpressionVisitor.Visit(Expression expression)
   at System.Linq.Expressions.ExpressionVisitor.VisitBinary(BinaryExpression node)
   at System.Linq.Expressions.BinaryExpression.Accept(ExpressionVisitor visitor)
   at Remotion.Linq.Parsing.ExpressionVisitors.SubQueryFindingExpressionVisitor.Visit(Expression expression)
   at System.Linq.Expressions.ExpressionVisitor.VisitLambda[T](Expression`1 node)
   at System.Linq.Expressions.Expression`1.Accept(ExpressionVisitor visitor)
   at Remotion.Linq.Parsing.ExpressionVisitors.SubQueryFindingExpressionVisitor.Visit(Expression expression)
   at Remotion.Linq.Parsing.ExpressionVisitors.SubQueryFindingExpressionVisitor.Process(Expression expressionTree, INodeTypeProvider nodeTypeProvider)
   at Remotion.Linq.Parsing.Structure.MethodCallExpressionParser.ProcessArgumentExpression(Expression argumentExpression)
   at System.Linq.Enumerable.WhereSelectEnumerableIterator`2.MoveNext()
   at System.Linq.Buffer`1..ctor(IEnumerable`1 source)
   at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
   at Remotion.Linq.Parsing.Structure.MethodCallExpressionParser.Parse(String associatedIdentifier, IExpressionNode source, IEnumerable`1 arguments, MethodCallExpression expressionToParse)
   at Remotion.Linq.Parsing.Structure.ExpressionTreeParser.ParseMethodCallExpression(MethodCallExpression methodCallExpression, String associatedIdentifier)
   at Remotion.Linq.Parsing.Structure.ExpressionTreeParser.ParseNode(Expression expression, String associatedIdentifier)
   at Remotion.Linq.Parsing.Structure.ExpressionTreeParser.ParseTree(Expression expressionTree)
   at Remotion.Linq.Parsing.Structure.QueryParser.GetParsedQuery(Expression expressionTreeRoot)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileQueryCore[TResult](Expression query, INodeTypeProvider nodeTypeProvider, IDatabase database, IDiagnosticsLogger`1 logger, Type contextType)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass15_0`1.<Execute>b__0()
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQueryCore[TFunc](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.Execute[TResult](Expression expression)
   at System.Linq.Queryable.Single[TSource](IQueryable`1 source, Expression`1 predicate)
   at Microsoft.EntityFrameworkCore.Query.ArrayQueryTest.Length_on_EF_Property() in C:\projects\Npgsql.EntityFrameworkCore.PostgreSQL\test\EFCore.PG.FunctionalTests\Query\ArrayQueryTest.cs:line 142
----- Inner Stack Trace -----
   at Remotion.Utilities.ArgumentUtility.CheckTypeIsAssignableFrom(String argumentName, Type actualType, Type expectedType)
   at Remotion.Linq.Parsing.Structure.IntermediateModel.MainSourceExpressionNode..ctor(String associatedIdentifier, Expression expression)
   at Remotion.Linq.Parsing.Structure.ExpressionTreeParser.ParseNonQueryOperatorExpression(Expression expression, String associatedIdentifier)

[roji]

FYI here's my context and the entity class:

```c#
public class ArrayContext : DbContext
{
public DbSet SomeEntities { get; set; }
public ArrayContext(DbContextOptions options) : base(options) {}
}

public class SomeEntity
{
public int Id { get; set; }
public int[] SomeArray { get; set; }
public int[,] SomeMatrix { get; set; }
public byte[] SomeBytea { get; set; }
public string SomeText { get; set; }
}
```

[anpete]

cc @MichaelKetting - Looks like a relinq parser bug.

[MichaelKetting]

@anpete Hmm, can't say that it isn't, can't say that it is without a deep dive into this issue. I've looked around just a bit in Github and it looks like EF.Property is just a marker for some transformation logic later on (https://github.com/aspnet/EntityFramework/blob/dev/src/EFCore/EF.cs). Since non-collection-type scenarios seem to work (https://github.com/aspnet/EntityFramework/issues/4875), the question is if either there is a transformation logic in re-linq that hits too early and does something bad with the collection type or the EF-logic changes the collection type (int[]) of the property to the non-collection type (int) of the collection element.

I'm assuming that
ctx.SomeEntities.Single(e => EF.Property and
ctx.SomeEntities.Single(e => e.SomeArray.Length == 2);
are semantically equivalent and that the non-reflection version works?

Could you point me to the type that handles the EF.Property references in EF?

[roji]

@MichaelKetting yes, the equivalent test which accesses the array property with EF.Property works fine (see this test).

As to the type that handles EF.Property references in EF, someone from the EF Core team will have to answer...

[MichaelKetting]

@roji Thanks for pointing out the test. I managed to only look at the one below the problematic one. Silly me...

[anpete]

The stack is indicating that this is happening very early (before any EF.Property handling) in the relinq parsing stage. We have done parameterization by this stage, though, so it could be some problem there.

[MichaelKetting]

@anpete Thanks, I'll look into getting a repro sample running in re-linq.

[divega]

EF Triage: we will keep this open to track when things get fixed, but moving to external milestone.

[MichaelKetting]

@anpete
Okay, I've been able to reproduce this but I'm not yet sure if this should be considered by-design or not.

First off, there is a way to get rid of the exception by registering a method call transformer:

  public static class EF
  {
    public class PropertyTransformer: IExpressionTransformer<MethodCallExpression>

    {
      public ExpressionType[] SupportedExpressionTypes
      {
        get { return new[] { ExpressionType.Call, }; }
      }

      public Expression Transform (MethodCallExpression expression)
      {
        var instance = expression.Arguments[0];
        var propertyName = (ConstantExpression) expression.Arguments[1];
        var propertyInfo = instance.Type.GetProperty ((string) propertyName.Value);
        return Expression.Property (instance, propertyInfo);
      }
    }

    [MethodCallExpressionTransformer (typeof(PropertyTransformer))]
    public static TProperty Property<TProperty> (object entity, string propertyName)
    {
      throw new InvalidOperationException ("not implemented");
    }
  }

This will take the reflection call and turn it into a regular property call. I'm not sure that's what you want, though, since it will remove the information that this was once an EF.Property-call.

There's also the bigger approach of using a custom expression type with a custom-registered IExpressionTransformer:

  public class PropertyTransformer : IExpressionTransformer<Expression>
  {
    public ExpressionType[] SupportedExpressionTypes
    {
      get { return new[] { ExpressionType.Call, }; }
    }

    public Expression Transform (Expression expression)
    {
      var methodCallExpression = (MethodCallExpression) expression;
      if (methodCallExpression.Method.DeclaringType != typeof (EF) && methodCallExpression.Method.Name != "Property")
        return expression;

      var instance = methodCallExpression.Arguments[0];
      var propertyName = (ConstantExpression) methodCallExpression.Arguments[1];
      var propertyInfo = instance.Type.GetProperty ((string) propertyName.Value);
      return new PropertyReferenceExpression (instance, propertyInfo);
    }
  }

  public class PropertyReferenceExpression : Expression
  {
    private readonly Expression _instance;
    private readonly PropertyInfo _propertyInfo;

    public PropertyReferenceExpression ()
    {      
    }

    public PropertyReferenceExpression (Expression instance, PropertyInfo propertyInfo)
    {
      _instance = instance;
      _propertyInfo = propertyInfo;
    }

    public override ExpressionType NodeType
    {
      get { return ExpressionType.Extension; }
    }

    public override Type Type
    {
      get { return _propertyInfo.PropertyType; }
    }

    protected override Expression VisitChildren (ExpressionVisitor visitor)
    {
      var newExpression = visitor.Visit (_instance);
      if (newExpression != _instance)
        return new PropertyReferenceExpression (newExpression, _propertyInfo);
      else
        return this;
    }

    protected override Expression Accept (ExpressionVisitor visitor)
    {
      return base.Accept (visitor);
    }
  }

Now to the underlying issue: re-linq has special handling for collection types to facilitate scenarios such as get-length by wrapping them into sub queries. Those sub queries in turn must either be roots (e.g. joining two tables) or property accesses. Everything else is considered highly suspect by the code as it stands right now. So, by using the above approaches you get arond this. The first one simply resolves the Reflection call into the respective underlying operation and the second one preserves the information that this was a Reflection call.

Let me know if I missed something in those approaches regarding your scenario :)

[smitpatel]

The main scenario for using EF.Property method is shadow property access. Shadow properties in EF are properties defined on the model but does not exists in CLR type. So in above transformations, propertyInfo is not available always. 😢

[MichaelKetting]

@smitpatel That's not an issue with the second version. I just used PropertyInfo because it was convenient while spiking around, but really, you just need the property's type (aka Expression.Type) and the property's name.

[smitpatel]

Basically, we need to transform the method call into something else so ReLinq does not process them as collection types?

[MichaelKetting]

Almost. What I acutally did was make sure that re-linq stops processing when reaching the EF.Property node and turn this entire thing into a sub query. The exception originated because linq would try to drill deeper into the expression tree, taking apart EF.Property and it's arguments and then end up with something that's no longer compatible with sub queries.

Just as a disclaimer: It might very well be that there's a missing extension point hidden in the ExpressionTreeParser.GetQueryOperatorExpression() or somewhere around that code to make such scenarios more...let's say obvious, but a) I'm not sure yet and b) if the above approach works when integrated in EF, you're set already with what you have :)

[MichaelKetting]

Okay, we decided that this would also make a nice extension point (rules for sub-query detection, https://www.re-motion.org/jira/browse/RMLNQ-114), but that's not something on the immideate roadmap so long as there's sensible workarounds available.

[smitpatel]

Removing milestone. This is something we may need to do on our end if we want to support this for postgre.
cc: @roji

[roji]

Note that this is definitely not the most high-priority issue, even for PostgreSQL support. Array types are probably not very widely used, and shadow property access is probably even more rare.

[smitpatel]

With relinq going away and not causing subquery, this issue should get fixed on its own in 3.0 pipeline.

[smitpatel]

@roji - Can you verify if this is fixed in new world?

[roji]

Will do. To do properly this will require syncing EFCore.PG to preview6 and the new pipeline which will probably be a bit of work :)

[roji]

Confirmed, this passes with the new query pipeline! Yay!