SQL Stored Procedure to Log Updates, Independent of Database Structure

  • keren ramot

    SSC Enthusiast

    Points: 143

    Comments posted here are about the content posted at http://www.sqlservercentral.com/columnists/kramot/2773.asp

  • Phil Factor

    SSCoach

    Points: 19823

    Nigel Rivett's technique is worth taking a look at.. http://www.simple-talk.com/sql/database-administration/pop-rivetts-sql-server-faq-no.5-pop-on-the-audit-trail/ 

    Best wishes,
    Phil Factor
    Simple Talk

  • colin.Leversuch-Roberts

    SSC Guru

    Points: 52551

    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.

    [font="Comic Sans MS"]The GrumpyOldDBA[/font]
    www.grumpyolddba.co.uk
    http://sqlblogcasts.com/blogs/grumpyolddba/

  • noeld

    SSC Guru

    Points: 96590

    I have to say that like colin this sort of operations are to be considered *very* well before you jump the gun. High Performance DB don't take this sort of things easily , you have to *work* to make it happen. (Been there done that)

    For a small thing that is not too heavily abused you could use whatever makes you happy.


    * Noel

  • keren ramot

    SSC Enthusiast

    Points: 143

    Noel,
    I agree it's not for everyone. I may have not stressed that enough, but I thought titling it "quick and dirty" implied that. This sort of solution would be required only if the architecture of the database is flawed to begin with, and if your DB is high traffic and flawedly designed it's likely to fail anyway. However I can not agree with colin's statement, because my CPU tests have shown that there is a huge difference between the cursor and the while loop - the loop preformed light years better then the cursor.
    Thanks for reading
    K
  • colin.Leversuch-Roberts

    SSC Guru

    Points: 52551

    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.

    [font="Comic Sans MS"]The GrumpyOldDBA[/font]
    www.grumpyolddba.co.uk
    http://sqlblogcasts.com/blogs/grumpyolddba/

  • Phil Factor

    SSCoach

    Points: 19823

    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?

     

    Best wishes,
    Phil Factor
    Simple Talk

  • Phil.Nicholas

    SSCrazy

    Points: 2067

    A few other questions, as someone mentioned this isn't the ideal and I have to put auditing into our applications sometime soon!

    When designing from scratch is the auditing best done as part of the calling stored procedure or is it best in the calling application?

    Assuming the database structure is large and complex would the data still be pivoted or replicate the table structure (perhaps in a separate database/server) or storing versions in the same table?

    How important is user friendliness when viewing of the audit trail (does anyone have any experience of end users wanting to view a record of changes without tech support)?

    What additional functionality would you expect to provide such as archiving etc.


    Phil Nicholas

  • Bob St. Aubyn

    SSC Enthusiast

    Points: 171

    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 = name

    FROM      syscolumns

    WHERE   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 = name

    FROM syscolumns

    WHERE 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!

  • Bob St. Aubyn

    SSC Enthusiast

    Points: 171

    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)

                         from information_schema.key_column_usage

                         where objectproperty(object_id(constraint_name),''IsPrimaryKey'') = 1

                         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

                         select @subsql = N''select @PKval = convert(varchar(60), d.''

                               + @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.

  • keren ramot

    SSC Enthusiast

    Points: 143

    Thank you Bob. I've been feeling a little ganged up on till you and Phil Factor came along. I knew that there must be someone out there who had or will encounter similar circumstances as I have and may find this article useful. Over time I've grown skeptical of that assumption, but your feedback had really made it worth my while. I am very glad you enjoyed the article and found it useful and I appreciate you posting your elaboration on the code.

    Thank you very much

    K

  • Mike Elliott

    SSC Enthusiast

    Points: 144

    I unfortunately have ntext fields in the table(s) that need to be audited and I've been trying to find a way to get around it.

    My attempt (I can't call it a solution because it doesn't work) is to try to just insert the fields that aren't ntext, text or image into a temp table. I do this by getting the names of the fields for the table being audited from the syscolumns table and put them in a variable.

    My code looks like this:

    CREATE TABLE #TableCache (

    columnName varchar(400)

    )

    INSERT INTO #TableCache

    SELECT '[' + sc.name + ']' as columnName

    FROM sysobjects as so JOIN syscolumns as sc

    ON so.id = sc.id

    WHERE so.xtype = 'U' AND sc.xtype NOT IN ( 99, 35, 34) AND so.name = @TableName

    AND sc.name not in ('Created', 'Modified', 'RowVersion')

    SET @FieldList = ''

    -- create a cursor

    DECLARE tableColumn CURSOR FOR

    SELECT *

    FROM #TableCache

    -- loop over the cursor

    -- and get a list of the columns in the table that are not ntext, text or image

    OPEN tableColumn

    FETCH NEXT FROM tableColumn INTO @columnName

    WHILE @@FETCH_STATUS = 0

    BEGIN

    if len(@FieldList) = 0

    begin

    SET @FieldList = '[' + @columnName + ']'

    end

    else

    begin

    SET @FieldList = @FieldList + ', [' + @ColumnName + ']'

    end

    -- next row, please

    FETCH NEXT FROM tableColumn INTO @columnName

    END

    print @FieldList

    -- get list of columns

    Set @SQL ='SELECT ' + @FieldList + ' INTO #ins FROM inserted'

    EXEC (@SQL)

    The trigger errors on the last line stating invalid object inserted. (NOTE: This code is inside a trigger)

    If I try to use this:

    SELECT @FieldList INTO #ins FROM inserted

    I get No column was specified for column 1 of '#ins'.

    Any ideas?

  • hawkeyeboy

    SSC Veteran

    Points: 200

    i get Incorrect syntax near '+@VarRandom+'.

  • k.ramot

    Grasshopper

    Points: 23

    I would say you’d have to be more specific then that. But before you do that, it’s possible the formatting on the page changed the code. So use the following... see how it does.

    CREATE PROCEDURE dbo.SPRouterForLog

    (

    @SPName nvarchar(50),

    @RecordId nvarchar(10),

    @str1 nvarchar(4000),

    @TableName nvarchar(100),

    @OpName nvarchar(50),

    @PageName nvarchar(50)

    ) AS

    declare @TrriggerCreate varchar(8000)

    declare @VarRandom nvarchar(50)

    set @VarRandom=ltrim(str(replace(Rand(), '.', '')))

    set @TrriggerCreate='CREATE TRIGGER [PreUpdateTrigger] ON dbo.'+@TableName+'

    FOR UPDATE

    AS

    DECLARE

    @iReturnCode int,

    @iNextRowId int,

    @iCurrentRowId int,

    @iLoopControl int,

    @old varchar(75),

    @new varchar(75),

    @tmpName sysname,

    @data_type nvarchar(30)

    SELECT * INTO #deleted'+@VarRandom+' FROM deleted

    SELECT * INTO #inserted'+@VarRandom+' FROM inserted

    SELECT @iLoopControl = 1

    SELECT @iNextRowId = MIN(colid)

    FROM syscolumns

    where id = object_id('''+@TableName+''')

    SELECT @iCurrentRowId = colid,

    @tmpName = name

    FROM syscolumns

    WHERE colid = @iNextRowId

    WHILE @iLoopControl = 1

    BEGIN

    SELECT @iNextRowId = NULL

    SELECT @iNextRowId = MIN(colid)

    FROM syscolumns

    WHERE colid > @iCurrentRowId and id = object_id('''+@TableName+''')

    if (substring (columns_updated(), 1+ round ((@iCurrentRowId - 1) / 8, 0), 1) & power (2, (@iCurrentRowId - 1) % 8) <> 0 )

    begin

    create table #pass'+@VarRandom+' (tempvar sysname null)

    create table #pass1'+@VarRandom+' (tempvar sysname null)

    set @data_type=null

    select @data_type=data_type FROM information_schema.columns WHERE table_name='''+@TableName+''' and Column_Name=@tmpName

    if (@data_type <> ''money'' )

    Begin

    exec (''insert #pass'+@VarRandom+' select d.''+@tmpName+'' from #deleted'+@VarRandom+' as d

    insert #pass1'+@VarRandom+' select i.''+@tmpName+'' from #inserted'+@VarRandom+' as i'')

    end

    else

    begin

    exec (''insert #pass'+@VarRandom+' select cast(d.''+@tmpName+'' as varchar) from #deleted'+@VarRandom+' as d

    insert #pass1'+@VarRandom+' select cast(i.''+@tmpName+'' as varchar) from #inserted'+@VarRandom+' as i'')

    End

    select @old = tempvar from #pass'+@VarRandom+'

    select @new = tempvar from #pass1'+@VarRandom+'

    drop table #pass1'+@VarRandom+'

    drop table #pass'+@VarRandom+'

    if @old is null

    begin

    set @old='' ''

    end

    if @new<>@old

    begin

    insert into tbl_log (TableName,RecordNumber,ActionBy,ActionPage,ChangeClmn,OldValue,NewValue,ActionDate) values ('''+@TableName+''','+@RecordId+','''+@OpName+''','''+@PageName+''',@tmpName,@old,@new,getdate())

    end

    end

    IF ISNULL(@iNextRowId,0) = 0

    BEGIN

    BREAK

    END

    SELECT @iCurrentRowId = colid,

    @tmpName = name

    FROM syscolumns

    WHERE colid = @iNextRowId and id = object_id('''+@TableName+''')

    end

    drop table #deleted'+@VarRandom+'

    drop table #inserted'+@VarRandom+'

    '

    EXEC(@TrriggerCreate)

    declare @strSqlExec nvarchar(4000)

    set @strSqlExec=@SPName+' '+@str1

    EXEC (@strSqlExec)

    drop trigger PreUpdateTrigger

    GO

  • hawkeyeboy

    SSC Veteran

    Points: 200

    Thank you for your reply Keren.

    I believe I see the error of my ways....I was attempting to use a portion of your code within a trigger.

    To be precise the section:

    declare @VarRandom NVARCHAR(50)

    set @VarRandom=ltrim(str(replace(Rand(), '.', '')))

    SELECT * INTO #deleted'+@VarRandom+' FROM deleted

    SELECT * INTO #inserted'+@VarRandom+' FROM inserted

    this is where I get the syntax error. Is the above possible within a trigger?

Viewing 15 posts - 1 through 15 (of 16 total)

You must be logged in to reply to this topic. Login to reply