-1

I have a paging class that includes an ApplyFilter() method, which filters rows for the current page. Since paging generally only makes sense in sorted results, this method accepts and returns an IOrderedQueryable<T> (instead of just a IQueryable<T>).

I use it like this:

var query = DbContext.Items
            .Where(i => i.Value > 0)
            .OrderBy(i => i.SortColumn);

query = pagination.ApplyFilter(query);

But there's a problem with my ApplyFilter() method.

public IOrderedQueryable<T> ApplyFilter<T>(IOrderedQueryable<T> query)
{
    return query.Skip((CurrentPage - 1) * PageSize).Take(PageSize);
}

The Skip() and Take() methods both return an IQueryable<T>, so I get an error that the return value cannot be converted from IQueryable<T> to IOrderedQueryable<T>.

How can I use Skip() and Take() and still have an IOrderedQueryable<T>?

Jonathan Wood
  • 65,341
  • 71
  • 269
  • 466
  • 1
    The query has no order to apply a `ThenBy` to. It doesn't make sense for it to be an IOrderedQueryable. The whole point of that interface existing is to ensure `ThenBy` is only usable in direct response to an `OrderBy` call. – Servy Dec 16 '21 at 01:12
  • @Servy: Yes, but it would be a nicety. You can see from my code that calls it that an overload the also returns `IOrderedQueryable` would be convenient. – Jonathan Wood Dec 16 '21 at 01:16
  • 2
    The downsides of attempting to claim a query is ordered when it's not seems to *significantly* outweigh the costs of simply calling `AsQueryable` on an ordered query that no longer needs to use a `ThenBy` (or simply using a new variable, as is usually the preferable option, variables aren't expensive to use). Use the correct solution for the problem at hand. Treating an ordered query as a general query is fine, treating a non-ordered query as if it's ordered is just causing trouble for yourself. – Servy Dec 16 '21 at 01:19
  • @Servy: Great points. Too bad `AsQueryable()` has to allocate additional objects. I don't think it's minimal overhead. – Jonathan Wood Dec 16 '21 at 16:27
  • What objects you you think it's going to allocate? It has *substantially* less overhead than any possible implementation you could ever conceivably come up with to turn a query into an ordered queryable (which could never possibly avoid any allocations in code that wouldn't throw at runtime). `AsQueryable` does nothing other than change the compile time type of the object. – Servy Dec 16 '21 at 16:42
  • @Servy: Well, I'm not exactly clear on all the details but, looking at the source code for `AsQueryable()`, I do see a call to `Activator.CreateInstance`. – Jonathan Wood Dec 16 '21 at 16:47
  • It just does a type check and a cast. https://referencesource.microsoft.com/#System.Core/System/Linq/IQueryable.cs,080010ec4b6beced If you're writing code so performance sensitive that the type check is going to be costly to you (which is going to be literally never since you're writing a database query that will be many, many orders of magnitude longer) you could write out the cast by hand rather than using `AsQueryable` to save the type check. The JITter might even optimize it out, but I wouldn't consider it worth checking given how much of a non-issue it is. – Servy Dec 16 '21 at 16:54
  • @Servy: Yeah, you're right. It's already an `IQueryable` so the type cast is sufficient. – Jonathan Wood Dec 16 '21 at 17:53

1 Answers1

0

You could do something cheesy, like

public IOrderedQueryable<T> ApplyFilter<T>(IOrderedQueryable<T> query)
{
    return query.Skip((CurrentPage - 1) * PageSize).Take(PageSize).OrderBy(i=>1);
}

At least for SQL Server the codegen was smart enough to ignore the OrderBy.

Or change the return type to IQueryable<T>, and don't reuse the query variable:

var q = db.Set<User>().OrderBy(u => u.id);
var q2 = ApplyFilter(q);

Or change the argument and return type to IQueryable<T> and throw an ArgumentException if someone passes one that isn't an IOrderedQueryable<T>

public IQueryable<T> ApplyFilter<T>(IQueryable<T> query)
{
    if (query is IOrderedQueryable<T> oq )
    {
        return oq.Skip((CurrentPage - 1) * PageSize).Take(PageSize);
    }
    else
    {
        throw new ArgumentException("Query must be IOrderdQueryable<T>");
    }
}
David Browne - Microsoft
  • 80,331
  • 6
  • 39
  • 67
  • 1
    Yeah, that hack is rather unsightly. Too bad you can't simply convert an `IQueryable` to an `IOrderedQueryable` somehow. (Without appending another operation.) – Jonathan Wood Dec 16 '21 at 01:17