we had a long discussion about cursors vs loops, a while loop is still a cursor and I doubt the impact within a single trigger would be much different.
My view = if you want to destroy your database performance add triggers - note that update triggers force all updates to be deferred , so no inplace updates, not sure if that is still so in sql 2005, this can give problems with transactional replication for instance.
to be honest cursor / loop = row by agonising row operations within triggers would be my worst nightmare for a production database. The overhead of deferred updates is also critical as this will cause fragmentation of table/index structures thus degrading performance further. We did the threads over loops and cursors so I'm not going to comment further. Itzik Ben-Gan has some good examples of where a cursor works faster and better, however, that's not really part of this thread.
I agree auditing is important, SOX, and it's tricky, and there are not too many alternatives, but triggers have, in my experience, not been the best solution.
I think that Keren has a good point. A lot of database developers are dealing with the task of revising existing code in difficult circumstances, or trying to tie down a tricky bug, and I have experienced times myself when such code has been extremely useful. However, I'd never actually design a database with this sort of code in it. Who would?
Hello forum,
I have a couple comments on Karen's posting. First, bravo for taking on the hotly controversial topic of handling change logging with triggers - only the brave tread here. And I think that you've adequately cautioned your readers that this is not the ideal solution, as most would agree that if you designed a brand new database with change logging you would abstract the application's direct access to the tables entirely and handle both the updates and change logging entirely through a batch of sp's.
I have taken Karen's code as a template and devised a solution in a client's existing database. This is a case where I cannot change all the client-sourced update commands (which quite often reference the tables directly), so the solution here had to be based on triggers. This is also a database that typically updates relatively few rows at a time and has plenty of horsepower on the server so there will be no appreciable performance degredation to the end user. There are rare situations where bulk updates are performed, but I was careful to make sure that all these new triggers were diabled prior to - and reenabled after - the bulk update process is run.
I did also find one teensy little error in Karen's code: Just before the WHILE loop, where the @CurrentRowID and @tmpname variables are initialized with this statement.
SELECT @CurrentRowId = colid, @tmpName = nameFROM syscolumnsWHERE colid = @NextRowId
The where clause is not specific enough to zero in on one row. The colid column in syscolumns is definitely not unique in the table. Here's a suggestion:
SELECT @CurrentRowId = colid, @tmpName = nameFROM syscolumnsWHERE id = object_id('''+@TableName+''') and objectproperty(id, ''IsUserTable'') = 1 and colid = @NextRowId
I think this is what Karen had in mind.
Lastly, thanks Karen for a great article! Even though this solution has a specific niche where it might fit perfectly, we should know that there are very few "silver bullet" solutions out there for SQL Server and should carefully measure EVERY solution we borrow from someone else before throwing it into production!
Sorry I forgot one more thing: I noticed that Karen's tbl_log (and the code that writes to it) doesn't include the PK(s) for the table the change came from. This makes it difficult at best to use the information gathered in your audit table to trace back to the row in the base table that the change was performed on.
This was not difficult to solve. In my particular database, I know I have composite PK's with a maximum of 5 columns. So I added PK1, PK2, PK3, PK4 and PK5 columns to my version of tbl_log, datatypes of varchar(60) or so. Then in the trigger code, just before spinning through the columns in the table to see which ones have changed, I get the PK column values like so:
--Loop through the PK columns for the table and grab values from the temp tables for each
select @PKpos = min(ordinal_position), @PKposmax = max(ordinal_position)
from information_schema.key_column_usage
where objectproperty(object_id(constraint_name),''IsPrimaryKey'') = 1
and table_name = '''+@TableName+'''
while @PKpos <= @PKposmax
begin
select @PKval = null
select @PKcolname = quotename(column_name)
and table_name = '''+@TableName+''' and ordinal_position = @PKpos
if exists(select * from #inserted'+@VarRandom+')
select @subsql = N''select @PKval = convert(varchar(60), d.''
+ @PKcolname+'') from #inserted'+@VarRandom+' d ''
else
+ @PKcolname+'') from #deleted'+@VarRandom+' d ''
exec sp_executesql @subsql, N''@PKval varchar(60) output'', @PKval output
if @PKpos = 1
set @PK1val = @PKval
else if @PKpos = 2
set @PK2val = @PKval
else if @PKpos = 3
set @PK3val = @PKval
else if @PKpos = 4
set @PK4val = @PKval
else if @PKpos = 5
set @PK5val = @PKval
select @PKpos = @PKpos+1
end
As you might guess I then wrote the @PK1val, @PK2val,... values to my ChangeLog table in corresponding columns. It was not really necessary to record what the column name was for each part of the composite because I can derive that easily by the table name. I can now join the ChangeLog table directly back to the row in the base table where the change was originally performed for reporting.
