Query Query Query – The Daily WTF

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).