Support of custom comparers for orderby queries

[bottomup]

I know it can't be translated to SQL, so can I force to evaluate locally?

Steps to reproduce

Some pieces of the relevant code:

IQueryable query = _entities.Set();
query = query.OrderBy(p => p, new Comparers.DistanceComparer(search.Location.Lat, search.Location.Long));
return query.ToList();

public class DistanceComparer : IComparer

The issue

I would like to sort a list by means of a LINQ query. This is not working when making use of a custom comparer.

Exception message:
An exception of type 'System.NotSupportedException' occurred in Remotion.Linq.dll but was not handled in user code

Additional information: Could not parse expression 'value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[Data.Entities.Product]).OrderBy(p => p, __p_0)': This overload of the method 'System.Linq.Queryable.OrderBy' is currently not supported.

Stack trace:
at Remotion.Linq.Parsing.Structure.MethodCallExpressionParser.GetNodeType(MethodCallExpression expressionToParse)
at Remotion.Linq.Parsing.Structure.MethodCallExpressionParser.Parse(String associatedIdentifier, IExpressionNode source, IEnumerable1 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, ILogger logger, Type contextType) at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass19_01.b__0()
at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQueryCore[TFunc](Object cacheKey, Func1 compiler) at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func1 compiler)
at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileQuery[TResult](Expression query)
at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query)
at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.Execute[TResult](Expression expression)
at Remotion.Linq.QueryableBase1.GetEnumerator() at System.Collections.Generic.List1..ctor(IEnumerable1 collection) at System.Linq.Enumerable.ToList[TSource](IEnumerable1 source)

Further technical details

EF Core version: 1.1.0
Operating system: Win10
Visual Studio version: VS 2015

[rowanmiller]

The workaround is to do the OrderBy after the ToList.

@anpete to follow up with relinq

[bottomup]

Ok, but together with skip and take (not in my posted example) that would be less useful.
Curious for the relinq follow-up though.

[divega]

@bottomup the workaround is to make things work in memory. Fixing the limitation in Re-LINQ would help in making the workaround unnecessary but it would still be evaluated in memory. I.e. all rows would be returned from the database, ordered in-memory and finally paging operations would be applied. I cannot think of any significant difference between this and the workaround in terms of how much data gets transfered (although there is a difference in terms of how many objects would actually be materialized). Maybe I am missing some other reason the workaround would be less useful?

As you understand, recognizing a custom comparer and translating it to adequate SQL so that the sorting and the paging can be performed on the server is an entirely different problem and in general we don't have a good way of doing it.

But this particular scenario looks like something that spatial support could help with. That feature is tracked as #1100 (at least for SQL Server).

[bottomup]

Yes, I'm waiting for #1100 to be implemented, that would solve my problem immediately. Hopefully in a next patch before 1.2.0.

Thanks for the elaboration.
The one thing I wonder is about in memory and difference in syntax. While I can filter on distance in a where without modifying the LINQ expressions, I can't do the same for ordering on distance, in this case with a custom comparer. If Re-LINQ would support using custom comparers in orderby statements, I would like to use that for now until spatial is supported.

[maumar]

we should file a bug on relinq

[divega]

@MichaelKetting just in case, do you have any insights on this? Thanks.

[MichaelKetting]

@divega Ah yes. That one's on the todo list, too (https://www.re-motion.org/jira/browse/RMLNQ-108). I have some thoughts on how to integrte this without breaking changes and will get back to you with them as soon as I can put them into a structured form. Sorry things are so slow but there's a bit of an imbalance between the time available for the different projects going on right now.

[divega]

@MichaelKetting Thanks a lot for the quick answer and for pointing to the existing issue. I did not know that we had already reported it 😄

This is not particularly urgent as we have already decided to punt it post-2.0.0, but it would will be great to know when you have a fix.

[OpenSpacesAndPlaces]

@ajcvickers Any timeline on this feature?

I'm running into the same problem with "Distinct(new IEqualityComparer)"

Effectively the same scenario as here:
#7234
(using the double projection solution results in a 10x slower query - ~500ms to 5000ms)

Unless I'm missing something (please let me know if so), there doesn't seem to be a clean/performant path to a apply a SQL distinct over a custom projection.

In my scenario, server processing isn't really an option as I'm pulling the total count and then paging (from a large dataset).

[ajcvickers]

@OpenSpacesAndPlaces Are you sure you're hitting the problem described here? I would suggest opening a new issue and including a small, runnable project/solution or complete code listing that demonstrates the behavior you are seeing so that we can investigate.

[OpenSpacesAndPlaces]

var query = (from mt in context.MyTable
		join mot in context.MyOtherTable on mt.Id = mot.Id
		where mot.SomeProperty = 5
		select new DTO() {
			Id = mt.Id,
			SomeProperty = mot.SomeProperty
		}).Distinct(new DTOComparer()).Skip(1).Take(5).ToList()

	public class DTOComparer : IEqualityComparer<DTO>
	{
		public bool Equals(DTO v1, DTO v2)
		{
			if (v2 == null && v1 == null)
				return true;
			else if (v1 == null || v2 == null)
				return false;
			else if (v1.Id == v2.Id)
				return true;
			else
				return false;
		}

		public int GetHashCode(DTO v)
		{
			return v.Id.GetHashCode();
		}
	}

Message | An error has occurred.
ExceptionMessage | Could  not parse expression    This overload of the method 'System.Linq.Queryable.Distinct' is currently not supported.

ExceptionType | System.NotSupportedException

StackTrace | at  Remotion.Linq.Parsing.Structure.MethodCallExpressionParser.GetNodeType(MethodCallExpression  expressionToParse)     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.Expressi…CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task  task)     at  System.Web.Http.Controllers.ActionFilterResult.<ExecuteAsync>d__5.MoveNext()  --- End of stack trace from previous location where exception was thrown  ---     at  System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task  task)     at  System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task  task)     at  System.Web.Http.Dispatcher.HttpControllerDispatcher.<SendAsync>d__15.MoveNext()
Response payload |  

To be clear #7234 and this item are not directly related in terms of the underlying problem, but it's related in that I was looking at different ways of trying to run a distinct on a projection.

".Select(t => new { }).Distinct().Select(t => new DTO{})" does work, it's just slow.

[smitpatel]

@OpenSpacesAndPlaces - How would you like us to translate DTOComparer to server when applying distinct? Can you write a SQL which can give you result you want?

[OpenSpacesAndPlaces]

@smitpatel

Ideally I wouldn't need a comparer (as I imagine that that probably doesn't translate well to Expressions/SQL.)

At the moment I have to use:
.Select(t => new { }).Distinct().Select(t => new DTO{})

Which ends over-complicating the sql/creating extra overhead.

Ideally I'd prefer to be able to-do like:

.Select(t => new DTO{}).Distinct()
or
.Select(t => new DTO{}).Distinct(p=> p.Id)

As far as raw sql - I'm not sure on your end what the simplest translation might be,
I imagine for a case like, ".Select(t => new DTO{}).Distinct(p=> p.Id)" you might have to-do like:

ROW_NUMBER() OVER (PARTITION BY ID ORDER BY ID DESC) as Row
....
WHERE Row = 1

vs. a straight distinct.

---

[smitpatel]

That is the point, custom comparers cannot be translated to server. So, such query regardless of EF Core executes it in memory, would do distinct/ordering/paging on client side. At that point, users can just do that themselves by calling ToList.

For the second query you write, that is not possible either. We can translate the lambda (even like row_number as you showed in SQL), but Queryable.Distinct does not accept lambda. So the method you propose does not exist.

[OpenSpacesAndPlaces]

Yes - this would be new:
.Select(t => new DTO{}).Distinct(p=> p.Id)

This working (slowly):
.Select(t => new { Id = t.Id }).Distinct().Select(t => new DTO{ Id = t.Id})

And this working (if you allow the output to remain anonymous)
.Select(t => new { Id = t.Id }).Distinct()

Then this not working at all (doesn't actually distinct):
.Select(t => new DTO{ Id = t.Id }).Distinct()

Seems like a bug.

---

None of this happening on the server is a big deal if you are just doing a GetAll - but if you're trying to count/page a large projected dataset, then this isn't ideal. Lets say you have a 10000 items in the tables and you want the first ten of some given query, being forced to load 10000 items into memory to-do distinct, then show 10 to the user.

[smitpatel]

It's not a bug. Once you cast to DTO, we cannot do Distinct because if you have overridden Equals method on your DTO then doing distinct on server may generate different results. Slow result is better than wrong result.

Paging is biggest reason, you want things to be evaluated on server side. But if you want to write your custom code on client for that, then it will not happen. If query needs to be run fully on server side, then care should be taken to understand what server can actually process.

[OpenSpacesAndPlaces]

It's not a bug. Once you cast to DTO, we cannot do Distinct because if you have overridden Equals method on your DTO then doing distinct on server may generate different results. Slow result is better than wrong result.

I'm not trying to be overly critical, please don't take this that way, but the parser silently not working, and ultimately requiring a convoluted workaround of double projection to get what you want. That seems like something is missing - even if it's a "Not Supported" exception.

Maybe I'm misunderstanding something, but why wouldn't a projection using a class be able to use the same built-in comparer as an the anonymous type?

If you are doing the following, the parser is just going to have to guess at it using the default:
.Select(t => new { Id = t.Id }).Distinct()

Why wouldn't it be able to guess at the same way in the other case:
.Select(t => new DTO{ Id = t.Id }).Distinct()

Or even if there was an overload:
.Select(t => new DTO{ Id = t.Id }).Distinct(UseDefaultComparer: true)

[smitpatel]

Because DTO can override Equals method but anonymous types cannot.

[ajcvickers]

Closing old issue as this is no longer something we intend to implement.