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

  • In terms of potential lost performance and wasted resources, this is probably the single dumbest thing:


    CREATE TABLE dbo.every_table_created
    (
        id int IDENTITY(1, 1) NOT NULL PRIMARY KEY,
        ...rest_of_columns_dont_matter_always_cluster_by_ident_regardless...
    )

    SQL DBA,SQL Server MVP(07, 08, 09) "It's a dog-eat-dog world, and I'm wearing Milk-Bone underwear." "Norm", on "Cheers". Also from "Cheers", from "Carla": "You need to know 3 things about Tortelli men: Tortelli men draw women like flies; Tortelli men treat women like flies; Tortelli men's brains are in their flies".

  • Here's a new one.


    SELECT <primary key>, <other fields>
    FROM Table_Stage

    INTERSECT

    SELECT <primary key>, <other fields> /*  all fields from Table_Prod  */
    FROM Table_Prod
    INNER JOIN Table_Stage
        ON Table_Prod.<primary key> = Table_Stage.<primary key>

    Drew

    J. Drew Allen
    Business Intelligence Analyst
    Philadelphia, PA

  • I inherited code from someone who couldn't do a thing without cursors.  I'd swear they had a DECLARE CURSOR snippet auto-populated in every new query window.  Nested cursors instead of using a JOIN in the first one.  And not a single modifier like FAST_FORWARD, although it would have been appropriate in almost every case.  It took years to weed all that crap out of the code base.

    The absolute worst example was something like this:

    DECLARE CURSOR dufus FOR SELECT xyz FROM MyTable WHERE xyz > 0
    DECLARE @xyz INT
    OPEN dufus
    FETCH NEXT FROM dufus INTO @xyz
    IF @@FETCH_STATUS = 0 BEGIN
        -- Do stuff
        -- Absolutely no reference to the cursor, or the variable populated in the FETCH NEXT statement.
    END
    CLOSE dufus
    DEALLOCATE dufus


    instead of simply:

    IF EXISTS(SELECT xyz FROM MyTable WHERE xyz > 0) BEGIN
        -- Do stuff
    END

  • SELECT * in a deployed production job

  • Scott Coleman - Wednesday, October 10, 2018 2:14 PM

    I inherited code from someone who couldn't do a thing without cursors.  I'd swear they had a DECLARE CURSOR snippet auto-populated in every new query window.  Nested cursors instead of using a JOIN in the first one.  And not a single modifier like FAST_FORWARD, although it would have been appropriate in almost every case.  It took years to weed all that crap out of the code base.

    The absolute worst example was something like this:

    DECLARE CURSOR dufus FOR SELECT xyz FROM MyTable WHERE xyz > 0
    DECLARE @xyz INT
    OPEN dufus
    FETCH NEXT FROM dufus INTO @xyz
    IF @@FETCH_STATUS = 0 BEGIN
        -- Do stuff
        -- Absolutely no reference to the cursor, or the variable populated in the FETCH NEXT statement.
    END
    CLOSE dufus
    DEALLOCATE dufus


    instead of simply:

    IF EXISTS(SELECT xyz FROM MyTable WHERE xyz > 0) BEGIN
        -- Do stuff
    END

    Just guessing, but maybe they came from an Oracle environment, where every access is thru a cursor (but with far less overhead than such a cursor in SQL Server).

    SQL DBA,SQL Server MVP(07, 08, 09) "It's a dog-eat-dog world, and I'm wearing Milk-Bone underwear." "Norm", on "Cheers". Also from "Cheers", from "Carla": "You need to know 3 things about Tortelli men: Tortelli men draw women like flies; Tortelli men treat women like flies; Tortelli men's brains are in their flies".

  • Scott Coleman - Wednesday, October 10, 2018 2:14 PM

    I inherited code from someone who couldn't do a thing without cursors.  I'd swear they had a DECLARE CURSOR snippet auto-populated in every new query window.  Nested cursors instead of using a JOIN in the first one.  And not a single modifier like FAST_FORWARD, although it would have been appropriate in almost every case.  It took years to weed all that crap out of the code base.

    The absolute worst example was something like this:

    DECLARE CURSOR dufus FOR SELECT xyz FROM MyTable WHERE xyz > 0
    DECLARE @xyz INT
    OPEN dufus
    FETCH NEXT FROM dufus INTO @xyz
    IF @@FETCH_STATUS = 0 BEGIN
        -- Do stuff
        -- Absolutely no reference to the cursor, or the variable populated in the FETCH NEXT statement.
    END
    CLOSE dufus
    DEALLOCATE dufus


    instead of simply:

    IF EXISTS(SELECT xyz FROM MyTable WHERE xyz > 0) BEGIN
        -- Do stuff
    END

    Are you working at my old company?? Except that this code was in a trigger.

    Michael L John
    If you assassinate a DBA, would you pull a trigger?
    To properly post on a forum:
    http://www.sqlservercentral.com/articles/61537/

  • probably the worst SQL code pattern I've seen goes something like this...
    scalar function to find "current" record:
    CREATE FUNCTION fun_GetCurrentStatus(@EntityID INT)
    RETURNS INT AS
    BEGIN
    DECLARE @EntityStatusID INT

    SELECT TOP 1 @EntityStatusID = EntityStatusID
    FROM EntityStatus
    WHERE EntityID = @EntityID
    AND DiscontinueDate IS NULL
    ORDER BY StatusDateTime DESC

    RETURN @EntityStatusID
    END

    Then calls all over many stored procs to this function within a JOIN like:
    ...
    FROM Entity e
      INNER JOIN EntityStatus es ON e.EntityID = es.EntityID
        AND es.EntiyStatusID = fun_GetCurrentStatus(e.EntityID)

    :crazy:

  • ScottPletcher - Wednesday, October 10, 2018 10:47 AM

    In terms of potential lost performance and wasted resources, this is probably the single dumbest thing:


    CREATE TABLE dbo.every_table_created
    (
        id int IDENTITY(1, 1) NOT NULL PRIMARY KEY,
        ...rest_of_columns_dont_matter_always_cluster_by_ident_regardless...
    )

    I agree that's stupid for "every table" but it's a whole lot more useful in more areas that you tend to think or advertise.

    --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)

  • ZZartin - Wednesday, October 10, 2018 2:17 PM

    SELECT * in a deployed production job

    A very strong code smell, indeed, but as with all else in SQL Server, "It Depends".  It's one of the most effect methods for WHERE EXISTS and, if you really do need to return everything from a table, especially a wide table, it's actually faster than breaking out every column that you need.  Of course, I wouldn't do that on a permanent table but I have no reservations with using SELECT * on a Temp Table or SELECT * INTO to create one.

    --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 - Wednesday, October 10, 2018 3:49 PM

    ScottPletcher - Wednesday, October 10, 2018 10:47 AM

    In terms of potential lost performance and wasted resources, this is probably the single dumbest thing:


    CREATE TABLE dbo.every_table_created
    (
        id int IDENTITY(1, 1) NOT NULL PRIMARY KEY,
        ...rest_of_columns_dont_matter_always_cluster_by_ident_regardless...
    )

    I agree that's stupid for "every table" but it's a whole lot more useful in more areas that you tend to think or advertise.

    Not based on my actual experience for the last 20 years.  Most people don't ever actually bother to review clustered key choices to determine the best clus key.  They just slap an identity on the table and claim they have the best "design".

    But I've seen literally only a handful of dbs where even half the tables were best clustered on identity, and one of those was a db of "master" tables, which naturally tend to best be clustered on ident.

    SQL DBA,SQL Server MVP(07, 08, 09) "It's a dog-eat-dog world, and I'm wearing Milk-Bone underwear." "Norm", on "Cheers". Also from "Cheers", from "Carla": "You need to know 3 things about Tortelli men: Tortelli men draw women like flies; Tortelli men treat women like flies; Tortelli men's brains are in their flies".

  • ScottPletcher - Wednesday, October 10, 2018 4:16 PM

    Jeff Moden - Wednesday, October 10, 2018 3:49 PM

    ScottPletcher - Wednesday, October 10, 2018 10:47 AM

    In terms of potential lost performance and wasted resources, this is probably the single dumbest thing:


    CREATE TABLE dbo.every_table_created
    (
        id int IDENTITY(1, 1) NOT NULL PRIMARY KEY,
        ...rest_of_columns_dont_matter_always_cluster_by_ident_regardless...
    )

    I agree that's stupid for "every table" but it's a whole lot more useful in more areas that you tend to think or advertise.

    Not based on my actual experience for the last 20 years.  Most people don't ever actually bother to review clustered key choices to determine the best clus key.  They just slap an identity on the table and claim they have the best "design".

    But I've seen literally only a handful of dbs where even half the tables were best clustered on identity, and one of those was a db of "master" tables, which naturally tend to best be clustered on ident.

    Don't knock the 20 year ring... I've got 22 years in this game myself.  😉  I was also an SQL Server MVP for 3 times longer than you were but that means nothing of knowledge because it's a service award, not a certification of knowledge. It's a part of the reason why I don't carry such information is my signature line.  I also wrote my own specialized database about 2 years before I even knew what SQL was and it was complete with CI's, NCIs, and a sort algorithm that blew the doors off of SHELL SORT for both speed and stability for additional sorts on the same data.

    Anyway, back to the important subject.  I agree that a lot of people are a bit crazy when it comes to clustered indexes but not just the people that think every table needs an IDENTITY column based CI.  I find that a lot of people that take the stance that virtually no table should be CI'd on an IDENTITY are just as crazy because they don't understand some of the other ramifications. CIs have more uses than making your queries run faster, especially when it comes to memory management (especially on wide tables or long tables), generally reduced memory requirements thanks to uniqueness and narrowness, bad page splits, and the resulting massive log files that can occur because of the page splits that happen due to "ExpAnsive" updates (which are very bad indeed).

    To summarize, yes, I agree that using an IDENTITY column on EVERY table is a really dumb idea but so is the opposite of that.

    As with all else in SQL Server and T-SQL, "It Depends".

    I believe I have previously provided you with a link to Kimberly Tripp's Microsoft Certified Master (now THERE's a cert worth bragging about) presentation on the "Great Clustered Index Debate".  Did you watch it?  If not, you should... everyone on the planet that works with SQL Server should.

    --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)

  • Just in case you didn't see it, they took it down from the MS site it was previously posted on.  Someone also took the copy down from YouTube.  However, they do have the video on the SQLSkills.com site now.  Here's the link for that video.
    https://s3.amazonaws.com/sqlskills.com/MCM/HDI-ITPro-TechNet-Winvideo-MCM_06_ClusteredIndexDebate.wmv

    Kimberly also has another great presentation on the subject of physical design being a major consideration at the following.
    https://www.youtube.com/watch?v=H-jPsp2QlT0

    --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 - 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

    I must be missing something.  I looked at this for nearly half an hour, and I don't get it.  Can someone explain why it doesn't work?  Or if it does work, why it isn't a good idea?

  • gvoshol 73146 - Thursday, October 11, 2018 5:55 AM

    I must be missing something.  I looked at this for nearly half an hour, and I don't get it.  Can someone explain why it doesn't work?  Or if it does work, why it isn't a good idea?

    It identifies duplicate CampaignKey values by providing a row number. It identifies the oldest one (row number 1) and more recent ones (row number > 1). It then selects only rows that have a prior row with the same CampaignKey.
    And then it does not delete this just-identief duplicate; instead it joins back to the original table on CampaignKey and deletes all rows with that value, Including the oldest one that they presumably wanted to keep.
    (If they did indeed intend to throw out all rows that have a duplicated CampaignKey value, there would have been easier ways)


    Hugo Kornelis, SQL Server/Data Platform MVP (2006-2016)
    Visit my SQL Server blog: https://sqlserverfast.com/blog/
    SQL Server Execution Plan Reference: https://sqlserverfast.com/epr/

  • Encountered way too often in code at my current customer:

    IF (SELECT COUNT(*) FROM SomeTable WHERE SomeCondition) > 0
    BEGIN;
        -- Do something
    END;


    Hugo Kornelis, SQL Server/Data Platform MVP (2006-2016)
    Visit my SQL Server blog: https://sqlserverfast.com/blog/
    SQL Server Execution Plan Reference: https://sqlserverfast.com/epr/

Viewing 15 posts - 16 through 30 (of 48 total)

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