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 = @mur
end
else
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 = @mur
end
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).