Crap Code: Total Disclosure

Published 8/10/2011 by KDub in SQL

This post comes as part of a challenge from the community, namely Adam Machanic [Blog] who is hosting #21 this month, centered around the T-SQL Tuesday (#TSQL2SDAY) theme. The topic for T-SQL Tuesday #21 is Crap Code. Little snippets of code we use when we are in a hurry, don't have time to 'do it right', or just simply don't know how to do it and can't take the time to learn it. It is temporary, just make it work...for now. I will go back and fix it later.

SQLBlog/com
As Adam states, "As professionals, we must strive to rid ourselves of bad habits. And the only way to learn the difference is to see lots, and lots, and lots of examples." By accepting this challenge, hundreds of SQL bloggers will be exposing their crap code via independent blog posts. Once all the posts are tallied, there will be plenty of examples for us to avoid, thereby making this theme complete. So now, join me as I offer up one of my own examples for your coding displeasure.

I am a DBA for a large manufacturing company that uses Dynamics AX as their ERP. We have several developers constantly working in either the code or on reporting. This one dev is constantly cutting corners because 'he just needs to get it done now, quick and dirty'.  He's a dot Net guy, and not terribly compelling with SQL. Well, more often than not, I catch some of this quick and dirty before it makes it into my databases. (Some of you may know that AX allows the user to do just about anything to the database right through the GUI, including adding columns and even entire tables, without the DBA's knowledge or assistance) Control is nearly impossible. My expertise is usually only called upon when rescue is needed (as in "cleanup on aisle 3!").

On this particular day he was running some T-SQL to repopulate a 'custom' column in a native table that had somehow gotten populated with some NULL values and was screwing up his report (yes, he does reporting and has access to DML). And yes, it is a derived column, and yes I know about normalization. It's control I have a problem with (see above). For some strange reason, the database was denying him access to run his UPDATE statement in this DB (LOL) and he asked me to run it for him. He sent me the T-SQL in email, and this is what I got:

.

--- updates recs with correct producttype
UPDATE salesbookhist
SET producttype = (
		SELECT producttype
		FROM inventtable
		WHERE salesbookhist.itemid = inventtable.itemid
			AND inventtable.dataareaid = '004'
		)
WHERE (
		SELECT producttype
		FROM inventtable
		WHERE salesbookhist.itemid = inventtable.itemid
			AND inventtable.dataareaid = '004'
		) IS NOT NULL
GO


.

There are 1.82M rows of data in this table and he was trying to update one column. I asked him how long it ran on the DEV server and he replied, "about 3 hours". He did not believe me when I said it should take no more than a few minutes. I then proceeded to point out the flaw in his code. Again I was met with disbelief. I showed him some examples of simple JOIN clauses, but he remained adamant that his code was sound (I am not going to write it for him). I have seen similar code from him with nested SELECT statements going 5 and 6 levels deep. For those, I had mentioned using a CTE, just for readability (I hate trying to decipher someone else's crap code. The least they could do is make it readable). On this issue, however, he returned with a nice shiny CTE, but he said it still ran like a dog. Again, I suggested a simple JOIN.

An hour later he came back with a reasonably well composed statement that contained adequate JOIN syntax. It ran for 34 seconds.

I hope this post sheds some light on why we should never write (or allow in our databases) crap code.


Comments are closed