1
\$\begingroup\$

I'm looking for any suggestions on how to improve my T-SQL for this procedure. It works okay, but I'd like to get advice to improve it from more experienced SQL developers. This procedure adds 4 columns to a table for row level auditing: CreatedId, CreatedDate, ModifiedId and ModifiedDate. It also creates a trigger for updating the modified columns as well as default constraints for setting the initial values. Also, should I create a database to house these administrative stored procedures e.g. [AdminTasks].[dbo].[RowLevelAuditAdd]?

For reference, this is using version Microsoft SQL Server 2016.

Here is the stored procedure create statement for review:

CREATE PROCEDURE [dbo].[RowLevelAuditAdd] 
      @SchemaName NVARCHAR(50) = NULL
    , @TableName NVARCHAR(50) = NULL
AS
BEGIN

/*
'--------------------------------------------------------------------------------------------------------------------
' Purpose: Adds row level auditing columns to a table
' Example: EXEC dbo.RowLevelAuditAdd 'dbo', 'ORGN';
' Note:    The table must have a primary key to create the update trigger
'--------------------------------------------------------------------------------------------------------------------

    -----------------------------------------------------
    -->>>>>>>>>>>>>>>>> FOR DEBUGGING <<<<<<<<<<<<<<<<<<<
    -----------------------------------------------------
    BEGIN
    DECLARE @SchemaName NVARCHAR(50)
    DECLARE @TableName NVARCHAR(50) 
    SET @SchemaName = 'dbo'
    SET @TableName = 'ORGN'
    -----------------------------------------------------
    -----------------------------------------------------

*/

    SET XACT_ABORT ON
    BEGIN TRANSACTION;
    SET NOCOUNT ON

    DECLARE @SqlCommand NVARCHAR(1000)
    DECLARE @TableKey NVARCHAR(1000)
    DECLARE @UserName NVARCHAR(50)
    DECLARE @CreatedId NVARCHAR(50)
    DECLARE @CreatedDate NVARCHAR(50)
    DECLARE @ModifiedId NVARCHAR(50)
    DECLARE @ModifiedDate NVARCHAR(50)
    DECLARE @TodayDate NVARCHAR(50)

    SET @CreatedId = 'CreatedId'
    SET @CreatedDate = 'CreatedDate'
    SET @ModifiedId = 'ModifiedId'
    SET @ModifiedDate = 'ModifiedDate'
    SET @UserName = LOWER(LEFT(RIGHT(SYSTEM_USER,(LEN(SYSTEM_USER)-CHARINDEX('\',SYSTEM_USER))), 50))
    SET @TodayDate = FORMAT(GETDATE(), 'dd-MMM-yyyy HH:mm:ss', 'en-US' )

    PRINT '=====================================================================';
    PRINT 'START - ALTER [' + @SchemaName + '].[' + @TableName + ']... ';

        IF COL_LENGTH(@SchemaName + '.' + @TableName, @CreatedId) IS NULL
        BEGIN

            PRINT '=====================================================================';
            PRINT 'START - ADD COLUMN [' + @CreatedId + ']... ';

            PRINT '1. alter table add ' + @CreatedId + ' column... ';
            SET @SqlCommand = 'ALTER TABLE [' + @SchemaName + '].[' + @TableName + '] ADD [' + @CreatedId + '] [NVARCHAR](50) NULL'
            EXEC (@SqlCommand)

            PRINT '2. update new column to a value... ' + @UserName;            
            SET @SqlCommand = 'UPDATE [' + @SchemaName + '].[' + @TableName + '] SET [' + @CreatedId + '] =''' + @UserName + ''' WHERE [' + @CreatedId + '] IS NULL'
            EXEC (@SqlCommand)

            PRINT '3. alter table alter new column add constraints... ';
            SET @SqlCommand = 'ALTER TABLE [' + @SchemaName + '].[' + @TableName + '] ALTER COLUMN [' + @CreatedId + '] [NVARCHAR](50) NOT NULL'
            EXEC (@SqlCommand)

            SET @SqlCommand = 'ALTER TABLE [' + @SchemaName + '].[' + @TableName + '] ADD CONSTRAINT [DF_' + @TableName + '_' + @CreatedId + ']  DEFAULT (LOWER(LEFT(RIGHT(SYSTEM_USER,(LEN(SYSTEM_USER)-CHARINDEX(''\'',SYSTEM_USER))), 50))) FOR [' + @CreatedId + ']'
            EXEC (@SqlCommand)

            PRINT '4. add column description... ';
            EXEC sys.sp_addextendedproperty @name=N'MS_Description', @value=N'Who created the record' , @level0type=N'SCHEMA',@level0name=@SchemaName, @level1type=N'TABLE',@level1name=@TableName, @level2type=N'COLUMN',@level2name=@CreatedId;

            PRINT 'END - ADD COLUMN [' + @CreatedId + ']... ';
            PRINT '=====================================================================';
        END

        IF COL_LENGTH(@SchemaName + '.' + @TableName, @CreatedDate) IS NULL
        BEGIN

            PRINT '=====================================================================';
            PRINT 'START - ADD COLUMN [' + @CreatedDate + ']... ';

            PRINT '1. alter table add ' + @CreatedDate + ' column... ';
            SET @SqlCommand = 'ALTER TABLE [' + @SchemaName + '].[' + @TableName + '] ADD [' + @CreatedDate + '] [DATETIME] NULL'
            EXEC (@SqlCommand)

            PRINT '2. update new column to a value... ' + @TodayDate;   
            SET @SqlCommand = 'UPDATE [' + @SchemaName + '].[' + @TableName + '] SET [' + @CreatedDate + '] = ''' + @TodayDate+ ''' WHERE [' + @CreatedDate + '] IS NULL'
            EXEC (@SqlCommand)

            PRINT '3. alter table alter new column add constraints... ';
            SET @SqlCommand = 'ALTER TABLE [' + @SchemaName + '].[' + @TableName + '] ALTER COLUMN [' + @CreatedDate + '] [DATETIME] NOT NULL'
            EXEC (@SqlCommand)

            SET @SqlCommand = 'ALTER TABLE [' + @SchemaName + '].[' + @TableName + '] ADD CONSTRAINT [DF_' + @TableName + '_' + @CreatedDate + ']  DEFAULT (GETDATE()) FOR [' + @CreatedDate + ']'
            EXEC (@SqlCommand)

            PRINT '4. add column description... ';
            EXEC sys.sp_addextendedproperty @name=N'MS_Description', @value=N'The date and time the record was created' , @level0type=N'SCHEMA',@level0name=@SchemaName, @level1type=N'TABLE',@level1name=@TableName, @level2type=N'COLUMN',@level2name=@CreatedDate;

            PRINT 'END - ADD COLUMN [' + @CreatedDate + ']... ';
            PRINT '=====================================================================';
        END

        IF COL_LENGTH(@SchemaName + '.' + @TableName, @ModifiedId) IS NULL
        BEGIN

            PRINT '=====================================================================';
            PRINT 'START - ADD COLUMN [' + @ModifiedId + ']... ';

            PRINT '1. alter table add ' + @ModifiedId + ' column... ';
            SET @SqlCommand = 'ALTER TABLE [' + @SchemaName + '].[' + @TableName + '] ADD [' + @ModifiedId + '] [NVARCHAR](50) NULL'
            EXEC (@SqlCommand)

            PRINT '2. update new column to a value... ' + @UserName;            
            SET @SqlCommand = 'UPDATE [' + @SchemaName + '].[' + @TableName + '] SET [' + @ModifiedId + '] =''' + @UserName + ''' WHERE [' + @ModifiedId + '] IS NULL'
            EXEC (@SqlCommand)

            PRINT '3. alter table alter new column add constraints... ';
            SET @SqlCommand = 'ALTER TABLE [' + @SchemaName + '].[' + @TableName + '] ALTER COLUMN [' + @ModifiedId + '] [NVARCHAR](50) NOT NULL'
            EXEC (@SqlCommand)

            SET @SqlCommand = 'ALTER TABLE [' + @SchemaName + '].[' + @TableName + '] ADD CONSTRAINT [DF_' + @TableName + '_' + @ModifiedId + ']  DEFAULT (LOWER(LEFT(RIGHT(SYSTEM_USER,(LEN(SYSTEM_USER)-CHARINDEX(''\'',SYSTEM_USER))), 50))) FOR [' + @ModifiedId + ']'
            EXEC (@SqlCommand)

            PRINT '4. add column description... ';
            EXEC sys.sp_addextendedproperty @name=N'MS_Description', @value=N'Who modified the record' , @level0type=N'SCHEMA',@level0name=@SchemaName, @level1type=N'TABLE',@level1name=@TableName, @level2type=N'COLUMN',@level2name=@ModifiedId;

            PRINT 'END - ADD COLUMN [' + @ModifiedId + ']... ';
            PRINT '=====================================================================';
        END

        IF COL_LENGTH(@SchemaName + '.' + @TableName, @ModifiedDate) IS NULL
        BEGIN

            PRINT '=====================================================================';
            PRINT 'START - ADD COLUMN [' + @ModifiedDate + ']... ';

            PRINT '1. alter table add ' + @ModifiedDate + ' column... ';
            SET @SqlCommand = 'ALTER TABLE [' + @SchemaName + '].[' + @TableName + '] ADD [' + @ModifiedDate + '] [DATETIME] NULL'
            EXEC (@SqlCommand)

            PRINT '2. update new column to a value... ' + @TodayDate;   
            SET @SqlCommand = 'UPDATE [' + @SchemaName + '].[' + @TableName + '] SET [' + @ModifiedDate + '] = ''' + @TodayDate+ ''' WHERE [' + @ModifiedDate + '] IS NULL'
            EXEC (@SqlCommand)

            PRINT '3. alter table alter new column add constraints... ';
            SET @SqlCommand = 'ALTER TABLE [' + @SchemaName + '].[' + @TableName + '] ALTER COLUMN [' + @ModifiedDate + '] [DATETIME] NOT NULL'
            EXEC (@SqlCommand)

            SET @SqlCommand = 'ALTER TABLE [' + @SchemaName + '].[' + @TableName + '] ADD CONSTRAINT [DF_' + @TableName + '_' + @ModifiedDate + ']  DEFAULT (GETDATE()) FOR [' + @ModifiedDate + ']'
            EXEC (@SqlCommand)

            PRINT '4. add column description... ';
            EXEC sys.sp_addextendedproperty @name=N'MS_Description', @value=N'The date and time the record was modified' , @level0type=N'SCHEMA',@level0name=@SchemaName, @level1type=N'TABLE',@level1name=@TableName, @level2type=N'COLUMN',@level2name=@ModifiedDate;

            PRINT 'END - ADD COLUMN [' + @ModifiedDate + ']... ';
            PRINT '=====================================================================';
        END

        IF NOT EXISTS (SELECT * FROM sys.triggers WHERE object_id = OBJECT_ID(N'[' + @SchemaName + '].[TR_' + @TableName + '_LAST_UPDATED]'))
        BEGIN

            PRINT '=====================================================================';
            PRINT 'START - ADD TRIGGER [TR_' + @TableName + '_LAST_UPDATED]... ';

            PRINT '1. get primary key from information schema';
            SELECT   
                @TableKey = COALESCE(@TableKey, '') + CASE WHEN ORDINAL_POSITION = 1 THEN 'ON' ELSE 'AND' END + ' t.' + COLUMN_NAME + ' = i.' + COLUMN_NAME + ' '
            FROM 
                INFORMATION_SCHEMA.KEY_COLUMN_USAGE
            WHERE 
                1=1
                AND OBJECTPROPERTY(OBJECT_ID(CONSTRAINT_SCHEMA + '.' + QUOTENAME(CONSTRAINT_NAME)), 'IsPrimaryKey') = 1
                AND TABLE_NAME = @TableName AND TABLE_SCHEMA = @SchemaName
            ORDER BY 
                ORDINAL_POSITION

            PRINT '2. build trigger dynamically';
            SET @SqlCommand = 'CREATE TRIGGER [' + @SchemaName + '].[TR_' + @TableName + '_LAST_UPDATED]' + CHAR(13);
            SET @SqlCommand += 'ON [' + @SchemaName + '].[' + @TableName + ']' + CHAR(13);
            SET @SqlCommand += 'AFTER UPDATE' + CHAR(13);
            SET @SqlCommand += 'AS' + CHAR(13);
            SET @SqlCommand += 'BEGIN' + CHAR(13);
            SET @SqlCommand += CHAR(9) + 'IF NOT UPDATE(' + @ModifiedDate + ')' + CHAR(13);
            SET @SqlCommand += CHAR(9) + 'BEGIN' + CHAR(13);
            SET @SqlCommand += CHAR(9) + CHAR(9) + 'UPDATE t' + CHAR(13);
            SET @SqlCommand += CHAR(9) + CHAR(9) + 'SET' + CHAR(13);
            SET @SqlCommand += CHAR(9) + CHAR(9) + '  t.' + @ModifiedDate + ' = CURRENT_TIMESTAMP' + CHAR(13);
            SET @SqlCommand += CHAR(9) + CHAR(9) + ', t.' + @ModifiedId + ' = LOWER(LEFT(RIGHT(SYSTEM_USER,(LEN(SYSTEM_USER)-CHARINDEX(''\'',SYSTEM_USER))), 50))' + CHAR(13);
            SET @SqlCommand += CHAR(9) + CHAR(9) + 'FROM [' + @SchemaName + '].[' + @TableName + '] AS t' + CHAR(13);
            SET @SqlCommand += CHAR(9) + CHAR(9) + 'INNER JOIN inserted AS i' + CHAR(13);
            SET @SqlCommand += CHAR(9) + CHAR(9) + @TableKey + ';' + CHAR(13);
            SET @SqlCommand += CHAR(9) + 'END' + CHAR(13);
            SET @SqlCommand += 'END;' + CHAR(13);
            EXEC (@SqlCommand)

            PRINT '3. enable trigger';
            SET @SqlCommand = 'ALTER TABLE [' + @SchemaName + '].[' + @TableName + '] ENABLE TRIGGER [TR_' + @TableName + '_LAST_UPDATED]';
            EXEC (@SqlCommand)

            PRINT 'END - ADD TRIGGER [' + @ModifiedDate + ']... ';
            PRINT '=====================================================================';
        END


    PRINT 'END - ALTER [' + @SchemaName + '].[' + @TableName + ']... ';
    PRINT '=====================================================================';

    --PRINT '******* ROLLBACK TRANSACTION ******* ';
    --ROLLBACK TRANSACTION;

    PRINT '******* COMMIT TRANSACTION ******* ';
    COMMIT TRANSACTION;

END

GO

Example of table script before the stored procedure is run:

--create the table
CREATE TABLE [dbo].[ORGN] (
    [ORGN_ID]       INT             IDENTITY (1, 1) NOT NULL,
    [ORGN_ABBR]     VARCHAR (5)     NOT NULL,
    [ORGN_NAME]     VARCHAR (100)   NULL
);

--Add the keys
ALTER TABLE [dbo].[ORGN]
    ADD CONSTRAINT [PK_ORGN] PRIMARY KEY NONCLUSTERED ([ORGN_ID] ASC);
ALTER TABLE [dbo].[ORGN]
    ADD CONSTRAINT [UK_ORGN] UNIQUE NONCLUSTERED ([ORGN_ABBR] ASC);

--Add some test records
INSERT INTO dbo.ORGN (ORGN_ABBR, ORGN_NAME) VALUES('AABA', 'Altaba Inc');
INSERT INTO dbo.ORGN (ORGN_ABBR, ORGN_NAME) VALUES('AAPL', 'Apple Inc');
INSERT INTO dbo.ORGN (ORGN_ABBR, ORGN_NAME) VALUES('GOOG', 'Alphabet Inc');
INSERT INTO dbo.ORGN (ORGN_ABBR, ORGN_NAME) VALUES('MSFT', 'Microsoft Corporation');
INSERT INTO dbo.ORGN (ORGN_ABBR, ORGN_NAME) VALUES('TSLA', 'Tesla Inc');

Example of the Results

results

Example of the procedure with parameters:

EXEC dbo.RowLevelAuditAdd 'dbo', 'ORGN';

Example of table after the stored procedure has been run:

CREATE TABLE [dbo].[ORGN] (
    [ORGN_ID]      INT           IDENTITY (1, 1) NOT NULL,
    [ORGN_ABBR]    VARCHAR (5)   NOT NULL,
    [ORGN_NAME]    VARCHAR (100) NULL,
    [CreatedId]    NVARCHAR (50) NOT NULL,
    [CreatedDate]  DATETIME      NOT NULL,
    [ModifiedId]   NVARCHAR (50) NOT NULL,
    [ModifiedDate] DATETIME      NOT NULL
);

ALTER TABLE [dbo].[ORGN]
    ADD CONSTRAINT [PK_ORGN] PRIMARY KEY NONCLUSTERED ([ORGN_ID] ASC);

ALTER TABLE [dbo].[ORGN]
    ADD CONSTRAINT [UK_ORGN] UNIQUE NONCLUSTERED ([ORGN_ABBR] ASC);

ALTER TABLE [dbo].[ORGN]
    ADD CONSTRAINT [DF_ORGN_CreatedDate] DEFAULT (getdate()) FOR [CreatedDate];

ALTER TABLE [dbo].[ORGN]
    ADD CONSTRAINT [DF_ORGN_CreatedId] DEFAULT (lower(left(right(suser_sname(),len(suser_sname())-charindex('\',suser_sname())),(50)))) FOR [CreatedId];

ALTER TABLE [dbo].[ORGN]
    ADD CONSTRAINT [DF_ORGN_ModifiedDate] DEFAULT (getdate()) FOR [ModifiedDate];

ALTER TABLE [dbo].[ORGN]
    ADD CONSTRAINT [DF_ORGN_ModifiedId] DEFAULT (lower(left(right(suser_sname(),len(suser_sname())-charindex('\',suser_sname())),(50)))) FOR [ModifiedId];


CREATE TRIGGER [dbo].[TR_ORGN_LAST_UPDATED]
ON [dbo].[ORGN]
AFTER UPDATE
AS
BEGIN
    IF NOT UPDATE(ModifiedDate)
    BEGIN
        UPDATE t
        SET
          t.ModifiedDate = CURRENT_TIMESTAMP
        , t.ModifiedId = LOWER(LEFT(RIGHT(SYSTEM_USER,(LEN(SYSTEM_USER)-CHARINDEX('\',SYSTEM_USER))), 50))
        FROM [dbo].[ORGN] AS t
        INNER JOIN inserted AS i
        ON t.ORGN_ID = i.ORGN_ID ;
    END
END;

Results

results

\$\endgroup\$
  • \$\begingroup\$ All questions must have an actual language tag, in this case sql. \$\endgroup\$ – Jamal Sep 9 '18 at 17:08
  • 1
    \$\begingroup\$ @aduguid it would've been sufficient. You will reach a much larger audience if you also tag with sql. A lot of concerns can be adressed by reviewers that are not necessarily proficient with the specific dialect you're using. You'd be missing out on these people if you only tagged with your specific dialect \$\endgroup\$ – Vogel612 Sep 9 '18 at 22:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.