Multirow Update Trigger 2

[tis9700]

Good Morning,
I've been using a trigger on my tables that some of you may recognize. It works beautifully if it's on a table where an update is performed on a single record which is the case for most of my tables.

Now I need it to work for multirow updates because I have stored procedures that run on a weekly basis that insert or update on that table (Students) from another table (Students_Temp).

What I know...
1. This line is the culprit "Select @studID = (Select studID From inserted i)

2. It needs to be written to allow for Multirow updates because it can't tell which studID it's looking for from inserted. It returns more than one value. I've tried an join with the Students table but get an "Ambiguous Column Name" error.

"Select studID From inserted i join Students s On i.studID = s.studID"

Can anyone advise me on this?


ALTER trigger tr_Audit_Students on dbo.Students for update
as
declare
@bit int ,
@field int ,
@maxfield int ,
@char int ,
@studID varchar(50),
@fieldname varchar(128) ,
@Students varchar(128) ,
@PKCols varchar(1000) ,
@sql varchar(4000),
@dtUpdated varchar(21) ,
@RacfID varchar(128)



select @Students = 'Students'
select @studID = (Select studID FROM inserted i)

-- date and user
select @RacfID =system_user ,
@dtUpdated = convert(varchar(8), getdate(), 112) + ' ' + convert(varchar(12), getdate(), 114)




-- get list of columns
select * into #ins from inserted
select * into #del from deleted

-- Get primary key columns for full outer join
select @PKCols = coalesce(@PKCols + ' and', ' on') + ' i.' + c.COLUMN_NAME + ' = d.' + c.COLUMN_NAME
from INFORMATION_SCHEMA.TABLE_CONSTRAINTS pk ,
INFORMATION_SCHEMA.KEY_COLUMN_USAGE c
where pk.TABLE_NAME = @Students
and CONSTRAINT_TYPE = 'PRIMARY KEY'
and c.TABLE_NAME = pk.TABLE_NAME
and c.CONSTRAINT_NAME = pk.CONSTRAINT_NAME

if @PKCols is null
begin
raiserror('no PK on table %s', 16, -1, @Students)
return
end

select @field = 0, @maxfield = max(ORDINAL_POSITION) from INFORMATION_SCHEMA.COLUMNS where TABLE_NAME = @Students
while @field < @maxfield
begin
select @field = min(ORDINAL_POSITION) from INFORMATION_SCHEMA.COLUMNS where TABLE_NAME = @Students and ORDINAL_POSITION > @field
select @bit = (@field - 1 )% 8+ 1
select @bit = power(2,@bit - 1)
select @char = ((@field - 1) / 8) + 1
select @studID from INFORMATION_SCHEMA.COLUMNS where TABLE_NAME = @Students and COLUMN_NAME = @studID
if substring(COLUMNS_UPDATED(),@char, 1) & @bit > 0
begin
select @fieldname = COLUMN_NAME from INFORMATION_SCHEMA.COLUMNS where TABLE_NAME = @Students and ORDINAL_POSITION = @field
select @sql = 'insert AUDIT (TableName, PK_ID, FieldName,OldValue, NewValue, dtUpdated, RacfID )'
select @sql = @sql + ' select ''' + @Students + ''''
select @sql = @sql + ',''' + @studID + ''''
select @sql = @sql + ',''' + @fieldname + ''''
select @sql = @sql + ',convert(varchar(1000),d.' + @fieldname + ')'
select @sql = @sql + ',convert(varchar(1000),i.' + @fieldname + ')'
select @sql = @sql + ',''' + @dtUpdated + ''''
select @sql = @sql + ',''' + @RacfID + ''''
select @sql = @sql + ' from #ins i full outer join #del d'
select @sql = @sql + @PKCols
select @sql = @sql + ' where i.' + @fieldname + ' <> d.' + @fieldname
select @sql = @sql + ' or (i.' + @fieldname + ' is null and d.' + @fieldname + ' is not null)'
select @sql = @sql + ' or (i.' + @fieldname + ' is not null and d.' + @fieldname + ' is null)'
exec (@sql)
end
end


Thanks for any assistance

 

[RiverGuy]

The easiest way is to just use joins, which you're pretty much there. For example:

SELECT a.PrimaryKeyColumn, a.ColumnName AS OldValue, b.ColumnName AS NewValue
FROM DELETED a 
INNER JOIN INSERTED b 
ON a.PrimaryKeyColumn = b.PrimaryKeyColumn.

 

[tis9700]

Hi RiverGuy,
Are you saying to create a whole new trigger or replace the culprit with the statement you posted? I'm assuming ther would have t o be a ColumnName for every column in the table that can be updated?
Thanks for the help!

 

[SQLSister]

Ok the ambiguous column name error means you have to specify which table you want the studid (honestly who named that field? Stud Id? Is that some sort of joke?) to come from.

However, the whole trigger is needs rewriting as it will not perform well. First what is the point in putting inserted and deleted into temp tables, that is just wasted processing. And under no circumstances would I use dynamic SQl in a trigger. If that ever resolves to something unexpected and untested (and it is very difficult to test all possible things it might resolve to unless you create all the possible code and then you should just use that code inteh trigger not the dynamic code), you could have a huge problem on your hands. For an audit table insert it doesn't even make sense to do that, you know in advance waht the fields are so write regular code. You've used all sorts of parameters where you don't need to and thing is just a mess.

What we do for out audit triggers is create the code dynamically when there is a structure change, but we put the code that was created in the trigger not the dynamic slq. That way it is easier to read and troubleshoot if there is a problem. And we can specifically read (and test) what all the possibilites will resolve to before committing them to the actual trigger. And it will be faster than dynamically creating on the fly. Remember dynamic sql is a bad thing. It should not be used when it is not needed. Anything that slows a trigger down even a tiny bit can have a huge impact on overall performance as triggers are often executed very frequently.

"NOTHING is more important in a database than integrity." ESquared

 

[RiverGuy]

tis9700,
What I was suggesting was pseudocode for the general idea of what you want to look at doing.

Personally, I would rather have an Audit version of the table where the column names and types are preserved, instead of a generic, one-row-per-column-per-update design.

However, even with your current design, you could probably make it work without the dynamic sql in your looping construct.

 

[tis9700]

Thanks to both for your advice but I'm totally lost.

SQLSister, I get your points. I'm rewriting the whole trigger. I've started by creating a separate table, StudentAudit to eliminate the need to store the tablename.

For Update
as
Insert Into dbo.StudentAudit
(PK_ID, FieldName, OldValue, NewValue, dtUpdated, RacfID)

Still working on using the inserted and deleted tables...

 

[RiverGuy]

OK, you are still going with a one row per modified column approach. Here's the easiest approach to understand.

--Column1
INSERT INTO dbo.StudentAudit 
(PK_ID, FieldName, OldValue, NewValue, dtUpdated, RacfID)
SELECT a.studID, a.Column1, b.Column1, GETDATE(), system_user
FROM DELETED a
INNER JOIN INSERTED b
ON a.studID = b.studID

--Column2
INSERT INTO dbo.StudentAudit 
(PK_ID, FieldName, OldValue, NewValue, dtUpdated, RacfID)
SELECT a.studID, a.Column2, b.Column2, GETDATE(), system_user
FROM DELETED a
INNER JOIN INSERTED b
ON a.studID = b.studID

--Etc

Now, I think I'm going to take back my earlier statement. To get by with writing a single insert statement, it looks like you're going to have to use dynamic sql. In this case, I still think it's a good idea to write each statement out explicitly. You can use SQL as a tool to help generate the insert statements if you have a lot of columns. Just keep in mind that the more dynamic you try to make something, it sometimes adds to the complexity, debugging and security issues exponentially.

 

[tis9700]

Hi RiverGuy,
My tables are fairly small. Max 15 columns. So I can do the Insert statements for the columns but would I have to use dynamic SQL for FieldName? I omitted it from my previous post.

Thanks

 

[RiverGuy]

No, you could just "hard code" it. For example:


SELECT 'NameOfYourFirstColumn' AS FieldName
SELECT 'NameOfYourSecondColumn' AS Fieldname

 

[tis9700]

Hey RiverGuy,
What do you think of this?

ALTER trigger tr_Audit_Students on dbo.Students for update
as
IF @@rowcount = 0
return
--Audit Last Name Change. Occurs on direct change only.
INSERT INTO dbo.Audit
(PK_ID, TableName,FieldName, OldValue, NewValue, dtUpdated, RacfID)
SELECT a.studID,'STUDENTS', 'studLAST NAME', b.studLName, a.studLName, getdate(), suser_sname()
FROM inserted a INNER JOIN deleted b
ON a.studID = b.studID
WHERE a.studLName <> b.studLName

--Audit First Name Change
INSERT INTO dbo.Audit
(PK_ID, TableName,FieldName, OldValue, NewValue, dtUpdated, RacfID)
SELECT a.studID,'STUDENTS', 'studFIRST NAME', b.studFName, a.studFName, getdate(), suser_sname()
FROM inserted a INNER JOIN deleted b
ON a.studID = b.studID
WHERE a.studFName <> b.studFName

--Audit Middle Initial Change
INSERT INTO dbo.Audit
(PK_ID, TableName,FieldName, OldValue, NewValue, dtUpdated, RacfID)
SELECT a.studID,'STUDENTS', 'studMID INITIAL', b.studMI, a.studMI, getdate(), suser_sname()
FROM inserted a INNER JOIN deleted b
ON a.studID = b.studID
WHERE a.studMI <> b.studMI

--Audit Suffix Change
INSERT INTO dbo.Audit
(PK_ID, TableName,FieldName, OldValue, NewValue, dtUpdated, RacfID)
SELECT a.studID,'STUDENTS', 'studSUFFIX', b.studSuffix, a.studSuffix, getdate(), suser_sname()
FROM inserted a INNER JOIN deleted b
ON a.studID = b.studID
WHERE a.studSuffix <> b.studSuffix

--Audit Address Change
INSERT INTO dbo.Audit
(PK_ID, TableName,FieldName, OldValue, NewValue, dtUpdated, RacfID)
SELECT a.studID,'STUDENTS', 'studADDRESS', b.studAddress, a.studAddress, getdate(), suser_sname()
FROM inserted a INNER JOIN deleted b
ON a.studID = b.studID
WHERE a.studAddress <> b.studAddress


--Audit City Change
INSERT INTO dbo.Audit
(PK_ID, TableName, FieldName, OldValue, NewValue, dtUpdated, RacfID)
SELECT a.studID, 'STUDENTS', 'studCITY', b.studCity, a.studCity, getdate(), suser_sname()
FROM inserted a INNER JOIN deleted b
ON a.studID = b.studID
WHERE a.studCity <> b.studCity

--Audit State Change
INSERT INTO dbo.Audit
(PK_ID, TableName, FieldName, OldValue, NewValue, dtUpdated, RacfID)
SELECT a.studID, 'STUDENTS', 'studSTATE', b.studState, a.studState, getdate(), suser_sname()
FROM inserted a INNER JOIN deleted b
ON a.studID = b.studID
WHERE a.studState <> b.studState

--Audit Zip Code Change
INSERT INTO dbo.Audit
(PK_ID, TableName, FieldName, OldValue, NewValue, dtUpdated, RacfID)
SELECT a.studID, 'STUDENTS', 'studZIP', b.studZip, a.studZip, getdate(), suser_sname()
FROM inserted a INNER JOIN deleted b
ON a.studID = b.studID
WHERE a.studZip <> b.studZip

--Audit Date of Birth Change. Occurs on direct change only.
INSERT INTO dbo.Audit
(PK_ID, TableName, FieldName, OldValue, NewValue, dtUpdated, RacfID)
SELECT a.studID, 'STUDENTS', 'studDOB', b.studDOB, a.studDOB, getdate(), suser_sname()
FROM inserted a INNER JOIN deleted b
ON a.studID = b.studID
WHERE a.studDOB <> b.studDOB

--Audit Social Security Number Change. Occurs on direct change only.
INSERT INTO dbo.Audit
(PK_ID, TableName, FieldName, OldValue, NewValue, dtUpdated, RacfID)
SELECT a.studID, 'STUDENTS', 'studSSN', b.studSSN, a.studSSN, getdate(), suser_sname()
FROM inserted a INNER JOIN deleted b
ON a.studID = b.studID
WHERE a.studSSN <> b.studSSN

--Audit Gender Change.
INSERT INTO dbo.Audit
(PK_ID, TableName, FieldName, OldValue, NewValue, dtUpdated, RacfID)
SELECT a.studID, 'STUDENTS', 'studGENDER', b.studGender, a.studGender, getdate(), suser_sname()
FROM inserted a INNER JOIN deleted b
ON a.studID = b.studID
WHERE a.studGender <> b.studGender

--Audit Phone Change
INSERT INTO dbo.Audit
(PK_ID, TableName, FieldName, OldValue, NewValue, dtUpdated, RacfID)
SELECT a.studID, 'STUDENTS', 'studPHONE', b.studPhone, a.studPhone, getdate(), suser_sname()
FROM inserted a INNER JOIN deleted b
ON a.studID = b.studID
WHERE a.studPhone <> b.studPhone

I know I have it going back to one table at this point. They want all audits in one table. I'm going to argue the point tomorrow. It seems to do the trick but do you think it will have a high overhead if say 100 records with multiple fields changed are updated?

Thanks for all the help from both RiverGuy and SQlSister (No unfortunately studID is not a joke. They wanted to use Proctor when referring to Instructors until I pointed out some possible unpleasant implications)

 

[SQLSister]

We have an audit table for each table. Otherwise you will run into lots of blocking as multiple tables are updated by differnt users. It would be bad to block users from updating scheduled courses because someone is importing 100000 records in some other table.

In fact, you might remember that when you select data from these tables in the future to make sure to use the (nolock) hint or you could run into blocking issues if you have a lot of users.

And StudID isn't nearly as amusing as StudGender!!!

"NOTHING is more important in a database than integrity." ESquared

 

[RiverGuy]

That looks good tis9700.

SQLSister makes an excellent point about the single-table audit design and the blocking. Another advantage with an audit table per user table is that you don't have to convert everything to a single data type--you can preserve your data types and table structures, which could make analysis easier and faster. If you ever get the chance to revisit the audit design, definitely look into it.

 

[tis9700]

Excellent! Thanks for everything! I always learn so much when I come to this forum. I reading the BOL on Locking Hints. And I am going to do the separate audit tables.

They wanted me to name a primary key "studMaster", I kid you not!