October 9, 2018 at 11:49 am
We all know there are plenty of recommendations as far as what to avoid when writing T-SQL. For example, it's oftentimes not a good idea to use functions in your WHERE clause because they'll be called for every row and the query engine can't use any indexes that may be on the column(s). Another practice that's not 'wrong' but you'd generally want to avoid is using non-ANSI joins (join in the WHERE clause) instead of ANSI 92 joins (e.g., INNER JOIN).
So these aforementioned techniques have a purpose but they're just not the best options. I'm curious to know what are some really dumb things you can do that have literally no purpose--so if you saw them, you'd know instantly that the person who wrote them clearly didn't understand what he/she was doing:
I'll start.
- Using DISTINCT in an IN clause. I've seen it before and it's completely unnecessary.
- LEFT joining two tables together and never referencing any columns of the RIGHT table.
Whether you've seen these types of things in your experience or can just think of some off the top of your head, I'd love to hear them.
Mike
Mike Scalise, PMP
https://www.michaelscalise.com
October 9, 2018 at 12:03 pm
You mean like the far too common create SQL Wide open to injection (my biggest pet hate):
[Code]CREATE PROC SomeSP @TableName varchar(8000) AS
DECLARE @s-2 = 'SELECT * FROM ' +@TableName;
EXEC (@S);
[/code]
Or the classic "Innie Outie" JOIN:SELECT {COLUMNS}
FROM Table1 T1
LEFT JOIN Table2 T2 ON T1.ID = T2.ID
WHERE T2.SomeColumn =2;
Thom~
Excuse my typos and sometimes awful grammar. My fingers work faster than my brain does.
Larnu.uk
October 9, 2018 at 12:40 pm
NOLOCK
Using DISTINCT with UNION or GROUP BY.
Drew
J. Drew Allen
Business Intelligence Analyst
Philadelphia, PA
October 9, 2018 at 9:46 pm
drew.allen - Tuesday, October 9, 2018 12:40 PMNOLOCKUsing DISTINCT with UNION or GROUP BY.
Drew
I have a variant on that: doing aggregates and grouping by the table's PK.
----------------------------------------------------------------------------------
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?
October 9, 2018 at 10:05 pm
VARCHAR(1) or NVARCHAR(1) (or most anything less than 10 characters but especially for 1 character.
WHERE ISNULL(somecolumn,0) > 0
WHERE somecolumn IS NOT NULL AND somecolumn > ' '
ALTER INDEX abcd ON dbo.sometable REORGANIZE
DECLARE @SomeVariable;
SELECT @SomeVariable = COUNT(*) FROM SomeTable;
IF @SomeVariable > 0 BEGIN
--Jeff Moden
Change is inevitable... Change for the better is not.
October 9, 2018 at 10:09 pm
And I disagree with NOLOCK being useless (although it should be WITH (NOLOCK)). It does have its uses.
--Jeff Moden
Change is inevitable... Change for the better is not.
October 10, 2018 at 1:45 am
Gotta love this, found yesterday:
--remove duplicates from the staging table
WITH t AS (
SELECT ID, ROW_NUMBER() OVER(PARTITION BY CampaignKey ORDER BY [Date]) AS rnum
FROM staging.EmailCampaign
)
DELETE ee
FROM t
INNER JOIN staging.EmailCampaign ee
ON t.ID = ee.ID
WHERE t.rnum > 1
For fast, accurate and documented assistance in answering your questions, please read this article.
Understanding and using APPLY, (I) and (II) Paul White
Hidden RBAR: Triangular Joins / The "Numbers" or "Tally" Table: What it is and how it replaces a loop Jeff Moden
October 10, 2018 at 3:08 am
Jeff Moden - Tuesday, October 9, 2018 10:09 PMAnd I disagree with NOLOCK being useless (although it should be WITH (NOLOCK)). It does have its uses.
Possibly the main problem with NOLOCK is that it's actually misnamed. If it was called the ReturnAnyOldCrap_PossiblyCauseCorruption hint, I think that then the people who insist on using it would ... well, probably still use it anyway. A few might, perhaps, stop because; <whine> "It takes too long to type now"
I'm a DBA.
I'm not paid to solve problems. I'm paid to prevent them.
October 10, 2018 at 6:11 am
ChrisM@Work - Wednesday, October 10, 2018 1:45 AMGotta love this, found yesterday:--remove duplicates from the staging table
WITH t AS (
SELECT ID, ROW_NUMBER() OVER(PARTITION BY CampaignKey ORDER BY [Date]) AS rnum
FROM staging.EmailCampaign
)
DELETE ee
FROM t
INNER JOIN staging.EmailCampaign ee
ON t.ID = ee.ID
WHERE t.rnum > 1
Heh... too funny. Ya just can't make stuff like that up.
--Jeff Moden
Change is inevitable... Change for the better is not.
October 10, 2018 at 6:29 am
andrew gothard - Wednesday, October 10, 2018 3:08 AMJeff Moden - Tuesday, October 9, 2018 10:09 PMAnd I disagree with NOLOCK being useless (although it should be WITH (NOLOCK)). It does have its uses.Possibly the main problem with NOLOCK is that it's actually misnamed. If it was called the ReturnAnyOldCrap_PossiblyCauseCorruption hint, I think that then the people who insist on using it would ... well, probably still use it anyway. A few might, perhaps, stop because; <whine> "It takes too long to type now"
While I agree that it's generally abused as a go-faster button for some really crappy code that needs to be fixed, there are places where its usage is not only ok, but is actually desired (my sp_WhatsRunning stored procedure, for example) because I actually DO need to be able to catch midstream transactions (as one of many examples). As with all else, "It Depends" but WITH (NOLOCK) is hardly useless code.
Now, let me ask you... do you use REORGANIZE on ANY of your index maintenance? Probably... and why do you do that? Because you were told that it's a "Best Practice" to do (usually) between 10% and 30% logical fragmentation but you've probably never done a deep dive to find out that it actually perpetuates fragmentation and, therefore, the need to defragment. WITH (NOLOCK) falls into the same category. It's not actually a "Best Practice" to NOT use it. It's actually a "Best Practice" to use it appropriately and correctly. 😉
Beware of "Best Practices" where people insist that something should either always be done or never be done. They WILL bite you especially if you mistake the clamor of the crowd for the wisdom of a group even in the presence of seemingly overwhelming evidence. In most cases and with very few exceptions, the only real truth is "It Depends". 😉
--Jeff Moden
Change is inevitable... Change for the better is not.
October 10, 2018 at 7:30 am
I still find myself occasionally having to strip:
SELECT TOP 100 PERCENT
...
ORDER BY xxx
from views because somebody decided it was a way to "get around" not being allowed to use ORDER BY in views. Similarly having to remove a GetDateView view because "you can't use GetDate in scalar functions but you can select from views".
Although my all time favourite was about 40 lines of code to determine that the first day of the month, as an integer, was 1 (literally a massive sequence of nested CASE statements with a convoluted set of checks for each month). Immediately followed by a single line of code that decided the last day of the month was 28 because, as per the comments, that will always be a valid day. To this day I have no idea how that ever came to be.
October 10, 2018 at 7:41 am
Also using the same expression in both the PARTITION BY and ORDER BY clauses of an OVER() clause.
J. Drew Allen
Business Intelligence Analyst
Philadelphia, PA
October 10, 2018 at 7:55 am
drew.allen - Wednesday, October 10, 2018 7:41 AMAlso using the same expression in both the PARTITION BY and ORDER BY clauses of an OVER() clause.
Haha - funnily enough, that exact same syntax was employed in the dedupe example I posted up ^^ here. I took it out to make what I figured was the most important point. That would make for some interesting posts though - how much stupid can you find in a single production statement?
For fast, accurate and documented assistance in answering your questions, please read this article.
Understanding and using APPLY, (I) and (II) Paul White
Hidden RBAR: Triangular Joins / The "Numbers" or "Tally" Table: What it is and how it replaces a loop Jeff Moden
October 10, 2018 at 9:11 am
ChrisM@Work - Wednesday, October 10, 2018 7:55 AMdrew.allen - Wednesday, October 10, 2018 7:41 AMAlso using the same expression in both the PARTITION BY and ORDER BY clauses of an OVER() clause.Haha - funnily enough, that exact same syntax was employed in the dedupe example I posted up ^^ here. I took it out to make what I figured was the most important point. That would make for some interesting posts though - how much stupid can you find in a single production statement?
I really fear what the responses would be to this question......
Also, I just noticed Gail's quote in your signature. A lot of people should heed her words...
Mike Scalise, PMP
https://www.michaelscalise.com
October 10, 2018 at 10:23 am
My SQL PTSD just kicked in as I recalled this little gem. How about a 1600 lines of
declare @theanswer int;
If (some wildly complicated query)
BEGIN
If (<some additional query>)
set @theanswer=1
else
BEGIN
if(yada yada yada)
Set @theanswer=2
--continue the pattern another 1500 lines....
END
Else
set @theanswer=300
--and now the kicker....
set @answer=0
33 minutes of querying to determine the answer, then just toss it out 🙂
----------------------------------------------------------------------------------
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?
Viewing 15 posts - 1 through 15 (of 48 total)
You must be logged in to reply to this topic. Login to reply