Using an ORM tool is no excuse to write bad queries, but it still happens

The other day while working on a pet project I noticed that the TSQL being generated by the Entity Framework was not issuing the correct queries to the database.  I was expecting the sorting to be done in the database, not the web server.  This caught me by surprise, because I had assumed that EF4 was smart enough to issue the proper query to do a server side sort.

As it turns out, EF4 (and LINQ to SQL) are smart enough to issue the right queries.  It was the engineer who wrote the wrong code.  So I need to pay closer attention, or stop writing data access code in the wee hours of the morning.

So here are two examples of code that will NOT do sorting in the database, but will do the sorting in the web server…

// this does sorting on the client, and it only sorts on DateCreated...
var query = repository.Find(it => it.CardId == viewModel.CardId)
    .OrderBy(it => it.DisplayIndex)
    .OrderByDescending(it => it.DateCreated);

// this also does the sorting on the client, and it only sorts on date created
viewModel.ViewItems = query.Select(it => Mapper.Map(it))
    .OrderBy(it => it.DisplayIndex)
    .OrderByDescending(it => it.DateCreated);

 

After some research I learned that the samples above sort by “DisplayIndex” and then re-sort the whole collection by “DateCreated” descending.  This is not at all what I wanted, in-fact, it was double fail and I should fire myself.

Basically, I wanted to execute this TSQL…

SELECT *
    FROM dbo.Exercise5
    WHERE CardId = 172
    ORDER BY DisplayIndex,
        DateModified DESC

 

After further research I learned that I could use either prototype below to accomplish the same task…

// this gives the expected database execution plan...
var query2 = from items in repository.Fetch() 
                where items.CardId == viewModel.CardId 
                orderby items.DisplayIndex, items.DateCreated descending 
                select items;

// this also gives the expected database execution plan
var query3 = repository.Fetch()
    .Where(it => it.CardId == viewModel.CardId)
    .OrderBy(it => it.DisplayIndex)
    .ThenByDescending(it => it.DateCreated);

 

Would more testing have found this?

A friend and I have been discussing whether or not code like this should be unit tested.  Some would argue yes because of the reasons that I have outlined above, and others would say no because its very hard to mock LINQ expressions, and that integration testing would catch this sort of thing.  Maybe a fitnesse test would, but that is a whole other can of worms.

I am not convinced that integration testing would catch this sort of thing.  In the next sample, we get the “correct” results, however, they are incorrect because the sorting is not done by the database server.  Sometimes, you just have to put down the automation tools and look under the hood. 

// this does sorting on the client
var query = repository.Find(it => it.CardId == viewModel.CardId)
    .OrderBy(it => it.DisplayIndex)
    .ThenByDescending(it => it.DateCreated);

 

IMO, one should favor sorting on the database server, as this could be more efficient.  In reality, until the result set gets reasonably large or the table becomes fairly populated with data, we are probably not going to see a noticeable difference in performance, so maybe its best left alone until there is an issue (if ever).

The moral of the story is to pay attention to the code that you write and also to verify that the code is actually doing what it should be doing.