The method ‘Skip’ is only supported for sorted input in LINQ to Entities. The method ‘OrderBy’ must be called before the method ‘Skip’

Question

Using Entity Framework 6.0.2 and .NET 4.5.1 in Visual Studio 2013 Update 1 with a DbContext connected to SQL Server:

I have a long chain of filters I am applying to a query based on the caller's desired results. Everything was fine until I needed to add paging. Here's a glimpse:

IQueryable<ProviderWithDistance> results = (from pl in db.ProviderLocations
                                            let distance = pl.Location.Geocode.Distance(_geo)
                                            where pl.Location.Geocode.IsEmpty == false
                                            where distance <= radius * 1609.344
                                            orderby distance
                                            select new ProviderWithDistance() { Provider = pl.Provider, Distance = Math.Round((double)(distance / 1609.344), 1) }).Distinct();

if (gender != null)
{
    results = results.Where(p => p.Provider.Gender == (gender.ToUpper() == "M" ? Gender.Male : Gender.Female));
}

if (type != null)
{
    int providerType;
    if (int.TryParse(type, out providerType))
        results = results.Where(p => p.Provider.ProviderType.Id == providerType);
}

if (newpatients != null && newpatients == true)
{
    results = results.Where(p => p.Provider.ProviderLocations.Any(pl => pl.AcceptingNewPatients == null || pl.AcceptingNewPatients == AcceptingNewPatients.Yes));
}

if (string.IsNullOrEmpty(specialties) == false)
{
    List<int> _ids = specialties.Split(',').Select(int.Parse).ToList();

    results = results.Where(p => p.Provider.Specialties.Any(x => _ids.Contains(x.Id)));
}

if (string.IsNullOrEmpty(degrees) == false)
{
    List<int> _ids = specialties.Split(',').Select(int.Parse).ToList();

    results = results.Where(p => p.Provider.Degrees.Any(x => _ids.Contains(x.Id)));
}

if (string.IsNullOrEmpty(languages) == false)
{
    List<int> _ids = specialties.Split(',').Select(int.Parse).ToList();

    results = results.Where(p => p.Provider.Languages.Any(x => _ids.Contains(x.Id)));
}

if (string.IsNullOrEmpty(keyword) == false)
{
    results = results.Where(p =>
        (p.Provider.FirstName + " " + p.Provider.LastName).Contains(keyword));
}

Here's the paging I added to the bottom (skip and max are just int parameters):

if (skip > 0)
    results = results.Skip(skip);

results = results.Take(max);

return new ProviderWithDistanceDto { Locations = results.AsEnumerable() };

Now for my question(s):

1. As you can see, I am doing an orderby in the initial LINQ query, so why is it complaining that I need to do an OrderBy before doing a Skip (I thought I was?)...

2. I was under the assumption that it won't be turned into a SQL query and executed until I enumerate the results, which is why I wait until the last line to return the results AsEnumerable(). Is that the correct approach?

3. If I have to enumerate the results before doing Skip and Take how will that affect performance? Obviously I'd like to have SQL Server do the heavy lifting and return only the requested results. Or does it not matter (or have I got it wrong)?

Answer 1

I am doing an orderby in the initial LINQ query, so why is it complaining that I need to do an OrderBy before doing a Skip (I thought I was?)

Your result starts off correctly as an ordered queryable: the type returned from the query on the first line is IOrderedQueryable<ProviderWithDistance>, because you have an order by clause. However, adding a Where on top of it makes your query an ordinary IQueryable<ProviderWithDistance> again, causing the problem that you see down the road. Logically, that's the same thing, but the structure of the query definition in memory implies otherwise.

To fix this, remove the order by in the original query, and add it right before you are ready for the paging, like this:

    ...
    if (string.IsNullOrEmpty(languages) == false)
        ...
    if (string.IsNullOrEmpty(keyword) == false)
        ...
    result = result.OrderBy(r => r.distance);

As long as ordering is the last operation, this should fix the runtime problem.

I was under the assumption that it won't be turned into a SQL query and executed until I enumerate the results, which is why I wait until the last line to return the results AsEnumerable(). Is that the correct approach?

Yes, that is the correct approach. You want your RDBMS to do as much work as possible, because doing paging in memory defeats the purpose of paging in the first place.

If I have to enumerate the results before doing Skip and Take how will that affect performance?

It would kill the performance, because your system would need to move around a lot more data than it did before you added paging.