What are some really dumb things you could write in a SQL Query

Question

What are some really dumb things you could write in a SQL Query

Mike Scalise

SSCrazy Eights

Points: 8145
More actions
October 9, 2018 at 11:49 am

#370082

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

Viewing 15 posts - 1 through 15 (of 48 total)

You must be logged in to reply to this topic. Login to reply

Thom A SSC Guru Points: 99127 More actions · Answer 1

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

drew.allen SSC Guru Points: 76984 More actions · Answer 2

NOLOCK

Using DISTINCT with UNION or GROUP BY.

Drew

J. Drew Allen
Business Intelligence Analyst
Philadelphia, PA

Matt Miller (4) SSC Guru Points: 124210 More actions · Answer 3

drew.allen - Tuesday, October 9, 2018 12:40 PM
NOLOCK
Using 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?

Jeff Moden SSC Guru Points: 1004290 More actions · Answer 4

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

RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

Change is inevitable... Change for the better is not.

Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally)

Jeff Moden SSC Guru Points: 1004290 More actions · Answer 5

And I disagree with NOLOCK being useless (although it should be WITH (NOLOCK)). It does have its uses.

--Jeff Moden

RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

Change is inevitable... Change for the better is not.

Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally)

ChrisM@Work SSC Guru Points: 186127 More actions · Answer 6

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

^{“Write the query the simplest way. If through testing it becomes clear that the performance is inadequate, consider alternative query forms.” - Gail Shaw}

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

andrew gothard SSChampion Points: 12314 More actions · Answer 7

Jeff Moden - Tuesday, October 9, 2018 10:09 PM
And 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.

Jeff Moden SSC Guru Points: 1004290 More actions · Answer 8

ChrisM@Work - Wednesday, October 10, 2018 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

Heh... too funny. Ya just can't make stuff like that up.

--Jeff Moden

RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

Change is inevitable... Change for the better is not.

Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally)

Jeff Moden SSC Guru Points: 1004290 More actions · Answer 9

andrew gothard - Wednesday, October 10, 2018 3:08 AM
Jeff Moden - Tuesday, October 9, 2018 10:09 PM
And 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

RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

Change is inevitable... Change for the better is not.

Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally)

andycadley SSCertifiable Points: 5296 More actions · Answer 10

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.

drew.allen SSC Guru Points: 76984 More actions · Answer 11

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

ChrisM@Work SSC Guru Points: 186127 More actions · Answer 12

drew.allen - Wednesday, October 10, 2018 7:41 AM
Also 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?

^{“Write the query the simplest way. If through testing it becomes clear that the performance is inadequate, consider alternative query forms.” - Gail Shaw}

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

Mike Scalise SSCrazy Eights Points: 8145 More actions · Answer 13

ChrisM@Work - Wednesday, October 10, 2018 7:55 AM
drew.allen - Wednesday, October 10, 2018 7:41 AM
Also 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

Matt Miller (4) SSC Guru Points: 124210 More actions · Answer 14

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?