Support IEquatable<T> in LINQ provider

[kevinchalet]

Hey,

First, I'd like to say I'm very impressed by the effort you guys put to make NHibernate better than ever (.NET Standard 2.0 is just awesome).

I'm currently writing NHibernate-based stores for my OpenIddict project and so far, the experience has been extremely pleasant. I'm only blocked by the fact the default EqualsGenerator doesn't support the IEquatable<T> interface, that I use in my NHibernate LINQ queries.

Consider the following model and mapping (a simplified copy of what's used in ASP.NET Core Identity 1.x/2.x, that uses exactly the same pattern):

public class MyIdentityUser<TKey> where TKey : IEquatable<TKey>
{
    public virtual TKey Id { get; set; }
}

public class MyIdentityUserMapping<TKey> : ClassMapping<MyIdentityUser<TKey>>
    where TKey : IEquatable<TKey>
{
    public MyIdentityUserMapping()
    {
        Id(user => user.Id);
    }
}

Thanks to the generic TKey identifier, you can write framework code that uses equality checks without forcing a specific type at compile time (e.g string, int, long, Guid, etc.):

public Task<IdentityUser<TKey>> GetUserAsync(TKey id)
    where TKey : IEquatable<TKey>
{
    return (from user in _session.Query<IdentityUser<TKey>>()
            where user.Id.Equals(id)
            select user).FirstOrDefaultAsync();
}

Since EqualsGenerator doesn't support this interface, all you get when trying to execute this method is an exception:

{System.NotSupportedException: Boolean Equals(System.Guid)
   at NHibernate.Linq.Visitors.HqlGeneratorExpressionVisitor.VisitMethodCallExpression(MethodCallExpression expression)
   at NHibernate.Linq.Visitors.HqlGeneratorExpressionVisitor.VisitExpression(Expression expression)
   at NHibernate.Linq.Visitors.HqlGeneratorExpressionVisitor.Visit(Expression expression, VisitorParameters parameters)
   at NHibernate.Linq.Visitors.QueryModelVisitor.VisitWhereClause(WhereClause whereClause, QueryModel queryModel, Int32 index)
   at Remotion.Linq.Clauses.WhereClause.Accept(IQueryModelVisitor visitor, QueryModel queryModel, Int32 index)
   at Remotion.Linq.QueryModelVisitorBase.VisitBodyClauses(ObservableCollection`1 bodyClauses, QueryModel queryModel)
   at Remotion.Linq.QueryModelVisitorBase.VisitQueryModel(QueryModel queryModel)
   at NHibernate.Linq.Visitors.QueryModelVisitor.Visit()
   at NHibernate.Linq.Visitors.QueryModelVisitor.GenerateHqlQuery(QueryModel queryModel, VisitorParameters parameters, Boolean root, Nullable`1 rootReturnType)
   at NHibernate.Linq.NhLinqExpression.Translate(ISessionFactoryImplementor sessionFactory, Boolean filter)
   at NHibernate.Hql.Ast.ANTLR.ASTQueryTranslatorFactory.CreateQueryTranslators(IQueryExpression queryExpression, String collectionRole, Boolean shallow, IDictionary`2 filters, ISessionFactoryImplementor factory)
   at NHibernate.Engine.Query.QueryExpressionPlan.CreateTranslators(IQueryExpression queryExpression, String collectionRole, Boolean shallow, IDictionary`2 enabledFilters, ISessionFactoryImplementor factory)
   at NHibernate.Engine.Query.QueryExpressionPlan..ctor(IQueryExpression queryExpression, Boolean shallow, IDictionary`2 enabledFilters, ISessionFactoryImplementor factory)
   at NHibernate.Engine.Query.QueryPlanCache.GetHQLQueryPlan(IQueryExpression queryExpression, Boolean shallow, IDictionary`2 enabledFilters)
   at NHibernate.Impl.AbstractSessionImpl.GetHQLQueryPlan(IQueryExpression queryExpression, Boolean shallow)
   at NHibernate.Impl.AbstractSessionImpl.CreateQuery(IQueryExpression queryExpression)
   at NHibernate.Linq.DefaultQueryProvider.PrepareQuery(Expression expression, IQuery& query)
   at NHibernate.Linq.DefaultQueryProvider.ExecuteAsync(Expression expression, CancellationToken cancellationToken)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1.ConfiguredTaskAwaiter.GetResult()
   at NHibernate.Linq.DefaultQueryProvider.<ExecuteAsync>d__22`1.MoveNext()

The fix is extremely simple: updating EqualsGenerator to support IEquatable:

public EqualsGenerator()
{
    SupportedMethods = new[]
    {
        // ... existing methods ...

        ReflectHelper.GetMethodDefinition<IEquatable<string>>(x => x.Equals(default(string))),
        ReflectHelper.GetMethodDefinition<IEquatable<char>>(x => x.Equals(default(char))),
        ReflectHelper.GetMethodDefinition<IEquatable<sbyte>>(x => x.Equals(default(sbyte))),
        ReflectHelper.GetMethodDefinition<IEquatable<byte>>(x => x.Equals(default(byte))),
        ReflectHelper.GetMethodDefinition<IEquatable<short>>(x => x.Equals(default(short))),
        ReflectHelper.GetMethodDefinition<IEquatable<ushort>>(x => x.Equals(default(ushort))),
        ReflectHelper.GetMethodDefinition<IEquatable<int>>(x => x.Equals(default(int))),
        ReflectHelper.GetMethodDefinition<IEquatable<uint>>(x => x.Equals(default(uint))),
        ReflectHelper.GetMethodDefinition<IEquatable<long>>(x => x.Equals(default(long))),
        ReflectHelper.GetMethodDefinition<IEquatable<ulong>>(x => x.Equals(default(ulong))),
        ReflectHelper.GetMethodDefinition<IEquatable<float>>(x => x.Equals(default(float))),
        ReflectHelper.GetMethodDefinition<IEquatable<double>>(x => x.Equals(default(double))),
        ReflectHelper.GetMethodDefinition<IEquatable<decimal>>(x => x.Equals(default(decimal))),
        ReflectHelper.GetMethodDefinition<IEquatable<Guid>>(x => x.Equals(default(Guid))),
        ReflectHelper.GetMethodDefinition<IEquatable<DateTime>>(x => x.Equals(default(DateTime))),
        ReflectHelper.GetMethodDefinition<IEquatable<DateTimeOffset>>(x => x.Equals(default(DateTimeOffset))),
        ReflectHelper.GetMethodDefinition<IEquatable<bool>>(x => x.Equals(default(bool)))
    };
}

When these definitions are added, everything works like a charm.
If you guys agree with the change, I'd be happy to send a PR.

[fredericDelaporte]

Hello,

Why not supporting IEquatable<T> in the LINQ provider indeed.

But for the implementation, just add ReflectHelper.GetMethodDefinition<IEquatable<object>>(x => x.Equals(default(object))). It should be enough.

ReflectHelper.GetMethodDefinition returns the generic method definition when the supplied method is generic, instead of the type-resolved one it has received as argument. So your series of addition would add seventeen times the same method definition.

(That is not the case for the current method definitions in this generator: they are all non-generic.)

[kevinchalet]

It should be enough.

Oh yeah, great tip, thanks!

Why not supporting IEquatable in the LINQ provider indeed.

Honestly, I'm not familiar enough with the NH LINQ provider to determine how easy/complicated this would be. Adding a few lines (well, now, just one) in EqualsGenerator seemed easier to me 😄

[fredericDelaporte]

That is the way it should be done, I was not thinking about something else. Yes you are welcome to contribute this. (Just make sure you follow the contributing guidelines.)

[kevinchalet]

lol, my bad. Not sure why, but I read "instead" instead of "indeed" and thought you were suggested supporting this feature in a different layer of NHibernate, sorry 😅

I'll read the guidelines and submit a PR today.

[kevinchalet]

ReflectHelper.GetMethodDefinition returns the generic method definition when the supplied method is generic, instead of the type-resolved one it has received as argument. So your series of addition would add seventeen times the same method definition.

Actually, this won't work because the method IEquatable<T>.Equals(T) is not generic: it's the interface which is generic, not the method.

[kevinchalet]

PR submitted: #1714

I used the approach mentioned in my initial post, as ReflectHelper.GetMethodDefinition<IEquatable<object>>(x => x.Equals(default(object))) didn't work as expected.

[fredericDelaporte]

Yes you are right. But then there is another solution. Something like this (preferably with the comment typo fixed and the hard-coded string changed for a nameof) can be done, then registered here.

[kevinchalet]

Interesting. If you prefer this option, I can certainly give it a try.

[fredericDelaporte]

I am also thinking about whether it could be possible to still handle this case with a single addition to SupportedMethods or not. Like registering instead the method from the generic type definition, then adding after this line a check on the DeclaringType for resolving the method on the generic type definition and checking again the dictionary.
Of course that is more work, but a dictionary lookup is better than the loop done on IRuntimeMethodHqlGenerator.

[fredericDelaporte]

The main point is whether it makes sense to support IEquatable<T> for any types or whether it should be done only for a limited set of types. Since NHibernate handles entity equality and enum equality, I think supporting any type should be done.