T-SQL & Development
I Know a Query’s Gonna Be Bad When… #TSQL2sday
I open it up and the first thing I see is a comment with a changelog showing a long list of dates and author initials.
Oh I know, you think documentation is a good thing, but:
- Somebody’s initials from ten years ago isn’t helping anything
- The big long list tells me nobody’s actually taking out the trash
- It tells me we probably don’t have change control or documentation where it actually matters
- It’s probably just the tip of the iceberg, and I’m going to find a submerged glacier of frozen code that has been stuck in time from 2008, and the business is afraid to change anything because “it, uh, works the way it is, for values of ‘works'”
- And somebody’s about to reference The Last Person™, and I’m gonna have to bust out the statute of limitations
We’ve probably been copy/pasting parts of it over the years, turning it into a giant Frankenstein body with one good arm and five copied version of it sprouting from various parts of his body. It’s turned into an octopus of subqueries where each subquery leg has an ever-so-slightly different WHERE clause. One looks for salespeople with local territories, another looks for regional territories, another for national territories, and yet another subquery for managers with multiple sales people under them, and the whole thing could be done in one set-based query if we could just step back and see the big picture.
When I see The Comment Changelog®, I know nobody sees the big picture.
They’re too focused on incremental copy/pastes, doing things the way they were done back in 2008, doing the bare minimum in order to avoid tipping the iceberg over and causing a tidal wave of problems.
When I see The Comment Changelog®, I get a little excited, because I’m also probably working with a staff that’s tired of dealing with the same old same old, a crew that’s also secretly excited to break out of old, inherited bad habits. They just need somebody to point out the obvious, and get buy-in from management that now is finally the time to start tackling the worst of the technical debt.
Starting with erasing that stupid changelog.
This blog post is brought to you by T-SQL Tuesday, and it’s Tuesday #200! This week’s topic was signs that a query is gonna be bad, and you’re probably going to see some posts today, and some posts next Tuesday, because as we all know, dates are hard.
Free, 3× a week
Get my new posts by email
Three posts a week, plus a Monday roundup of the best database news from around the web.
You can spare yourself the changelog if you use a code control.
True, but old habits die hard. I was at an engineering company and we had version control tools, but everyone was still adding entries in the change log header.
Even with version control, if dead code is not removed or just commented out, there is always code bloat.
We use a code-quality pipeline when checking in code which also checks for dead code (and other criteria).
Quite a task to do strategic organisation when you have daily work at your hands…
I looooove that kind of pipeline. Are you using a commercial tool for it, or something y’all whipped up in Github Actions?
Oh, that’s easy. I saw a script to defrag and it doesn’t check for page count, sets the reorg at 10% and the rebuild to 30%. Most of the tables are under 2000 pages. It’s apparently run by users at random times. It’s also from 2013.
They are complaining about blocking. Did I mention it’s a NOLOCK application? You will not find a script or procedure lacking the tender mercies of NOLOCK.
I’m shocked.
That’s a classic case of outdated, one-size-fits-all maintenance scripts causing more harm than good. The person who built the script a long time ago had good intentions, but … yeah no.
Definitely agree – DELETE IT.
Your
ever-so-slightly different WHERE clauseproposition triggers me. Would be nice to have some sort ofstored predicatefeature in the engine. Something in the middle of TVFs, views, CTEs, without needing to give up and having totally dynamic SQL that complicates static analysis. Though this is a lot easier with libraries at the application level.Here I was, having a nice Tuesday, and I get a notification with a horse-sized hard-to-swallow pill.
Brent, I feel like I just got blindsided with the Skeletor meme: “Comment blocks to track changes are useless! Until we meet again!”
Part of me does/enforces them because that’s how I learned 15 years ago, but the other part of me does them because I want my team to be intentional about the “why.” I leave spaces for Jira ticket numbers as well so we can track things and make the changes justified.
But I’m realizing the problem is lack of source/version control as you state (and an antiquated mindset, among other things).
Thanks Brent, I’m going to make it a point to evaluate other options for accomplishing the same thing. I appreciate this very necessary post!
“Are we the baddies?” 😀
HAHAHA I dunno about you, but the skull on my cap is just a Paul Frank logo!
I think there are extreme cases on both ends. Consider Joel Spolsky’s argument against “clean slate” rewwrites. https://www.joelonsoftware.com/2000/04/06/things-you-should-never-do-part-i/
I use the comment changelog in the SQL Server object definition to document the fixes for those edge cases, including ticket number for traceability. Should I actually be using SQL scripts in source code control to manage changes and put the documentation in check-in comments instead? Maybe. But then I can’t quickly fix a sproc with a simple call to sp_helptext.
“Quickly fix” can be a synonym for “quickly break” though, be careful. 😉
[…] Brent Ozar knows when a query is going to be bad: […]
[…] I groan when I see a text changelog at the top of a stored procedure or function, listing a bunch of initials and dates going back to the Frutiger Aero era. […]
Belt and suspenders. We use Azure DevOps, but as a performance tuner I spend my time in Redgate Monitor and SSMS, and it is a PITA to hunt down a modification history in ADO, so we put the work item number and a brief description of the change in a header comment block as well as in the Pull Request. Given frequent releases, it makes it easy to see which changes have made it to production and which have not just by looking in SSMS. It saves time, at the risk of the two sources getting out of sync.