Building Better Stored Procedures

  • autoexcrement

    SSCertifiable

    Points: 5885

    Are you saying there is literally a "CREATE OR ALTER" feature? Or are you just using that name informally? The code examples you gave in the screenshot below appear to be identical, and I didn't see any "CREATE OR ALTER". 


    "If I had been drinking out of that toilet, I might have been killed." -Ace Ventura

  • Andy Warren

    SSC Guru

    Points: 119676

  • autoexcrement

    SSCertifiable

    Points: 5885

    Thanks, I didn't find that with a quick google search, and the article code (screenshot) seems to be wrong, which further confused me. Hope it will be corrected. Thanks for the link!


    "If I had been drinking out of that toilet, I might have been killed." -Ace Ventura

  • marykdba

    Mr or Mrs. 500

    Points: 574

    The graphic between "Still not great, but we also got CREATE OR ALTER. That means I can just write this code all the time." and "We should have had that at the beginning." is the same graphic as before "Still not great, but we also got CREATE OR ALTER. That means I can just write this code all the time."  Was the second graphic supposed to demonstrate CREATE OR ALTER?

  • David Rueter

    SSCrazy

    Points: 2632

    A good recommendation is to have a simple, but useful header that you use to document the purpose of your procedure and then include comments in places where you think the next developer might not know why you implemented some code. It's easy to determine what is happening, and certainly everyone should have a VCS that stores code to know who and when things changed, but the why is often lost. I might re-use a comment on why I made a change in both the procedure and as my VCS commit message.

    If you create comments before the BEGIN statement, the comments show up in the SQL Profiler command text (and other places you look at command text).  This is very annoying when trying to do analysis or performance tuning, because you see the silly comment instead of the actual command being executed.  I'd strongly suggest moving the comment to after the BEGIN so as not to cause frustrations down the road.

  • andycao

    Say Hey Kid

    Points: 698

    dale_berta - Thursday, March 14, 2019 6:08 AM

    sqlservercentral-631096 - Thursday, March 14, 2019 2:50 AM

    Hey Steve,
    Great article, and thanks for sharing.
    You discussed whether to use create procedure or create a stubb proc and do an alter.  Finishing off with "I'm not sure which I like better, but I don't really think any of them are great".
    Now, I'm no DBA so I may be talking hokum ... but as I understand it if you use "if exists drop proc" variation you lose any permissions allocated to the proc.  The second version with the alter doesn't lose the permissions.
    Now ideally we'd have permissions scripted as part of an upgrade procedure, but in real life permissions get added outside a normal release cycle.
    Hence I tend to prefer creating a stubb and using alter, which may sway your indecision as to which is preferred 😉
    Hope this helps.

    Nailed it! We do a CREATE with initial permissions. All subsequent changes are ALTERs, which almost never have permission changes.

    No one else has mentioned this: ALTER also lets you keep the CreatedOn date of the SP.  That information can be golden!  It's almost like folks here have never worked on systems that are many, many years old.

  • David Rueter

    SSCrazy

    Points: 2632

    Regarding CREATE vs. ALTER...maybe it is just personal taste, but I don't mind--or maybe even like--the two different commands.  We are not just declaring a procedure (or a function, or a view, or whatever), but we are explicitly creating an object in the database.  Once there is an object in the database, it should not change unless we explicitly want it to change.

    For the same reason that I am glad SQL gives us separate INSERT and UPDATE statements for rows, I'm glad it gives us CREATE and ALTER for objects.

    Yes, sometimes an 'upsert' is handy--and there are ways to do that if needed.  But since dropping and re-creating objects is not without side effects (such as loosing permissions, loosing asymmetric keys, executing DDL triggers, triggering a re-compile of the object, changing the object id, etc.) it is possibly dangerous to be in the mindset that a DROP + CREATE produces the same results as an ALTER.

  • David Rueter

    SSCrazy

    Points: 2632

    This might be a good time for a plug for my SQLVer Version Tracking article.  (It might be time for me to write a new article on this that describes some updated capabilities).  One new thing this lets you do is have a comment block like:


    /*/ver Here is a version-specific comment.

    This will be saved to sqlver.tblSchemaLog, but will be stripped from the source code of the object.
    */

    This way you can simply execute:
    ver 'MyProcName'

    (ver being a synonym to a SQLVer stored procedure) 

    This will return a reultset of all versions of this object, including date/time/user of the update, the version-specific comments, the actual SQL command executed, and the hash of the cleaned source code.

    Or, you can use a comment like this to update the object-level (i.e. non-version-specific) comments for the object in sqvler.tblSchemaManifest:


    /*/manifest Here is a comment for the object itself.

    This will be saved to sqlver.tblSchemaManifest (replacing any existing comment there), but will be stripped from the source code of the object.
    */

  • Steve Jones - SSC Editor

    SSC Guru

    Points: 717790

    David Rueter - Thursday, March 14, 2019 9:59 AM

    If you create comments before the BEGIN statement, the comments show up in the SQL Profiler command text (and other places you look at command text).  This is very annoying when trying to do analysis or performance tuning, because you see the silly comment instead of the actual command being executed.  I'd strongly suggest moving the comment to after the BEGIN so as not to cause frustrations down the road.

    This doesn't seem to be the case in the XEvents Profiler. I'd strongly suggest moving to XE for future work, though if you are pre-2012, that's not necessary.

  • SimonHolzman

    Old Hand

    Points: 388

    I like to practice "defensive" programming and so I want the system to fail if I accidentally hit "execute".

    Just having CREATE prevents this accidentally replacing the original version unless I deliberately change it to say ALTER.

    Similarly, the comments should only be in the body of the Stored Procedure so that, if the original source file gets lost, they are included in the copy of the Stored Procedure that is actually running (a good reason not to encrypt the SPs).

    In my experience, programmers are much more likely to update comments that are embedded in the code than to update notes that are in a separate file and this even applies to the original source code file. Encrypting the SP can force the programmer to use the source code file but there is still the risk that the source code file will get lost or might not be the same version as the one actively running. Thus, I update the live code and keep backups of the before and after versions.

    This is not the way it "should" be done, but it is more reliable in a small environment where there is no-one with the time and authority to enforce proper standards.

  • RonKyle

    SSC-Dedicated

    Points: 31472

    I like to practice "defensive" programming and so I want the system to fail if I accidentally hit "execute".
    Just having CREATE prevents this accidentally replacing the original version unless I deliberately change it to say ALTER.

    Me, too. Having a CREATE OR ALTER statement does not give me a good feeling.  

  • astruthers 75845

    Grasshopper

    Points: 12

    Great post. A minor point.
    If you are using VCS, TFS, or some code repository this makes sense:
     -- uspLogError logs error information in the ErrorLog table about the
     -- These notes will NOT get persisted with the object
     -- and will NOT be available to future developers scripting this object from  SSMS
     ALTER PROCEDURE [dbo].[uspLogError]
         @ErrorLogID [int] = 0 OUTPUT -- contains the ErrorLogID of the row inserted
     AS                               -- by uspLogError in the ErrorLog table
     BEGIN
         SET NOCOUNT ON;
      etc...
     END
    Problem is, quite often a developer will enherit a system, and the code reporsitory may or may not be available.
    When that happens, you might use SSMS to script the object, in order to get ther "source" code of the objects.
    In your example, those notes that come BEFORE the ALTER\CREATE statement are lost. They never got saved when the original object got created.
    If you include all notes AFTER the ALTER\CREATE statement, they get persisted with the object, and would be available to a future developer scripting out those objects.
     ALTER PROCEDURE [dbo].[uspLogError]
         @ErrorLogID [int] = 0 OUTPUT -- contains the ErrorLogID of the row inserted
     AS                               -- by uspLogError in the ErrorLog table
     -- uspLogError logs error information in the ErrorLog table about the
     -- These notes will get persisted with the object
     -- and will be available if this object gets scripted out in SSMS in the future
     BEGIN
         SET NOCOUNT ON;
      etc...
     END
    In Summary:
    Always write all notes for an object AFTER the ALTER\CREATE statement.
  • Jeff Moden

    SSC Guru

    Points: 995976

    astruthers 75845 - Thursday, March 14, 2019 2:24 PM

    In your example, those notes that come BEFORE the ALTER\CREATE statement are lost.

    That's not actually true if you're using SSMS.  The comments before the Create statement WILL actually be included in the stored procedure.  Try it yourself and see.

    As to  whether or not that's the correct way to do things or not, my take is that the first thing in a stored procedure should be the ALTER or CREATE statement.

    --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.
    "If you think its expensive to hire a professional to do the job, wait until you hire an amateur."--Red Adair
    "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)

  • astruthers 75845

    Grasshopper

    Points: 12

    Jeff, 
    Just tested, and stand by what I said.
    -- Comments before Create\Alter statement
    CREATE PROCEDURE sp_Bletch
     @BrandID varchar(6)
    AS
    -- Comments After Create\Alter statement
     SELECT
      [ID]
      ,[Name]
      ,[BrandID]
     FROM [dbo].[BrandItems]
     WHERE [BrandID] = @BrandID;
    GO

    Then, if you Right-Click on object, and "Script Procedure As" the external comments are not included.

  • Jeff Moden

    SSC Guru

    Points: 995976

    astruthers 75845 - Thursday, March 14, 2019 9:26 PM

    Jeff, 
    Just tested, and stand by what I said.
    -- Comments before Create\Alter statement
    CREATE PROCEDURE sp_Bletch
     @BrandID varchar(6)
    AS
    -- Comments After Create\Alter statement
     SELECT
      [ID]
      ,[Name]
      ,[BrandID]
     FROM [dbo].[BrandItems]
     WHERE [BrandID] = @BrandID;
    GO

    Then, if you Right-Click on object, and "Script Procedure As" the external comments are not included.

    You tested this in SSMS?  Interesting.  I also tested before I posted and it worked as I said both in 2008 SSMS against an SQL Server 2008 installation and SSMS revs 17.2 and 17.91 against a 2016 SQL Server installation... what version of SSMS are you using?

    Just in case there's a doubt, here's what I got when I did the very same thing you did to script the procedure back out...

    /****** Object: StoredProcedure [dbo].[uspLogError]  Script Date: 3/15/2019 12:28:54 AM ******/
    SET ANSI_NULLS ON
    GO

    SET QUOTED_IDENTIFIER ON
    GO

    -- uspLogError logs error information in the ErrorLog table about the
    -- These notes will NOT get persisted with the object
    -- and will NOT be available to future developers scripting this object from SSMS
    CREATE PROCEDURE [dbo].[uspLogError]
    @ErrorLogID [int] = 0 OUTPUT -- contains the ErrorLogID of the row inserted
    AS -- by uspLogError in the ErrorLog table
    BEGIN
    SET NOCOUNT ON;
    Select 1;
    END
    GO

    The comments where saved as a part of the stored procedure.

    --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.
    "If you think its expensive to hire a professional to do the job, wait until you hire an amateur."--Red Adair
    "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)

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

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