SQL Table and Merge Statement

Question

I am new to the code review area of Stack and welcome all positive criticism.

I would be grateful if someone could review my code below and let me know where I can improve or have gone wrong that I cannot see.

The only thing that I have left is to write the delete and would appreciate help in doing this is possible too please.

My code is

 CREATE TABLE CRM_PERS_CONTACT_INFORMAITON(
 SEQ_ID INT IDENTITY(1,1) not null primary key,
 PERS_ID INT NOT NULL,
 PREFERRED BIT DEFAULT (0), 
 CEASED_DATE DATETIME,
 COMMUNICATION_TYPE VARCHAR(50),
 COMMUNICATION_VALUE VARCHAR(255),
 DATE_ADDED DATETIME DEFAULT (GETDATE()),
 ADDED_BY VARCHAR(50),
 UPDATED_DATE DATETIME DEFAULT(GETDATE()),
 UPDATED_BY VARCHAR(50)
)
GO
CREATE TABLE CRM_PERS_CONTACT_INFORMAITON_HISTORY(
 SEQ_ID INT IDENTITY(1,1) not null primary key,
 PERS_ID INT NOT NULL,
 PREFERRED BIT DEFAULT (0), 
 CEASED_DATE DATETIME,
 COMMUNICATION_TYPE VARCHAR(50),
 COMMUNICATION_VALUE VARCHAR(255),
 DATE_ADDED DATETIME DEFAULT (GETDATE()),
 ADDED_BY VARCHAR(50),
 ACTION_TYPE VARCHAR(20)
 )
 GO
CREATE PROCEDURE MERGE_CRM_PERS_CONTACT_INFORMATION
@SEQ_ID INT,
@PERS_ID INT ,
@PREFERRED BIT , 
@CEASED_DATE DATETIME,
@DATE_ADDED DATETIME, 
@ADDED_BY VARCHAR,
@UPDATED_DATE DATETIME,
@UPDATED_BY VARCHAR,
@COMMUNICATION_TYPE VARCHAR,
@COMMUNICATION_TYPE_VALUE VARCHAR,
@ACTION_TYPE VARCHAR
AS
BEGIN
MERGE CRM_PERS_CONTACT_INFORMAITON as Target
USING (select 
 @PERS_ID 
,@PREFERRED 
,@CEASED_DATE 
,@DATE_ADDED 
,@ADDED_BY 
,@UPDATED_DATE 
,@UPDATED_BY 
,@COMMUNICATION_TYPE 
,@COMMUNICATION_TYPE_VALUE 
) AS Source(
PERS_ID 
,PREFERRED 
,CEASED_DATE 
,DATE_ADDED 
,ADDED_BY 
,UPDATED_DATE 
,UPDATED_BY 
,COMMUNICATION_TYPE 
,COMMUNICATION_TYPE_VALUE
)
ON (Source.PERS_ID = Target.PERS_ID
AND Source.COMMUNICATION_TYPE = Target.COMMUNICATION_TYPE)
WHEN MATCHED THEN 
 update SET 
 COMMUNICATION_TYPE = @COMMUNICATION_TYPE
 ,COMMUNICATION_VALUE = @COMMUNICATION_TYPE_VALUE
 ,UPDATED_DATE = GETDATE()
 ,UPDATED_BY = @UPDATED_BY
 ,CEASED_DATE = @CEASED_DATE
 ,PREFERRED = @PREFERRED
WHEN NOT MATCHED BY TARGET THEN
 INSERT (
 PERS_ID 
 ,PREFERRED 
 ,CEASED_DATE 
 ,COMMUNICATION_TYPE
 ,COMMUNICATION_VALUE
 ,DATE_ADDED 
 ,ADDED_BY 
 ,UPDATED_DATE 
 ,UPDATED_BY 
 )
 VALUES (
 @PERS_ID 
 ,@PREFERRED 
 ,@CEASED_DATE 
 ,@COMMUNICATION_TYPE
 ,@COMMUNICATION_TYPE_VALUE
 ,@DATE_ADDED 
 ,@ADDED_BY 
 ,GETDATE() 
 ,@UPDATED_BY 
 )
WHEN NOT MATCHED BY SOURCE THEN 
DELETE;
SET @ACTION_TYPE = $action
INSERT INTO CRM_PERS_CONTACT_INFORMAITON_HISTORY(
 PERS_ID 
,PREFERRED 
,CEASED_DATE 
,COMMUNICATION_TYPE
,COMMUNICATION_VALUE
,DATE_ADDED 
,ADDED_BY
,ACTION_TYPE 
)
VALUES (
 @PERS_ID 
,@PREFERRED 
,@CEASED_DATE 
,@COMMUNICATION_TYPE
,@COMMUNICATION_TYPE_VALUE
,GETDATE() 
,@ADDED_BY
,@ACTION_TYPE
);
END 
GO

Answer 1

When you declare your variables, like @COMMUNICATION_TYPE VARCHAR, you really need to specify a length.

MSDN

When n is not specified in a data definition or variable declaration statement, the default length is 1. When n is not specified when using the CAST and CONVERT functions, the default length is 30

So go ahead and declare them 20, 50, and 250, respectively, as you did in your table declarations

Additionally, in order to SET @ACTION_TYPE = $action you need to output this from your MERGE() statement.

...
WHEN NOT MATCHED BY SOURCE THEN 
DELETE
OUTPUT $action INTO @ACTION_TYPE;

More commonly MERGE() would be used with a set. That is, you would take in a table variable into your STORED PROCEDURE and your MERGE statement would conduct it's logic on each row. Thus, you could have some rows that were INSERTED, some that were UPDATED, and some that were DELETED. In this instance, you would store the OUTPUT to a table variable or temp table.

CREATE TABLE #MyTempTable(...)
...
MERGE
...
OUTPUT $action, deleted.*, inserted.* INTO #MyTempTable; 
INSERT INTO CRM_PERS_CONTACT_INFORMAITON_HISTORY
SELECT(...) FROM #MyTempTable

• user127535: Yes, i just realised that when i was testing it and saw that I was only getting one character.
• user127535: One thing I am having an issue with is the $action I want to determine if its an insert update or delete so that I can update my history table. Are you able to help
• S3S: Sure @SimonPrice I added some information with that.