ODataQueryOptions do not correctly apply $filter and $orderby Options to IQueryable when using EntityFramework 6 with SQL Server

This blog post is a bit more lengthly than my usual posts and the idea came from an issue I faced when working in one of our project where we use an OData v3 REST interface that talks to an MS SQL Server (2012 R2) via EntityFramework v6.1.3.

In short: we wanted to implement ‘entity framework row level security’ (in a different approach than described in Web app with a multi-tenant database using Entity Framework and Row-Level Security) and realised that the ODataQueryOptions were not applied to the underlying SQL query but instead were only filtered by the framework when returning the result set. In our scenario this had a heavy impact, as we were traversing the resulting entity set effectively doing a full table scan on every query. A simple request https://www.example.com/baseurl/Endpoint/Jobs?$filter=startswith(Name , 'A') therefore took several seconds instead of microseconds.

This post is structured in the following sections:

The Setup

The situation started with this simple controller method for a Get* method that queries a SQL table Node via:

~/Nodes()?$filter=Name ne 'Root%20Node'&$orderby=Modified, Created desc, Id asc&$skip=5&$top=35`
  1. We have a model Node.cs
  2. … that is queried by the controller GetNodes in NodesController.cs
  3. We issue a GET request against this controller and see its response in HttpRequestResponse.js
  4. The internal SQL query is shown in SqlCommand.sql revealed via the IDbCommandInterceptor interface enabled in web.config

Here we see that the query options from the ODATA query are applied to the SQL table via three nested SQL SELECT statements. The outer SELECT adds a TOP 16 (which is page size plus 1) to determine if an OData NextLink should be added to the HTTP response. In fact this is a bit redundant as the inner SELECT statement utilises the OFFSET option to apply both the $skip and the $top options to the query. And for whatever reason the $orderby clause is applied twice via SQL ORDER BY in the inner and outer command. In addition, it is interesting to see, that the $top constraint is applied to the inner SELECT statement, though the outer statement reduces the amount to the defined page size (which is smaller). So in a worst case scenario a client performs a query which $top set to a huge value that is later restricted to a much smaller page size.

In the HTTP response you can see the odata.nextLink which includes the original query options plus the adjusted $skip and $top options. For an unknown reason the $top option is set to 20 though our defined PageSize is set to 15.

Note1: Just in case you wonder what the @DynamicFilterParam_1 param means. This is due to the fact that we utilise EntityFramework.DynamicFilters to implement tenant filtering in our database tables.

Note2: The outer SELECT statement is actually inserted due to the EnableConstantParameterization property which is by default set to true.

The Problem

So far, so good. However, when we implemented a row level or entity level check things got a little bit messy.

  1. The model stays the same (actually through the course of this blog post)
  2. The controller NodesController.cs now performs some kind of permission check based on the entity …
  3. … and the result is just as expected with 15 entities and an adjusted $skip option (and $top still wrong, but that does not matter).
  4. however, the underlying SQL statement reveals that it actually performs a full table scan SELECT * FROM Node and the filtering and ordering are performed when returning via Ok<IEnumerable>(entitySet).

You can imagine that first performing a permission check on every entity and secondly filtering the actual results is far from desired. And there is side effect that is currently not observable, but we will come to that later.

Note: for illustration purpose in this example our ‘permission check’ only returns entities Id % 2 == 0.

First Approach

Luckily, we have some kind of queryOptions passed to our controller method that have an ApplyTo method that should help us to transform the $orderby and $filter options into SQL statements.

  1. The model stays the same
  2. This time we use ApplyTo to apply $orderby, $filter, $top and $skip
  3. … and get a result back
  4. The underlying SQL statement shows that the options are really applied at the database level

This looks good at first sight. However, looking closer we see a couple of potential problems:

  1. The OData NextLink is missing
  2. The response only returns 13 entities (instead of requested 35 or PageSize (which is 15) entities. The page size is actually totally ignored
  3. The first two entities (with Id 19 and 21) which we got returned from the the previous query are skipped from this response
  4. The inner SQL SELECT does restrict the query to the $skip and $top option (I am not a SQL expert nor did I research on that; so maybe this is not a performance impact)

Of course, our permission check ‘cut away’ some records from the result set. This was not visible in the previous example as we first accidentally performed an implicit full table scan (SELECT *) and then performed the permission check. And as we end up with a result set smaller than the page size, OData certainly does not include a next link.
This is somehow understandable, but the real problem stems from fact the we miss two valid entities. When debugging the code, we see that our permission actually does process the missing records. This effectively means that the return statement Ok<IEnumerable>(entitySet) skips the 5 records which is not our intention. If we had selected a smaller $top count we would have got returned even fewer records. Furthermore the $skip count in the nextlink is wrong as it does not take the skipped records from the permission check into calculation.

The Solution

To actually perform a query to the database that actually reads until the pagesize (+1) is reached we need a modified ApplyTo method and a SetNextLink method that takes any skipped records into account.

  1. The model stays the same
  2. The controller performs a query via our new PageableEntityFilter method. This method does all the heavy lifting and invokes our permission check as a Func that either returns true or false to indicate if an entity should be returned to the caller.
  3. This time our response returns us records with Id starting at 18 and generates a $skip option of 41 (and the $top option is also corrected). Id 18 is actually correct, as the entity set of Nodes has Ids starting at 8. As according to our permission filter every even entity is to be included in the result set and we specified to skip 5 entities (8, 10, 12, 14, 16) the first entity to be returned is 18.
  4. This time we get two SQL statements showing an increased skip OFFSET of 15 and 31

While implementing this method I ran into a couple of problems:

  1. ODataQueryOptions.Skip and ODataQueryOptions.Top are readonly properties with Getters only resulting in having these options applied multiple times when re-querying the database table
  2. ODataQueryOptions does not have a constructor that can be used to manually initialise query options with a custom Skip or Top
  3. SkipOptions and TopOptions itself are readonly as well
  4. Secifying an SQL OFFSET requires to have an ORDER BY clause as well, meaning that we have to specify an orderby if there is none specified by the caller
  5. ODataQueryOptions.ApplyTo() incorrectly applies its $orderby clause to the outer SQL statement only resulting in a wrongly ordered result set
  6. Converting the OrderByClause expression into an IQueryable compatible expression does not seem to be possible
  7. ODataQueryOptions.ApplyTo() works with typeless IQueryable that must be cast to a specific entity to be returned by OData

The Details

To be honest this approach might come as some kind of unusual solution as it uses some internals that are not for the faint of heart:

  1. We write ‘readonly’ properties via reflection and its internal backing fields, see SetTopOption and SetSkipOption
  2. We have to manually recreate an OrderByClause by analysing all cascaded clauses, ApplyOrderedTo
  3. We have to create a custom ‘nextlink’ via an ODataController extension method

Though this solution does have its downsides, in our opinion its advantages outweigh its disadvantages. The original approach with a full table scan does just not perform. In addition we created a very flexible solution that acts as a pageable entity filter that can perform just about anything deemed feasible.

Note1: in case you wonder why we use implementation classes instead of static extension classes: we think for testing purposes it makes more sense to use a facade class

Note2: the PageableEntityFilterContext is a convenience for the filter method allowing the filter to create commonly used classes once and pass them to the actual filter (AccessManager in our example).

Note3: When not manually re-creating the OrderByClause, but just trying to use ApplyTo on an IQueryable with Skip() you receive an System.NotSupportedException in OrderByLifter.PassthroughOrderByLifter.Skip.

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

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: