December 12, 2007 at 3:18 pm
I've been doing some consulting work for a client, and I've been looking at some of the queries written by some of the more junior developers (not in age nor experience).
I've included a couple of what I call "DOH!" queries.("DOH!" being a euphimism from "The Simpsons").
what is truly frightening is that this is PRODUCTION CODE !
DECLARE @NumberofTrans int
...
SET @NumberofTrans = '0'
DOH!
OK, ok, sure it works. But.....:w00t:
This next one really got me......
declare @emailstr varchar(500)
declare @emaillast varchar(50)
declare @emailtmp varchar(50)
DECLARE @NumberofTrans int
declare @msg varchar (200)
Select top 1 @emaillast = email
From SomeTable
Where TransactionRptActive = 'Y' order by email
set @emailstr = @emaillast
set @emailtmp = ''
while @emaillast > @emailtmp begin
set @emailtmp = @emaillast
Select top 1 @emaillast = email From SomeTable
Where TransactionRptActive = 'Y' and email > @emailtmp order by email
if @emaillast <> @emailtmp
set @emailstr = @emailstr + ';' + @emaillast
end
DOH!
OK, ok, sure it works too. But.....:w00t:, how aobut this instead ?
declare @EmailList varchar(max)
set @EmailList = ''
select @EmailList = @EmailList+email+';' From SomeTable
Where TransactionRptActive = 'Y' and Email is not null
group by email order by Email
As it turns out, this bits of inefficient tsql are just on the surface. And, I could show you things much worse I have found at this company.
It would seem to me that there are some developers out there who like to use their SQL server clusters to both accomplish work and heat up the room by using extra cpu cycles to accomplish the job.
But, on the other hand, just one floor above the people who write the code above, is a group that writes some of the tightest and most efficient code I've ever seen.(stored procedures and table functions that are just too cool).
It's never black and white, always shades of grey.
December 13, 2007 at 1:08 am
One of the best I saw at a previous company.
SELECT ... FROM SomeTable WHERE SomeColumn = @Var and Status = 1 or status = 2
Just like that, no brackets. Even better, Status was a bit column...
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
December 13, 2007 at 6:17 am
We found one in production earlier this year that looked a bit like this:
DECLARE @var int
SET @var = 0
IF @var > 0
BEGIN
...do some work
END
return 0
And they had honestly spent a couple of weeks trying to troubleshoot the issue before they brought the dba's on board.:w00t:
"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
- Theodore Roosevelt
Author of:
SQL Server Execution Plans
SQL Server Query Performance Tuning
December 13, 2007 at 7:17 am
A whole lot more subtle than the obvious candidates for the "SQL Darwin Award" that you guys have shown, is my favorite clock-cyle waster/index defeater...
[font="Courier New"]AND ISNULL(somecolumn,' ') <> ' '[/font]
In fact, it's one of the interview questions I use... I write it on the white-board and ask "What's wrong with this picture and how would you rewrite it?" Then I hand them the marker...
Does two things... demonstrates their knowledge of NULLs and gives me a feel for how well they can communicate.
--Jeff Moden
Change is inevitable... Change for the better is not.
December 13, 2007 at 8:02 am
I vote for Grant's. That's beautiful.
December 13, 2007 at 8:27 am
The really beautiful thing about it was that the SET @VAR = 0 was directly butt up against the IF statement. It wasn't like there was 15 pages of intervening code or something.
BTW, speaking of butts, for fun stories swing by sqldumbass.com.
"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
- Theodore Roosevelt
Author of:
SQL Server Execution Plans
SQL Server Query Performance Tuning
December 13, 2007 at 8:32 am
LOL. There is some comfort in knowing that I'm not the only one gets upset about this particular genre of queries.
It brings up a more serious question. Is it the function of the DBA to review any and all objects that are put on the servers they maintain ? I tend to believe that it is. If I am responsible for maintaining that server, I sure as hades would want to know that I've got objects like that burning up CPU cycles and potentially locking out others.
About my second one, as you can tell, they are generating an email list that ultimately ends up in the recipients parameter of xp_sendmail. Although I preserved their "order by" in my revised version, I still cannot fathom why the "order by". I mean, does it really matter if the list of Email addresses are sorted or not ? I suppose that one is above my head. I suppose this is what is meant by the phrase, "A-list".
December 13, 2007 at 8:38 am
The order by is a serious puzzler. If the performance was OK, I'd probably ignore it with a shrug, but if performance was in any way an issue, it sure demands a good explanation.
"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
- Theodore Roosevelt
Author of:
SQL Server Execution Plans
SQL Server Query Performance Tuning
December 13, 2007 at 10:16 am
Grant, that's seriously impressive. How anyone can debug that for more than 2 min, I don't know.
Seeing Grant's reminded me of this little time bomb I found a couple years back. It was the first time I looked at code and exclaimed out loud "What the F..."
Virtual cookie to the first person who spots the problem with this.
For info, @somestring was declared as Varchar(8000) and was built up from several fields of one row of a table. Then this little gem.
.. Do stuff
[font="Courier New"]WHILE LEN(@SomeString)<7500
SET @SomeString = @SomeString + ' '
[/font]
.. Do more stuff
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
December 13, 2007 at 10:52 am
Oh this is great... I've seen so many good examples. The lack of brackets when you're throwing around OR statements sets my teeth on edge. The ISNULL() failure is also excellent, Jeff.
I ran into this one a few years back while reviewing a series of stored procedures a predecessor had left behind.
...
WHERE LEN(district_code) > 20
AND district_code = '28010'
or my other favorite I found...
...
WHERE LEN(filter1) > 20
When the field length of filter1 is 15... nice...
That along with multiple nested IN statements, crazy temp table usage, and the general assumption of "Well it returns the right answer so it must be good" gives me headaches.
December 13, 2007 at 10:56 am
GilaMonster (12/13/2007)
Virtual cookie to the first person who spots the problem with this.For info, @somestring was declared as Varchar(8000) and was built up from several fields of one row of a table. Then this little gem.
.. Do stuff
WHILE LEN(@SomeString)<7500
SET @SomeString = @SomeString + ' '
.. Do more stuff
I could imagine the developer screaming at the DBA about how the system was so slow it would never return an answer. The misuse of CHAR/VARCHAR is horrendous. :rolleyes:
December 13, 2007 at 11:26 am
Fortunately for my server (I found that in production), the data was such that the resulting string always was >7600 characters by that point.
Char/varchar aside, what's wrong with the SPACE function? Not to mention the lack of understanding of what LEN does
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
December 13, 2007 at 11:28 am
SELECT Count(*)
FROM TSQLCoders
WHERE FamiliarWithLEN = 1
AND FamiliarWithDATALENGTH = 0
Result:
Too bloody many
December 13, 2007 at 11:31 am
Aaron Ingold (12/13/2007)
That along with multiple nested IN statements, crazy temp table usage, and the general assumption of "Well it returns the right answer so it must be good" gives me headaches.
At least you get users that seem to check that it's the "right" answers. Most of the time I get - "well it was giving us data back so we figured it was okay". Most of the same words are there, but boy - what a difference in meaning:)
I ran into this gem about 18 months ago...
select id, et....
from tableA
where id in
(select ID from tableA
--where field2>2
)
...Let's just say it had been running that way for a while....
[/code]
----------------------------------------------------------------------------------
Your lack of planning does not constitute an emergency on my part...unless you're my manager...or a director and above...or a really loud-spoken end-user..All right - what was my emergency again?
December 13, 2007 at 11:41 am
There are T-SQL people who know & use the SQL built in functions.
And then, there are "The Others" ...
Code below called many times per minute on 1 of my servers. I'll let you enjoy this standalone and won't bore you with the code that actually calls the function, which just happens to be inside nested cursors:
CREATE FUNCTION dbo.FN_GetLastDayByMonth (@monthstring varchar(7))
RETURNS tinyint AS
BEGIN
DECLARE @monthday tinyint
DECLARE @Yearno smallint
DECLARE @temint smallint
DECLARE @monthno varchar(2)
DECLARE @temstr varchar(1)
-- Get total number of days in a Month
SET @monthno = RIGHT(@monthstring, 2)
SET @temstr = LEFT(@monthno, 1)
IF (@temstr='-' OR @temstr='/')
BEGIN
SET @monthno = '0' + RIGHT(@monthno, 1)
END
IF (@monthno='01' OR @monthno='03' OR @monthno='05' OR @monthno='07' OR @monthno='08' OR @monthno='10' OR @monthno='12')
SET @monthday = 31
ELSE IF (@monthno='04' OR @monthno='06' OR @monthno='09' OR @monthno='11')
SET @monthday = 30
ELSE IF (@monthno='02')
BEGIN
SET @Yearno = LEFT(@monthstring, 4)
SET @temint = @Yearno % 4
IF (@temint=0)
SET @monthday = 29
ELSE
SET @monthday = 28
END
RETURN @monthday
END
Viewing 15 posts - 1 through 15 (of 21 total)
You must be logged in to reply to this topic. Login to reply