Is SELECT * ok in a Trigger. Or am I asking for trouble?

Question

I'm caught in a debate at work, and I need some advice on possible Pitfalls I could be overlooking.

Imagine a scenario where a Trigger is used to copy Deleted Records to an Audit Table. The Trigger uses SELECT *. Everyone points and shouts and tells us how bad this is.

However, if a modification is made to the Structure of the Main Table, and the Audit Table is overlooked then the Trigger will generate an error letting people know the Audit table also needs modification.

The error will be caught during testing on our DEV servers. But we need to ensure Production Matches DEV, so we allow SELECT * in Production Systems (Triggers only).

So my Question is: I'm being pushed to remove the SELECT *, but I'm not sure how else to ensure we are automatically capturing Development Errors of this nature, any ideas or is this best practice?

I've put together an example below:

--Create Test Table
CREATE TABLE dbo.Test(ID INT IDENTITY(1,1), Person VARCHAR(255))
--Create Test Audit Table
CREATE TABLE dbo.TestAudit(AuditID INT IDENTITY(1,1),ID INT, Person VARCHAR(255))

--Create Trigger on Test
CREATE TRIGGER [dbo].[trTestDelete] ON [dbo].[Test] AFTER DELETE
NOT FOR REPLICATION
AS
BEGIN
    SET NOCOUNT ON;
    INSERT  dbo.TestAudit([ID], [Person])
    SELECT  *
    FROM    deleted
END

--Insert Test Data into Test
INSERT INTO dbo.Test VALUES
('Scooby')
,('Fred')
,('Shaggy')

--Perform a delete
DELETE dbo.Test WHERE Person = 'Scooby'

UPDATE (rephrase question):

I'm a DBA and need to ensure Developers don't provide poorly thought-out Deployment scripts by contributing to our Best Practice Documentation. SELECT * causes an error in DEV when the Developer overlooks the Audit Table (this is a safety net) so the error is caught early in the development process. But somewhere in the SQL Constitution - 2nd amendment it reads "Thou shalt not use SELECT *" . So now there is a push to get rid of the Safety Net.

How would you replace the Safety Net, or should I consider this to be best practice for Triggers?

UPDATE 2: (solution)

Thankyou for all of your input, I'm not sure if I have a clear answer because this appears to be a very Grey subject. But collectively you've provided discussion points that can help our developers move forward with defining their Best Practice.

Thanks Daevin for your contribution, your answer provides the groundwork for some test mechanisms that our Developers can implement. +1

Thanks CM_Dayton, your suggestions contributing to best practice can be beneficial to anyone who is devloping Audit Triggers. +1

Big Thanks to ypercube, you've raised plenty of thought around the issues concerning tables undergoing different forms of definition changes. +1

In conclusion:

Is Select * ok in a tigger? Yes, it's a Grey area, don't blindly follow the "Select * is Bad" ideology".

Am I asking for Trouble? Yes, we do more than just add new columns to tables.

Answer 1

Typically, it is considered "lazy" programming.

Given that you're specifically inserting two values into your TestAudit table here, I'd be careful to make sure your select is also getting exactly two values. Because if, for some reason, that Test table has or ever gets a third column, this trigger will fail.

Not directly related to your question but if you're setting up an audit table, I'd also add some additional columns to your TestAudit table to...

• track the action you are auditing (delete in this case, vs inserts or updates)
• date/time column to track when the audit event occurred
• user ID column to track who carried out the action you are auditing.

So that results in a query like:

INSERT dbo.TestAudit([ID], [Person], [AuditAction], [ChangedOn], [ChangedBy])
SELECT [ID], [Person], 
   'Delete', -- or a 'D' or a numeric lookup to an audit actions table...
   GetDate(), -- or SYSDATETIME() for greater precision
   SYSTEM_USER -- or some other value for WHO made the deletion
FROM deleted

That way you're getting the exact columns you need and you're auditing what/when/why/who the audit event is about.

Answer 2

I commented this on your question, but I figured I'd try to actually present a code solution.

I usually agree with SELECT * being lazy, but since you have a legitimate reason to use it it's more grey than black-and-white.

What you should (in my opinion) try doing is something like this, but adjust it to ensure the column names and data types are the same (since someone could change data types and still cause issues in the db not normally caught with your SELECT * 'safety net'.

You could even create a function that will let you quickly check if the Audit version of the table matches the non-audit version:

-- The lengths are, I believe, max values for the corresponding db objects. If I'm wrong, someone please correct me
CREATE FUNCTION TableMappingComparer(
    @TableCatalog VARCHAR(85) = NULL,
    @TableSchema VARCHAR(32) = NULL,
    @TableName VARCHAR(128) = NULL) RETURNS BIT
AS
BEGIN
    DECLARE @ReturnValue BIT = NULL;
    DECLARE @VaryingColumns INT = NULL;

    IF (@TableCatalog IS NOT NULL
            AND @TableSchema IS NOT NULL
            AND @TableName IS NOT NULL)
        SELECT @VaryingColumns = COUNT(COLUMN_NAME)
            FROM (SELECT COLUMN_NAME,
                        DATA_TYPE -- Add more columns that you want to ensure are identical
                    FROM INFORMATION_SCHEMA.COLUMNS
                    WHERE TABLE_CATALOG = @TableCatalog
                        AND TABLE_SCHEMA = @TableSchema
                        AND TABLE_NAME = @TableName
                EXCEPT
                    SELECT COLUMN_NAME,
                            DATA_TYPE -- Add more columns that you want to ensure are identical
                        FROM INFORMATION_SCHEMA.COLUMNS
                        WHERE (TABLE_CATALOG = @TableCatalog
                            AND TABLE_SCHEMA = @TableSchema
                            AND TABLE_NAME = @TableName + 'Audit')
                            AND (COLUMN_NAME != 'exclude your audit table specific columns')) adt;
    IF @VaryingColumns = 0
        SET @ReturnValue = 1
    ELSE IF @VaryingColumns > 0
        SET @ReturnValue = 0

    RETURN @ReturnValue;
END;

The SELECT ... EXCEPT SELECT ...Audit will show you what columns in the table are not in the Audit table. You could even change the function to return the name of columns that are not the same instead of just whether they map or not, or even raise an exception.

You can then run this before moving from DEV to PRODUCTION servers for each table in the db, using a cursor over:

SELECT TABLE_NAME
    FROM INFORMATION_SCHEMA.TABLES
    WHERE NOT (TABLE_NAME LIKE '%Audit')

Answer 3

The statement that will cue the trigger will fail and the trigger will fail. It would be better practice to document the trigger and audit trail so you know to modify the query to add the columns instead of specify the *.

At the very least you should modify the trigger so it can fail gracefully while logging errors to a table and perhaps put an alert on the table the trigger is logging the errors to.

This also brings to mind, you can put a trigger or alert when someone alters the table and adds more columns or removes columns, to notify you to append the trigger.

Performance wise I believe * does not change anything, it just increases the chances of failures down the road when things change and also can cause network latency when you pull more information across the network when you need. There is a time and place for *, but I feel as described above you have better solutions and tools to try instead.

Answer 4

If your either your original or audit table structures change at all, you're guaranteeing that you'll run into an issue with your select *.

INSERT INTO [AuditTable]
(Col1,Col2,Col3)
SELECT * 
FROM [OrigTable] or [deleted];

If either changes, the trigger will error.

You could do:

INSERT INTO [AuditTable]
SELECT * 
FROM [OrigTable];

But as CM_Dayton says, that's lazy programming and opens the door for other inconsistencies. For this scenario to work, you'd have to make absolutely sure that you're updating the structure of both tables at the same time.