So while I was on vacation last week, I saw Brent Ozar’s invitation post for the 200th episode of T-SQL Tuesday. And fittingly this was a T-SQL topic. No need to bring out any of my gushy feelings about past events or mistakes or whatever. Just “When I’m looking at a query, I bet it’s bad if I see.” Well, that is unfortunately too easy.

I came back from that vacation (and another trip after to the Music City Hot Chicken Festival) with a head full of guck. (Not actually a word, but you all know what I mean.) Brent was early in his invite (Last Tuesday was the 30th of June), so I didn’t realize they were due today, at least not until I saw Hugo’s post, and then went back to read the invitation in full. This is probably the easiest topic ever, so here goes.

In the invitation, Brent notes to direct this to someone like yourself back when you were starting. All of these things are things I have done, and some probably will do again when I am not taking a proper amount of time to do a task.

My list

I still feel like garbage, so I decided just a simple list would do. I will also preface this by saying each item could include “without a coherent comment.”

Everything on this list fills me with dread unless I read someone say: “Such and such was needed because the optimizer wouldn’t….” and then I at least know why they believed needed it.

DISTINCT – I have written about this not that long ago, and it has been one of those things that has been an issue forever. DISTINCT where someone hasn’t explained or begged forgiveness for it’s use is the easiest signal of them all that something is likely wrong.

Layers, lots of layers – You know what else has layer Shrek, onions have layers. Like everything on my list, none of this means there is anything wrong with the code. It just makes me worry about what I am about to try to read.

Layers are the worst, because often they aren’t needed. There are scads of posts on things like using a CTE to filter early that don’t make sense once you understand how things work.

• CTEs – I love CTEs, I really do. They are one of my favorite things added back in SQL Server 2005 and I got to write a chapter in Pro SQL Server 2005 get it now for 13 bucks!). They allow us to solve a lot of interesting problems in one statement (which gives the optimizer a better chance to create a plan.
  
  But just like views, when you layer them deeper and deeper, they become unwieldy and really hard to debug (especially when they get so complex you get terrible execution plans!)
• Temp tables replacing CTEs – Just like all items on my list, there is nothing specifically wrong with this. In fact, one way to fix a CTE laden query that isn’t performing well is to materialize the results since SQL Server doesn’t have a way to materialize the results itself. There are plenty of cases where this is necessary. And it every one of them, the person with skill to do this would also have skill enough to explain WHY they had to do it. Otherwise you are left wondering “why?” and that always leads to a lot of work deciding what was necessary.

Wobbly naming standards – Standards have always been a thing for me because otherwise I am a mess. So I immediately notice things mixing Pascal with snake case. But the biggest one are table aliases. Things like FROM Schema.ComplexTableName AS a (and of course there are multiple tables joined together. SCTN is better, but I often just use the entire name of the table/view unless it is a really long name.

Isolation levels and query hints – Against them? Not at all. There is so much you can do with these to make your data quality better. READUNCOMMITTED/NOLOCK? Good for some things. SERIALIZABLE? Essential tool for managing concurrency. But if you haven’t explained WHY you freaking put NOLOCK in your query, I 100% know that something is wrotten. (The w makes it much worse!)

I guess it all boils down to comments

Nothing I put in here are things I would never use. I use the tools that get the results the easiest/fastest and most importantly correct. And all the stuff in my list, even including NOLOCK has use (okay, except for aliases a, b, c, d, perhaps). Thing is, they are all generally anti-patterns that I know are instinctively poor practice without some special reason. So one of two things occurs.

1. There is a comment and I can see why the original writer (sometimes me, as I never claimed to be perfect!) had in mind. I can then decide if it is needed as I am working on my change to the code.
   
2. There is no comment and I have to imagine why you added something so regularly heinous as DISTINCT to your query. Were you fixing some bad data that you couldn’t correct in the source? Or do you have an error in a join 3 temp tables and CTEs ago and I need to go figure out how and why that occurs (since you just know that is the problem that you have been sent to correct!)

So anytime you feel yourself doing something “interesting” in your code, add a little comment.

-- SERIALIZABLE this code can't handle changes to the source 
-- data once read due to blah blah blah

And at least then I can question the intent you had, not just know that this is going to take twenty times longer than I expected.