SQL Trigger - Audting Config Table - Send Email with update details

Question

Keep in mind i'm not an expert or developer, working in IT operations and improving my coding skills.

Goal: Trigger to send email when a specific configuration table is updated or deleted. Should never happen unless done for a release

CREATE TRIGGER [Cfg].[t_tablename_TrackConfigChanges] ON [Cfg].[tablename]
AFTER INSERT, UPDATE, DELETE
AS

     /* All purpose trigger for auditing of config tables for Insertion, Update, and Delete */

     --If no rows were updated, then end this trigger (caused by an update or insert affects no rows)
     IF @@ROWCOUNT = 0
         BEGIN
             RETURN
         END

     SET NOCOUNT ON;

     DECLARE @isInsert BIT
     DECLARE @isdelete BIT


     --If something was inserted flag our variable
     IF EXISTS
             (
               SELECT TOP 1 1
               FROM inserted
             )
         BEGIN
             SET @isInsert = 1
         END
     --If something was deleted flag our variable
     IF EXISTS
             (
               SELECT TOP 1 1
               FROM deleted
             )
         BEGIN
             SET @isdelete = 1
         END

     /* If something was inserted, or deleted send an email about it with the specified action and the data that was changed, updated, or deleted */

     IF @isInsert = 1
        OR @isdelete = 1
         BEGIN
             BEGIN TRY

                 DECLARE @table_name VARCHAR(30)
                 DECLARE @user VARCHAR(100)
                 DECLARE @email_subject VARCHAR(1000)
                 DECLARE @body NVARCHAR(MAX)= N'';
                 DECLARE @operation VARCHAR(10)

                 /* Get the table that this trigger is executing for */

                 SELECT @table_name = OBJECT_SCHEMA_NAME(parent_id)+'.'+OBJECT_NAME(parent_id)
                 FROM sys.triggers
                 WHERE object_id = @@PROCID

                 SET @user = SYSTEM_USER


            /* Since an email cannot access the inserted or deleted tables, or any local variables 
            as it executes in a different session, I decided to append the results of the query 
            into one big string separated by carriage returns for each row */


                 IF @isInsert = 1
                    AND @isdelete = 1
                     BEGIN
                         SET @operation = 'UPDATE'


                         SELECT @body+=CHAR(13)+CHAR(10)+'columnname'+RTRIM(columnname)+' columnname'+RTRIM(columnname)+' columnname'+RTRIM(columnname)+'| INSERTED'
                         FROM inserted;
                         SELECT @body+=CHAR(13)+CHAR(10)+'columnname'+RTRIM(columnname)+' columnname'+RTRIM(columnname)+' columnname'+RTRIM(columnname)+'| DELETED'
                         FROM deleted;
                     END
                 ELSE
                     BEGIN
                         IF @isInsert = 1
                             BEGIN
                                 SET @operation = 'INSERT'
                                 SELECT @body+=CHAR(13)+CHAR(10)+'columnname'+RTRIM(columnname)+' columnname'+RTRIM(columnname)+' columnname'+RTRIM(columnname)+'| INSERTED'
                                 FROM inserted;
                             END
                         ELSE
                             BEGIN
                                 IF @isdelete = 1
                                     BEGIN
                                         SET @operation = 'DELETE'
                                         SELECT @body+=CHAR(13)+CHAR(10)+'columnname'+RTRIM(columnname)+' columnname'+RTRIM(columnname)+' columnname'+RTRIM(columnname)+'| DELETED'
                                         FROM deleted;
                                     END
                             END
                     END


                 /* Send the email to the appropriate team */

                 SET @email_subject = '[Database Configuration Monitor] '+@@ServerName+' - '
                 SET @email_subject+=@operation+' operation has been performed on '+@table_name+' by '+@user

                 EXEC msdb.dbo.sp_send_dbmail 
                      @profile_name = 'Profile', 
                      @recipients = '[email protected]', 
                      --@copy_recipients = 'Whoever I want to copy'
                      @body = @body, 
                      @subject = @email_subject

                 /* If an email fails, it sets @error to a non zero value */

                 IF @@error <> 0
                     BEGIN

                         /* Declare a dummy varaible as we dont want the transaction to fail if the email fails */

                         DECLARE @dummy BIT= 1
                     END
             END TRY
             BEGIN CATCH

             /* If anything fails, its ok as we don't want to cause a transaction to rollback due to a failure */

             END CATCH
         END      

Answer 1

If you're using SELECT TOP 1 1 in an effort to improve performance, know that this isn't always what will happen. In particular, the TOP operator can constrain the ability of the optimizer to pick the best plan. SELECT 1 inside of a EXISTS will do just fine in 99.9% of cases.

Additionally, I find it easier to read this:

SELECT @isInsert = COUNT( * )
  WHERE EXISTS( SELECT 1 FROM inserted )

This will always set @isInsert to either 1 or 0, and reduces the amount of necessary code.

---

In some cases, using functions like OBJECT_SCHEMA_NAME can actually impair performance by causing blocking (can't find a reference, but at least observationally it has been found to be true before). Also, you don't even need it - you know the name of your table already.

---

Instead of your big IF ELSE block, you could do this:

SET @operation = CASE WHEN @isInsert = 1 AND @isDelete = 1 THEN 'UPDATE'
                      WHEN @isInsert = 1 THEN 'INSERT'
                      ELSE 'DELETE' END

SET @body = STUFF( ( SELECT CHAR(13) + CHAR(10) + FormattedColumnResults + '| ' + OperationType
                       FROM ( SELECT 'columnname' + RTRIM( columnname ) FormattedColumnResults,
                                     'INSERTED' OperationType
                                FROM inserted
                              UNION ALL
                              SELECT 'columnname' + RTRIM( columnname ) FormattedColumnResults,
                                     'DELETED' OperationType
                                FROM deleted ) ModifiedRows
                      FOR XML PATH(''), TYPE ).value('(./text())[1]', 'varchar(MAX)' ), 1, 2, '' )

Here I'm relying on the fact that setting @operation is very simple, and then using STUFF to safely concatenate values. In particular, your previous result was using unsupported syntax, and could have ended up giving incorrect results. This also doesn't need to be worried about whether or not the value is actually inserted/deleted, because those tables would just be empty.

---

Lastly, if the email fails you should probably log that to a table (and include the message you would have sent) so the information can be recovered later (and someone knows to troubleshoot your trigger)