CodeSOD: Query Query Query

Bob's employer had a data-driven application which wasn't performing terribly well. They had some in-house database administrators, but their skills were more "keep things running," and less "do deep optimizations". The company opted to hire a contract DBA to come in, address the performance problems, and leave.

In actual fact, the DBA came in, ran some monitoring, and then simply wrote some guidance- generic, and frankly useless guidance. "Index on frequently queried fields," and "ensure database statistics are gathered on the appropriate schedule."

The only meaningful output of the contractor's visit was a list of the badly performing stored procedures. Bob was given the list and told, "Go fix these."

One stored procedure needed to populate variables with data from the order table. Specifically, it needed to collect sender_id, user_group, and file_ref from the orders table. Here's how it did that:

if (@origin = -2 or @origin = -4 or @origin = -5 or @origin = -6 or @origin = -7 or @origin = -8 or @origin = -9)begin select @id = sender_id from order where ref = @mur select @group = user_group from order where ref = @mur if (@ltaId is null) begin select @ltaId = null end select @file_ref = file_ref from order where ref = @mur select @ref = ref from order where mur = @murendelse begin select @id = sender_id from order where mur = @mur select @group = user_group from order where mur = @mur if (@ltaId is null) begin select @ltaId = null end select @file_ref = file_ref from order where mur = @mur select @ref = ref from order where mur = @murend

Let's start with the if condition. We never love magic numbers, but that's hardly a WTF. The real problem is that the two branches differ only in whitespace. There's no reason for the if. Perhaps once upon a time there was, but there isn't now.

Now, all the fields we need are in the table order, they're all selected by the field mur which is a unique key, but for some reason, this code needs to run the query three times to get its data. Also, mur is a unique key in the domain but not in the database- no unique constraint or index was applied, which is part of the reason performance was as bad as it was.

But the real highlight, for me, is how @ltaId gets set. If it's null, set it equal to null. That's a ^chef's kiss^ right there. Beautiful(ly horrible).

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!