0

I'm trying to do some dynamic SQL like so in order to create an audit log that just shows the changes.

When I run that I'm getting an error stating

Must declare the scalar variable "@OldValueOut".

I thought I did declare that, via the second parameter to the sp_executesql call.

ALTER TRIGGER [dbo].[Lab_SubSpace_Contact_Changed] 
   ON [dbo].[Lab_SubSpace_Contact]
   AFTER INSERT,DELETE,UPDATE
AS 
BEGIN
    SET NOCOUNT ON;

    DECLARE @sql VARCHAR(4000),
            @sqlDeleted VARCHAR(4000),
            @sqlInserted NVARCHAR(4000),
            @columns NVARCHAR(4000),
            @item VARCHAR(80),
            @pos INT,
            @oldValue VARCHAR(100),
            @newValue VARCHAR(100),
            @tableName VARCHAR(80),
            @pkName VARCHAR(80);

    SET @tableName = 'Lab_SubSpace_Contact';
    SET @pkName = 'SubSpace_Id';

    SELECT * INTO #deleted FROM deleted;
    SELECT * INTO #inserted from inserted;

    SET @columns = STUFF((SELECT ',' + name
                          FROM sys.columns
                          WHERE object_id = OBJECT_ID(@tableName)
                            AND SUBSTRING(COLUMNS_UPDATED(), ((column_id - 1) / 8 + 1), 1) & (POWER(2, ((column_id - 1) % 8 + 1) - 1)) = POWER(2, (column_id - 1) % 8)
                          FOR XML PATH('')
                         ), 1, 1, '');

    WHILE LEN(@columns) > 0
    BEGIN
        SET @pos = CHARINDEX(',', @columns);
        IF @pos = 0
        BEGIN
            SET @item = @columns
        END;
        ELSE
        BEGIN
            SET @item = SUBSTRING(@columns, 1, @pos - 1);
        END;

        SET @sqlDeleted = N'SELECT @OldValueOut = ' + QUOTENAME(@item) + 'FROM #deleted WHERE ' + QUOTENAME(@pkName) + ' = ' + CONVERT(VARCHAR(100), @pkName);
        print @sqlDeleted;
        EXECUTE sp_executesql @sqlDeleted, N'@OldValueOut NVARCHAR(100) OUTPUT', @OldValueOut = @oldValue OUTPUT;

        SET @sqlInserted = N'SELECT @NewValueOut = ' + @item + ' FROM inserted WHERE ' + QUOTENAME(@pkName) + ' = ' + CONVERT(VARCHAR(100), @pkName);
        EXECUTE sp_executesql @sqlDeleted, N'@NewValueOut NVARCHAR(100) OUTPUT', @NewValueOut = @newValue OUTPUT;

        IF (LTRIM(RTRIM(@newValue)) != LTRIM(RTRIM(@oldValue)))
        BEGIN
            SET @sql = 'INSERT INTO [dbo].[Audit_Log] (Table_Name, Primary_Key, Column_Name, Old_Value, New_Value, Modified, Updated_By_WWID) VALUES (' +
                        QUOTENAME(@tableName, '''') + ',' +
                        QUOTENAME(@pkName, '''') + ',' + 
                        QUOTENAME(@item, '''') + ',' +
                        QUOTENAME(@oldValue, '''') + ',' +
                        QUOTENAME(@newValue, '''') + ',' +
                        GETDATE() + ',' +
                        '1234' + ')';
            EXEC(@sql);
        END;

        SET @item = '';
        SET @newValue = '';
        SET @oldValue = ''

        IF @pos = 0 
        BEGIN
            SET @columns = '';
        END;
        ELSE
        BEGIN
            SET @columns = SUBSTRING(@columns, @pos + 1, LEN(@columns) - @pos);
        END;
    END ;

    DROP TABLE #inserted;
    DROP TABLE #deleted;
END
Gargoyle
  • 9,590
  • 16
  • 80
  • 145
  • you may need to post the whole code – Lamak May 16 '18 at 21:04
  • @Lamak Question updated with the whole trigger – Gargoyle May 16 '18 at 21:08
  • Curious about use of `#inserted`, `#deleted`. Why not just use `inserted`, `deleted`? – STLDev May 16 '18 at 21:12
  • Can't answer that. A colleague told me to do it this way once for some reason and I've just kept it that way. Probably extra overhead I don't need here. – Gargoyle May 16 '18 at 21:14
  • I would ditch the use of the redundant temporary tables. No value add. – STLDev May 16 '18 at 21:15
  • 1
    @STLDeveloper I'm not so sure it is redundant. I'm guessing that the seudo tables `INSERTED` and `DELETED` are valid only in the scope of the trigger....and when using dynamic SQL you execute it in another scope – Lamak May 16 '18 at 21:16
  • My `@sqlDeleted` is wrong still. How do I reference the *value* of the `@pkName`? In other works, `@pkName = 'SubSpace_Id'` here, and that's the primary key that I want to grab the value of. – Gargoyle May 16 '18 at 21:19
  • Voting to close as not reproducible. – STLDev May 16 '18 at 21:19

2 Answers2

1

I never remember the right way to order declarations for variables with sp_executesql. So, I just cheat and use the same variable name:

DECLARE @oldValueOut VARCHAR(100);

    SET @sqlDeleted = N'
SELECT @OldValueOut = ' + QUOTENAME(@item) + '
FROM #deleted 
WHERE ' + QUOTENAME(@pkName) + ' = ' + CONVERT(VARCHAR(100), @pkName);

EXECUTE sp_executesql @sqlDeleted, 
                      N'@OldValueOut NVARCHAR(100) OUTPUT',
                      @OldValueOut = @oldValueOut OUTPUT;

Here is a simplified version that shows that this works for @oldValueOut.

Gordon Linoff
  • 1,242,037
  • 58
  • 646
  • 786
0

Whoops. My second sp_execute was also referencing the old value instead of the new value. Thanks everyone.

Gargoyle
  • 9,590
  • 16
  • 80
  • 145