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

  • Jeff Moden - Wednesday, October 10, 2018 6:29 AM

    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". 😉

    Oh, anything can be misused, and due to - well, mostly experience - when I hear the phrase "Best Practice" these days I generally turn off my bullshit detector to stop it terminally overloading.  Particularly (and I know this is not what you're arguing here at all) "we use NOLOCK because it's a best practice to add it to all queries for performance".  Including, apparently, updates and inserts, and also on data that peoples' lives depend on - never quite seen the use case on that one.
    In terms of index maintenance - yeah, we do it on most of our systems and Reorg comes as part of the scripts, and it's not an area where I'm going to roll my own.  Stats updates we do on all of them, because that's where you get most of the performance benefit from from index maintenance.

    I'm a DBA.
    I'm not paid to solve problems. I'm paid to prevent them.

  • Attitude of some developers:
    /*I don't delete data, but when I do, I do it in prod with no Begin tran and no where clause. I mean who needs where clause? SQL Server is smart enough to know which data I am thinking of deleting*/
    delete from table

    "He who learns for the sake of haughtiness, dies ignorant. He who learns only to talk, rather than to act, dies a hyprocite. He who learns for the mere sake of debating, dies irreligious. He who learns only to accumulate wealth, dies an atheist. And he who learns for the sake of action, dies a mystic."[/i]

  • andrew gothard - Tuesday, October 16, 2018 5:09 AM

    Jeff Moden - Wednesday, October 10, 2018 6:29 AM

    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". 😉

    Oh, anything can be misused, and due to - well, mostly experience - when I hear the phrase "Best Practice" these days I generally turn off my bullshit detector to stop it terminally overloading.  Particularly (and I know this is not what you're arguing here at all) "we use NOLOCK because it's a best practice to add it to all queries for performance".  Including, apparently, updates and inserts, and also on data that peoples' lives depend on - never quite seen the use case on that one.
    In terms of index maintenance - yeah, we do it on most of our systems and Reorg comes as part of the scripts, and it's not an area where I'm going to roll my own.  Stats updates we do on all of them, because that's where you get most of the performance benefit from from index maintenance.

    Yeah... I definitely agree that people using WITH(NOLOCK) in that fashion definitely need to turn in their man-card. 😉

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

  • 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

    How are we quoting Gails article without using the correct isolation mode or query hint? If you don't use the correct isolation mode or query hint then the "where not exists( … )" also has a chance of failing.

    I think we all need to reread the linked article.

Viewing 4 posts - 46 through 48 (of 48 total)

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