Read comma separated values and insert in SQL Server 2016 table using String_Split

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.

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:

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