Query: Inconsistent behavior of one-to-one and one-to-many relationships with overloaded operator !=

axel-habermaier commented 7 years ago

I stumbled across a surprising inconsistency between one-to-one and one-to-many relationships in conjunction with an overloaded operator !=. I would not expect the following code to work - at the very least, however, I would not expect to find a difference between the arity of relationships.

Exception message: The operands for operator 'NotEqual' do not match the parameters of method 'op_Inequality'.
Stack trace:    at System.Linq.Expressions.Expression.GetMethodBasedBinaryOperator(ExpressionType binaryType, Expression left, Expression right, MethodInfo method, Boolean liftToNull)
   at System.Linq.Expressions.Expression.NotEqual(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 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)
   at Remotion.Linq.Clauses.WhereClause.TransformExpressions(Func`2 transformation)
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.NavigationRewritingExpressionVisitor.NavigationRewritingQueryModelVisitor.VisitWhereClause(WhereClause whereClause, QueryModel queryModel, Int32 index)
   at Remotion.Linq.QueryModelVisitorBase.VisitBodyClauses(ObservableCollection`1 bodyClauses, QueryModel queryModel)
   at Remotion.Linq.QueryModelVisitorBase.VisitQueryModel(QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.NavigationRewritingExpressionVisitor.Rewrite(QueryModel queryModel, QueryModel parentQueryModel)
   at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.OptimizeQueryModel(QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.CreateQueryExecutor[TResult](QueryModel queryModel)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileQueryCore[TResult](Expression query, INodeTypeProvider nodeTypeProvider, IDatabase database, ILogger logger, Type contextType)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass19_0`1.<CompileQuery>b__0()
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQueryCore[TFunc](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query)
   at Remotion.Linq.QueryableBase`1.GetEnumerator()
   at System.Collections.Generic.EnumerableHelpers.ToArray[T](IEnumerable`1 source, Int32& length)
   at System.Collections.Generic.EnumerableHelpers.ToArray[T](IEnumerable`1 source)
   at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
   at ConsoleApp5.Program.Main(String[] args) in c:\users\axel\documents\visual studio 2017\Projects\ConsoleApp5\ConsoleApp5\Program.cs:line 17

Steps to reproduce

Create a new .NET Core 1.1 console app in VS 2017.02.

Add the following to the generated .csproj file:

<ItemGroup>
<PackageReference Include="Microsoft.EntityFrameworkCore.InMemory" Version="1.1.2" />
</ItemGroup>

Replace the generated Program.cs with the following code:

namespace ConsoleApp5
{
using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations.Schema;
using System.Linq;
using Microsoft.EntityFrameworkCore;

class Program
{
    static void Main(string[] args)
    {
        var c = new TestContext();
        c.Database.EnsureCreated();

        c.A.Where(x => x.B != null).ToArray();
        c.A.Where(x => x.C != null).ToArray();
    }
}

class TestContext : DbContext
{
    public DbSet<A> A { get; set; }
    public DbSet<B> B { get; set; }
    public DbSet<C> C { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseInMemoryDatabase("test");
    }
}

class Base
{
    public int Id { get; set; }
}

class A : Base
{
    public C C { get; set; }
    public B B { get; set; }
}

class B : Base
{
    [ForeignKey(nameof(AKey))]
    public A A { get; set; }

    public int AKey { get; set; }
}

class C : Base
{
    public List<A> A { get; set; }
}
}

Run the application. There should be no output and no exceptions.

Now add the following code to the Base class:

    public static bool operator ==(Base left, Base right)
    {
        return Equals(left, right);
    }

    public static bool operator !=(Base left, Base right)
    {
        return !Equals(left, right);
    }

Run the application. The first query, involving only the one-to-one relationship, works fine. The second query involving the one-to-many relationship, however, results in the exception mentioned above.

Expected behavior

Either both queries should result in the exception, or both should work. However, I don't really see how custom equality operators should be supported by SQL translator, hence I would assume that both queries should throw.

Actual behavior

Only the one-to-many relationship throws. The one-to-one relationship probably ignores the custom operator !=.

Further technical details

EF Core version: 1.1.2 Database Provider: both SqlServer and InMemory Operating system: Win10, x64 IDE: VS 2017.02

ajcvickers commented 7 years ago

@axel-habermaier You need to be very careful doing any custom equality on entity types for two reasons:

EF query translation does not (and basically cannot) understand the semantics of your equality operators and so query translations will be the same regardless of whether operators are overloaded or not. But when expressions are evaluated on the client, then the operators will be used, which can result in differences in behavior.
Tracking of entities inside DbContext for identity resolution is forced to use reference equality wherever possible because EF can only track a single instance of a given entity with a given primary key value. However, in some cases, notably collection navigation properties, EF cannot force reference equality because the application code creates the collection type rather than EF doing so. This can then result in inconsistent behavior when doing fixup of collections.

Leaving this open for now in case @divega @anpete or @smitpatel want to comment more.

axel-habermaier commented 7 years ago

@ajcvickers: Thanks for your response. I understand that overloading equality is problematic and I don't really want to use it. However, I stumbled across this seemingly inconsistent behaviour of one-to-one and many-to-one relationships that happens to involve overloaded equality operators. So this issue is a suggestion to improve this inconsistency rather than allowing custom equality.

ajcvickers commented 7 years ago

@axel-habermaier I'm not sure how we would improve it, but leaving it open for ideas from others.

smitpatel commented 4 years ago

This works in 3.1 without any exceptions.

dotnet / efcore