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

  • Hugo Kornelis - Thursday, October 11, 2018 7:14 AM

    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)

    actually - the join is by ID, not campaign key, so presumably they are NOT deleting the original one.  Still - the extra join is unnecessary.

    ----------------------------------------------------------------------------------
    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 - Wednesday, October 10, 2018 5:07 PM

    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.

    For many (most?) people, identity is automatically a default CI, making it just a lazy crutch.  And that harms performance more than most everything else combined.

    The other major problem is that, for most shops, no logical data design is ever done.  Just physical tables are laid out.  And it's most often done by developers with no actual data modeling experience.  All that combines to produce tables that are not normalized at all and are based on programming concerns rather than data concerns.  

    And "tuning" consists in creating covering indexes for every major query.  Of course creating a custom "mini-table" to match every query will improve the speed of that query.  Bbut overall, the overhead is quite enormous compared to properly selecting the best CI keys first.  And the maintenance is never-ending: the second you add another column to the query, your covering index isn't any more.

    I've almost never seen cases where even half the tables in a db a best clustered by identity.  As one pattern, standard "child" tables, such as Order_Items, should almost always be clustered first on the parent id, such as order_id.  Child tables alone often account for ~50% of the tables in a db.

    Another pattern to indicate that a CI of identity should be reviewed is if you see several nonclus indexes all starting with the same column.  For example, CI on id, and 4 nc indexes with ColA as the lead column.  That table should be reviewed to determine if ColA would make a better CI.

    SQL DBA,SQL Server MVP(07, 08, 09) A socialist is someone who will give you the shirt off *someone else's* back.

  • Hugo Kornelis - Thursday, October 11, 2018 7:16 AM

    Encountered way too often in code at my current customer:

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

    I found code where the "do something" was to update the same table with the same conditions.

    Drew

    J. Drew Allen
    Business Intelligence Analyst
    Philadelphia, PA

  • drew.allen - Thursday, October 11, 2018 7:49 AM

    Hugo Kornelis - Thursday, October 11, 2018 7:16 AM

    Encountered way too often in code at my current customer:

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

    I found code where the "do something" was to update the same table with the same conditions.

    Drew

    Heh... funny you should mention that.  There is actually a circumstance where you need to do an inplace update of existing data and that's when you want to move all LOB data in a table to "out of row".  You first have to set the option with sp_tableoption (or whatever) and that will cause any new LOB data to be inserted out of row.  But, to get the old blob data to move out of row, you have to update existing values with the same value to get them to move.

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

  • drew.allen - Thursday, October 11, 2018 7:49 AM

    Hugo Kornelis - Thursday, October 11, 2018 7:16 AM

    Encountered way too often in code at my current customer:

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

    I found code where the "do something" was to update the same table with the same conditions.

    Drew

    This is also very common:
    IF NOT EXISTS(SELECT * FROM dbo.MyTable mt WHERE mt.Code = @Code) 
      INSERT INTO dbo.MyTable (Code, ... )  
      VALUES (@Code,   ...)

    They could just have:
     INSERT INTO dbo.MyTable (Code, ...) 
     SELECT @Code, ...
     WHERE NOT EXISTS(SELECT * FROM dbo.MyTable mt WHERE mt.Code = @Code)

  • Jonathan AC Roberts - Thursday, October 11, 2018 9:51 AM

    drew.allen - Thursday, October 11, 2018 7:49 AM

    Hugo Kornelis - Thursday, October 11, 2018 7:16 AM

    Encountered way too often in code at my current customer:

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

    I found code where the "do something" was to update the same table with the same conditions.

    Drew

    This is also very common:
    IF NOT EXISTS(SELECT * FROM dbo.MyTable mt WHERE mt.Code = @Code) 
      INSERT INTO dbo.MyTable (Code, ... )  
      VALUES (@Code,   ...)

    They could just have:
     INSERT INTO dbo.MyTable (Code, ...) 
     SELECT @Code, ...
     WHERE NOT EXISTS(SELECT * FROM dbo.MyTable mt WHERE mt.Code = @Code)

    True - but  is either particularly more efficient than  the other? Both require you to scan check the table before deciding to insert or not, so this looks to be more stylistic than a "dumb thing to do" (as would imply the topic)

    (edit to move to a more generic description rather than something that implies how the check is performed.)

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

  • Matt Miller (4) - Thursday, October 11, 2018 9:56 AM

    Jonathan AC Roberts - Thursday, October 11, 2018 9:51 AM

    drew.allen - Thursday, October 11, 2018 7:49 AM

    Hugo Kornelis - Thursday, October 11, 2018 7:16 AM

    Encountered way too often in code at my current customer:

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

    I found code where the "do something" was to update the same table with the same conditions.

    Drew

    This is also very common:
    IF NOT EXISTS(SELECT * FROM dbo.MyTable mt WHERE mt.Code = @Code) 
      INSERT INTO dbo.MyTable (Code, ... )  
      VALUES (@Code,   ...)

    They could just have:
     INSERT INTO dbo.MyTable (Code, ...) 
     SELECT @Code, ...
     WHERE NOT EXISTS(SELECT * FROM dbo.MyTable mt WHERE mt.Code = @Code)

    True - but  is either particularly more efficient than  the other? Both require you to scan the table before deciding to insert or not, so this looks to be more stylistic than a "dumb thing to do" (as would imply the topic)

    Not a performance issue, but a concurrency issue
    Why is this upsert code broken? by Gail Shaw

  • DesNorton - Thursday, October 11, 2018 10:00 AM

    Matt Miller (4) - Thursday, October 11, 2018 9:56 AM

    Jonathan AC Roberts - Thursday, October 11, 2018 9:51 AM

    drew.allen - Thursday, October 11, 2018 7:49 AM

    Hugo Kornelis - Thursday, October 11, 2018 7:16 AM

    Encountered way too often in code at my current customer:

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

    I found code where the "do something" was to update the same table with the same conditions.

    Drew

    This is also very common:
    IF NOT EXISTS(SELECT * FROM dbo.MyTable mt WHERE mt.Code = @Code) 
      INSERT INTO dbo.MyTable (Code, ... )  
      VALUES (@Code,   ...)

    They could just have:
     INSERT INTO dbo.MyTable (Code, ...) 
     SELECT @Code, ...
     WHERE NOT EXISTS(SELECT * FROM dbo.MyTable mt WHERE mt.Code = @Code)

    True - but  is either particularly more efficient than  the other? Both require you to scan the table before deciding to insert or not, so this looks to be more stylistic than a "dumb thing to do" (as would imply the topic)

    Not a performance issue, but a concurrency issue
    Why is this upsert code broken? by Gail Shaw

    Ok fair, assuming that's a key 🙂 - but isn't the primary "dumb" thing trying to manage the numbering sequence yourself? Most of those requests have been of dubious business value when I met them, and the maintenance alone would put that in the "beyond stupid" bucket when I ran into them.

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

  • 1. I have a long way to go because I only understand the implications of some of these code posts.
    2. I wonder if any of my code would be on these lists. :ermm:

    -Mark
    MSSQL 2019 Standard, Azure Hosted. Techie/Sysadmin by trade; Three years as a "DBA" now.

  • usererror - Friday, October 12, 2018 11:26 AM

    1. I have a long way to go because I only understand the implications of some of these code posts.
    2. I wonder if any of my code would be on these lists. :ermm:

    Don't feel bad about it, there's some of these snippets I look at and can't quite figure out...
    (I work better with being able to actually test it out and see what it's doing)

    As for the second, again, I'm sure if Jeff M went back through his old code, he'd have stuff to post here...
    :Whistling:

  • jasona.work - Friday, October 12, 2018 11:53 AM

    usererror - Friday, October 12, 2018 11:26 AM

    1. I have a long way to go because I only understand the implications of some of these code posts.
    2. I wonder if any of my code would be on these lists. :ermm:

    Don't feel bad about it, there's some of these snippets I look at and can't quite figure out...
    (I work better with being able to actually test it out and see what it's doing)

    As for the second, again, I'm sure if Jeff M went back through his old code, he'd have stuff to post here...
    :Whistling:

    My code is pristine if I do say so myself, and I do.  😀

    Drew

    J. Drew Allen
    Business Intelligence Analyst
    Philadelphia, PA

  • drew.allen - Friday, October 12, 2018 12:23 PM

    jasona.work - Friday, October 12, 2018 11:53 AM

    usererror - Friday, October 12, 2018 11:26 AM

    1. I have a long way to go because I only understand the implications of some of these code posts.
    2. I wonder if any of my code would be on these lists. :ermm:

    Don't feel bad about it, there's some of these snippets I look at and can't quite figure out...
    (I work better with being able to actually test it out and see what it's doing)

    As for the second, again, I'm sure if Jeff M went back through his old code, he'd have stuff to post here...
    :Whistling:

    My code is pristine if I do say so myself, and I do.  😀

    Drew

    I bet it's commented and perfectly formatted too. 😛

    Mike Scalise, PMP
    https://www.michaelscalise.com


  • set @counter = @counter + 1 /* increment counter */  /* I see this or slight variations of it all the time */
    --------------------...
    /* not quite as bad */
    set @counter = @counter + 1 /* increment files processed counter */  
    --...
    set @xy = (@ab + @cd) / 12.75 /* calc new sales goal */

    SQL DBA,SQL Server MVP(07, 08, 09) A socialist is someone who will give you the shirt off *someone else's* back.

  • ScottPletcher - Friday, October 12, 2018 1:51 PM


    set @counter = @counter + 1 /* increment counter */  /* I see this or slight variations of it all the time */
    --------------------...
    /* not quite as bad */
    set @counter = @counter + 1 /* increment files processed counter */  
    --...
    set @xy = (@ab + @cd) / 12.75 /* calc new sales goal */

    I must be having a slow day - what's the dumb part here?

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

  • Matt Miller (4) - Friday, October 12, 2018 2:53 PM

    ScottPletcher - Friday, October 12, 2018 1:51 PM


    set @counter = @counter + 1 /* increment counter */  /* I see this or slight variations of it all the time */
    --------------------...
    /* not quite as bad */
    set @counter = @counter + 1 /* increment files processed counter */  
    --...
    set @xy = (@ab + @cd) / 12.75 /* calc new sales goal */

    I must be having a slow day - what's the dumb part here?

    The first variable name and comment are both useless -- anyone knows that adding 1 to a counter increments it, but WHAT does THAT mean?

    Using clear variables names eliminates the need for less clear comments, i.e., the code should be more like:
    set @files_processed_count = @files_processed_count + 1

    Similarly, the last one should be something like:
    set @new_sales_goal = (@existing_sales_goal + @stretch_goal) / 12.75 /* 12 mths/yr plus an adjustment factor */

    SQL DBA,SQL Server MVP(07, 08, 09) A socialist is someone who will give you the shirt off *someone else's* back.

Viewing 15 posts - 31 through 45 (of 48 total)

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