1
\$\begingroup\$

I know that this question has been asked many times over many years and I have read every one of them but I wanted to get a modern solution since software has changed and String_Split has been introduced in SQL Server 2016.

I have a stored procedure that splits a comma-separated string and inserts the values into a table after it deletes the value if it is already there. The code works but sometimes I write code that is totally convoluted.

ALTER PROCEDURE [dbo].[Admin_Save_PMIDS]
    @MemberID INT,
    @PMIDList VARCHAR(MAX)
AS
BEGIN
    SELECT Split.a.value('.', 'VARCHAR(100)') AS PMID
    INTO #pmids
    FROM   
        (SELECT    
             CAST('<M>' + REPLACE(@PMIDList, ',', '</M><M>') + '</M>' AS XML) AS String) AS A
    CROSS APPLY 
        String.nodes('/M') AS Split (a)   

    DELETE FROM [dbo].[PublicationTable]
    WHERE (MemberID = @MemberID) 
     AND  PMID IN (SELECT PMID FROM #pmids)

    INSERT INTO [dbo].[PublicationTable] (PersonID, PMID, MemberID)
        SELECT        
            @MemberID, PMID, @MemberID
        FROM            
            [#pmids]
        WHERE        
            (LEN(PMID) > 0)

    DROP TABLE #pmids
END

As you can see I'm creating a temporary table and using an XML structure as a cross apply. The code works but since I'm doing a lot of updating in the application I figured I'd get some current opinions.

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

The following are a few suggestions on how I'd write the stored procedure.

Source Control

If you don't already have a database project, create one in Visual Studio. Then check it in to source control. Microsoft Azure DevOps Services is free & private for teams of 5 or less (this is per project, so 5 developers per project). Then you'll be able to track changes you make to your stored procedures, views, tables, etc.

Error Handling

Use Try, Catch blocks to handle your errors. You can then write them to a log file or table.

Tranactions

Use transactions for rolling back your queries during testing. If a transaction is successful, all of the data modifications made during the transaction are committed and become a permanent part of the database. If a transaction encounters errors and must be cancelled or rolled back, then all of the data modifications are erased.

Common Table Expressions (CTE)

CTE's in your SQL help with documentation. The expression name can then let other developers know why you used that expression e.g. current_suppliers or active_projects.

Merge

If I need to perform more than one DML query on a table, I usually will use a MERGE statement.

Formatting

I would download the following tool for SSMS and Visual Studio, Poor Man's T-Sql Formatter and on GitHub. I use it when I have to edit other developer's code. It's a great way to standardize your SQL. I find it does most of the formatting for me, but I'll still make a few changes after.

Here are the settings I used:

screenshot

Commas

I would put the commas in front to clearly define new columns. Versus code wrapped in multiple lines. It also makes trouble-shooting code easier.

e.g. , S.[PersonID]

Estimated Execution Plan

It's a good idea to check the Estimated Execution Plan. The shortcut in Microsoft SQL Server Management Studio (SSMS) is Ctrl + L. You can even run 2 queries in the same tab to compare the plans. Then you can see differences in the query plans using STRING_SPLIT.


Revised SQL

Without table definitions and sample records I was unable to test this, but it should give you a good start.

ALTER PROCEDURE [dbo].[Admin_Save_PMIDS] 
      @MemberID INT
    , @PMIDList VARCHAR(MAX)
AS
BEGIN

    BEGIN TRY

        SET XACT_ABORT ON;
        BEGIN TRANSACTION;

            WITH
            pm_ids
            AS
            (
                SELECT 
                      [PMID] = Split.a.value('.', 'VARCHAR(100)')
                    , [PersonID] = @MemberID
                    , [MemberID] = @MemberID
                FROM 
                    (
                    SELECT CAST('<M>' + REPLACE(@PMIDList, ',', '</M><M>') + '</M>' AS XML) AS String
                    ) AS A
                CROSS APPLY String.nodes('/M') AS Split(a)
            )
            MERGE [dbo].[PublicationTable] AS T
            USING pm_ids AS S
            ON T.[PMID] = S.[PMID] AND T.[MemberID] = S.[MemberID] --< Update the join to the correct unique key for the table
            WHEN NOT MATCHED BY TARGET
            THEN INSERT
            (
                  [PMID]
                , [PersonID]
                , [MemberID]
            )
            VALUES
            (
                  S.[PMID]
                , S.[PersonID]
                , S.[MemberID]
            )
            WHEN NOT MATCHED BY SOURCE --< You may want to add a WHERE clause here
            THEN DELETE
            --OUTPUT @@SERVERNAME AS [ServerName], DB_NAME() AS [DatabaseName], $action, inserted.*, deleted.* --< show the changed records here
            ;

        ROLLBACK TRANSACTION; --< use the rollback for testing
        --COMMIT TRANSACTION; --< use the commit for production

    END TRY
    BEGIN CATCH
        --you can write this to a log table 
        SELECT 
              [ServerName] = @@SERVERNAME
            , [DatabaseName] = DB_NAME()
            , [ErrorProcedure] = ERROR_PROCEDURE()
            , [ErrorNumber] = ERROR_NUMBER()
            , [ErrorSeverity] = ERROR_SEVERITY()
            , [ErrorState] = ERROR_STATE()
            , [ErrorLine] = ERROR_LINE()
            , [ErrorMessage] = ERROR_MESSAGE()
            , [ErrorDateTime] = GETDATE();

        -- Test whether the transaction is uncommittable.  
        IF (XACT_STATE()) = -1  
        BEGIN  
            PRINT  
                N'The transaction is in an uncommittable state.' + 'Rolling back transaction.';
            ROLLBACK TRANSACTION;  
        END;  

        -- Test whether the transaction is committable.  
        IF (XACT_STATE()) = 1  
        BEGIN  
            PRINT  
                N'The transaction is committable.' + 'Committing transaction.';
            COMMIT TRANSACTION;     
        END;  

    END CATCH

END
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.