Query: compilation error for queries that compare reference navigation to null, if the navigation can be optimized into FK

[maumar]

repro:

from o in ctx.Orders
where o.CustomerID == "ALFKI"
select o.Customer == null

exception:

System.InvalidOperationException : The operands for operator 'Equal' do not match the parameters of method 'op_Equality'.
	at System.Linq.Expressions.Expression.GetMethodBasedBinaryOperator(ExpressionType binaryType, Expression left, Expression right, MethodInfo method, Boolean liftToNull)
	at System.Linq.Expressions.Expression.Equal(Expression left, Expression right, Boolean liftToNull, MethodInfo method)
	at System.Linq.Expressions.Expression.MakeBinary(ExpressionType binaryType, Expression left, Expression right, Boolean liftToNull, MethodInfo method, LambdaExpression conversion)
	at System.Linq.Expressions.Expression.MakeBinary(ExpressionType binaryType, Expression left, Expression right, Boolean liftToNull, MethodInfo method)
	D:\git\EntityFramework\src\EFCore\Query\ExpressionVisitors\Internal\NavigationRewritingExpressionVisitor.cs(382,0): at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.NavigationRewritingExpressionVisitor.VisitBinary(BinaryExpression node)
	at System.Linq.Expressions.BinaryExpression.Accept(ExpressionVisitor visitor)
	at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
	D:\git\EntityFramework\src\EFCore\Query\ExpressionVisitors\Internal\NavigationRewritingExpressionVisitor.cs(391,0): at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.NavigationRewritingExpressionVisitor.VisitConditional(ConditionalExpression node)
	at System.Linq.Expressions.ConditionalExpression.Accept(ExpressionVisitor visitor)
	at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
	at Remotion.Linq.Clauses.SelectClause.TransformExpressions(Func`2 transformation)
	D:\git\EntityFramework\src\EFCore\Query\ExpressionVisitors\Internal\ExpressionTransformingQueryModelVisitor.cs(90,0): at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.ExpressionTransformingQueryModelVisitor`1.VisitSelectClause(SelectClause selectClause, QueryModel queryModel)
	D:\git\EntityFramework\src\EFCore\Query\ExpressionVisitors\Internal\NavigationRewritingExpressionVisitor.cs(1510,0): at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.NavigationRewritingExpressionVisitor.NavigationRewritingQueryModelVisitor.VisitSelectClause(SelectClause selectClause, QueryModel queryModel)
	at Remotion.Linq.Clauses.SelectClause.Accept(IQueryModelVisitor visitor, QueryModel queryModel)
	at Remotion.Linq.QueryModelVisitorBase.VisitQueryModel(QueryModel queryModel)
	D:\git\EntityFramework\src\EFCore\Query\ExpressionVisitors\Internal\NavigationRewritingExpressionVisitor.cs(237,0): at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.NavigationRewritingExpressionVisitor.Rewrite(QueryModel queryModel, QueryModel parentQueryModel)
	D:\git\EntityFramework\src\EFCore\Query\EntityQueryModelVisitor.cs(288,0): at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.OptimizeQueryModel(QueryModel queryModel, Boolean asyncQuery)
	D:\git\EntityFramework\src\EFCore.Relational\Query\RelationalQueryModelVisitor.cs(1157,0): at Microsoft.EntityFrameworkCore.Query.RelationalQueryModelVisitor.OptimizeQueryModel(QueryModel queryModel, Boolean asyncQuery)
	D:\git\EntityFramework\src\EFCore\Query\EntityQueryModelVisitor.cs(165,0): at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.CreateQueryExecutor[TResult](QueryModel queryModel)
	D:\git\EntityFramework\src\EFCore\Storage\Database.cs(70,0): at Microsoft.EntityFrameworkCore.Storage.Database.CompileQuery[TResult](QueryModel queryModel)
	--- End of stack trace from previous location where exception was thrown ---
	at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
	D:\git\EntityFramework\src\EFCore\Query\Internal\QueryCompiler.cs(164,0): at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileQueryCore[TResult](Expression query, INodeTypeProvider nodeTypeProvider, IDatabase database, IDiagnosticsLogger`1 logger, Type contextType)
	D:\git\EntityFramework\src\EFCore\Query\Internal\QueryCompiler.cs(103,0): at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass15_0`1.<Execute>b__0()
	D:\git\EntityFramework\src\EFCore\Query\Internal\CompiledQueryCache.cs(69,0): at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQueryCore[TFunc](Object cacheKey, Func`1 compiler)
	D:\git\EntityFramework\src\EFCore\Query\Internal\CompiledQueryCache.cs(44,0): at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func`1 compiler)
	D:\git\EntityFramework\src\EFCore\Query\Internal\QueryCompiler.cs(99,0): at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query)
	D:\git\EntityFramework\src\EFCore\Query\Internal\EntityQueryProvider.cs(62,0): at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.Execute[TResult](Expression expression)
	at Remotion.Linq.QueryableBase`1.GetEnumerator()
	at System.Linq.Buffer`1..ctor(IEnumerable`1 source)
	at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)

[maumar]

This happens because in nav rewrite, when we visit binary expression, after visiting left and right operands we recreate new expression using

Expression.MakeBinary(node.NodeType, newLeft, newRight, node.IsLiftedToNull, node.Method)

problem is that for o.Customer == null, the left operand gets converted into FK: o.CustomerID, which is typed as string, not Customer. However we still try to use method from the original expression which is op_Equality(Customer, Customer)

[smitpatel]

@maumar - Perhaps defer this to Nav rewrite?

[smitpatel]

I believe this is already fixed in new entity equality. Need a regression test.

[maumar]

verified this issue has been fixed in 3.0

[roji]

@maumar want to also write the regression test? If so you can unassign me, or otherwise unassign yourself etc.

[maumar]

@roji i have a pr with regression tests for all those issues almost ready