Update Trigger Problem - SQL Server Forums

Please start any new threads on our new site at https://forums.sqlteam.com. We've got lots of great SQL Server experts to answer whatever question you can come up with.

All Forums

SQL Server 2005 Forums

Transact-SQL (2005)

Update Trigger Problem

Author

Topic

spizotfl
Starting Member

8 Posts

Posted - 2010-01-22 : 09:59:46

Hi, I have to add auditing to a database. I have to log changes by column with old/new values listed. I found Nigel Rivett's audit trigger here: http://www.simple-talk.com/sql/database-administration/pop-rivetts-sql-server-faq-no.5-pop-on-the-audit-trail/

I was able to modify the trigger to work with my individual tables instead of one central table. The trigger works as I would like on my development machine in all cases (ie. when connecting through our asp.net app and when running update statements against the db directly.)

I then ran the trigger creation scripts against the testing server. From the testing version of the application only one of the tables is logging changes. The other 9 don't log anything and they do not throw any errors. Everything works as expected on direct table edits, either through sql update statements or opening the table and changing a field.

Here is one of the non-working triggers:


CREATE TRIGGER tr_AuditUpdateFARS ON tblFARS FOR UPDATE
AS

DECLARE @bit INT ,
       @field INT ,
       @maxfield INT ,
       @char INT ,
       @fieldname VARCHAR(128) ,
       @TableName VARCHAR(128) ,
       @sql VARCHAR(2000), 
       @UpdateDate VARCHAR(21) ,
       @UserName VARCHAR(128) ,
       @Type CHAR(1)
       
-- Set Audited table name
SELECT @TableName = 'tblFARS'

-- Action
SELECT @Type = 'U'

-- get list of columns
SELECT * INTO #ins FROM inserted
SELECT * INTO #del FROM deleted

SELECT @field = 0, 
       @maxfield = MAX(COLUMNPROPERTY(OBJECT_ID(TABLE_SCHEMA + '.' + TABLE_NAME), COLUMN_NAME, 'ColumnID')) 
       FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_NAME = @TableName
	   
WHILE @field < @maxfield
BEGIN
	   SELECT @field = MIN(COLUMNPROPERTY(OBJECT_ID(TABLE_SCHEMA + '.' + TABLE_NAME), COLUMN_NAME, 'ColumnID')) 
			   FROM INFORMATION_SCHEMA.COLUMNS 
			   WHERE TABLE_NAME = @TableName 
			   AND COLUMNPROPERTY(OBJECT_ID(TABLE_SCHEMA + '.' + TABLE_NAME), COLUMN_NAME, 'ColumnID') > @field
	   SELECT @bit = (@field - 1 )% 8 + 1
	   SELECT @bit = POWER(2,@bit - 1)
	   SELECT @char = ((@field - 1) / 8) + 1
	   IF SUBSTRING(COLUMNS_UPDATED(),@char, 1) & @bit > 0
	   BEGIN
			   SELECT @fieldname = COLUMN_NAME 
					   FROM INFORMATION_SCHEMA.COLUMNS 
					   WHERE TABLE_NAME = @TableName 
					   AND COLUMNPROPERTY(OBJECT_ID(TABLE_SCHEMA + '.' + TABLE_NAME), COLUMN_NAME, 'ColumnID') = @field
			   IF @fieldname <> 'ModifyDate'
				   BEGIN
				   SELECT @sql = '
				   INSERT tblFARSAudit (	Type, 
				   SSN, 
				   ContractorID,
				   DCFPurp, 
				   EvalDate,
				   ProvID,
				   FieldName, 
				   OldValue, 
				   NewValue, 
				   ChangeDate, 
				   UserName)
					select ''' + @Type + ''', i.SSN, i.ContractorID, i.DCFPurp, i.EvalDate, i.ProvID'
					+ ',''[' + @fieldname + ']'''
					+ ',convert(varchar(1000),d.[' + @fieldname + '])'
					+ ',convert(varchar(1000),i.[' + @fieldname + '])'
					+ ', GetDate()'
					+ ', i.UserName'
					+ ' from #ins i full outer join #del d'
					+ ' on i.SSN = d.SSN AND i.ContractorID = d.ContractorID AND i.ProvID=d.ProvID'
					+ ' and i.EvalDate = d.EvalDate AND i.DCFPurp = d.DCFPurp'
					+ ' where i.[' + @fieldname + '] <> d.[' + @fieldname 
					+ '] or (i.[' + @fieldname + '] is null and  d.[' + @fieldname + '] is not null)' 
					+ ' or (i.[' + @fieldname + '] is not null and  d.[' + @fieldname + '] is null)' 
				   EXEC (@sql)
			   END
	   END
END

I have compared the non-working triggers against the working one and the only difference that I see is the specific column names for the pk fields. If needed, I can post the one that works.

I have tried inserting RAISERROR statements to do a rough debugging. I have found that if I put one after the IF SUBSTRING(COLUMNS_UPDATED()... portion the error doesn't happen.

It would all be much easier to make sense of if it failed on all of the tables.

I appreciate any suggestions.

TG
Master Smack Fu Yak Hacker

6065 Posts

Posted - 2010-01-22 : 10:12:34

Please post DDL for tblFARS and tblFARSAudit.

Be One with the Optimizer
TG

spizotfl
Starting Member

8 Posts

Posted - 2010-01-22 : 10:20:11

tblFARS


USE [OneFamFTP]
GO
/****** Object:  Table [dbo].[tblFARS]    Script Date: 01/22/2010 10:18:55 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
SET ANSI_PADDING ON
GO
CREATE TABLE [dbo].[tblFARS](
	[SSN] [char](9) COLLATE SQL_Latin1_General_CP1_CI_AS NOT NULL,
	[ContractorID] [char](10) COLLATE SQL_Latin1_General_CP1_CI_AS NOT NULL,
	[DCFPurp] [char](1) COLLATE SQL_Latin1_General_CP1_CI_AS NOT NULL,
	[EvalDate] [smalldatetime] NOT NULL,
	[ProvID] [char](10) COLLATE SQL_Latin1_General_CP1_CI_AS NOT NULL,
	[HSAClientID] [char](9) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
	[AdmitDate] [smalldatetime] NULL,
	[SiteID] [char](2) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
	[ProgPurp] [char](1) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
	[MGAFScore] [tinyint] NULL,
	[EduLevel] [char](2) COLLATE SQL_Latin1_General_CP1_CI_AS NOT NULL,
	[FMHINum] [char](9) COLLATE SQL_Latin1_General_CP1_CI_AS NOT NULL,
	[SAHist] [char](1) COLLATE SQL_Latin1_General_CP1_CI_AS NOT NULL,
	[Depress] [tinyint] NULL,
	[Anxiety] [tinyint] NULL,
	[HyperAff] [tinyint] NULL,
	[Thought] [tinyint] NULL,
	[Cognitiv] [tinyint] NULL,
	[Medical] [tinyint] NULL,
	[Traumati] [tinyint] NULL,
	[Substanc] [tinyint] NULL,
	[Relation] [tinyint] NULL,
	[Famrela] [tinyint] NULL,
	[Famenvi] [tinyint] NULL,
	[soclegal] [tinyint] NULL,
	[Workscho] [tinyint] NULL,
	[ADLFunct] [tinyint] NULL,
	[SelfCare] [tinyint] NULL,
	[DangSelf] [tinyint] NULL,
	[DangOth] [tinyint] NULL,
	[Security] [tinyint] NULL,
	[ProvInfo] [char](20) COLLATE SQL_Latin1_General_CP1_CI_AS NOT NULL,
	[ContID1] [char](5) COLLATE SQL_Latin1_General_CP1_CI_AS NOT NULL,
	[ContID2] [char](5) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
	[ContID3] [char](5) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
	[MedRecpID] [char](10) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
	[MedProvID] [char](9) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
	[MedPlanID] [char](2) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
	[CntyServ] [char](2) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
	[EnterDate] [smalldatetime] NOT NULL,
	[ModifyDate] [smalldatetime] NOT NULL,
	[UserName] [varchar](30) COLLATE SQL_Latin1_General_CP1_CI_AS NOT NULL,
 CONSTRAINT [PK_tblFARS] PRIMARY KEY CLUSTERED 
(
	[SSN] ASC,
	[ContractorID] ASC,
	[DCFPurp] ASC,
	[EvalDate] ASC,
	[ProvID] ASC
)WITH (PAD_INDEX  = OFF, STATISTICS_NORECOMPUTE  = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS  = ON, ALLOW_PAGE_LOCKS  = ON) ON [PRIMARY]
) ON [PRIMARY]

GO
SET ANSI_PADDING OFF

tblFARSAudit


USE [OneFamFTP]
GO
/****** Object:  Table [dbo].[tblFARSAudit]    Script Date: 01/22/2010 10:18:45 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
SET ANSI_PADDING ON
GO
CREATE TABLE [dbo].[tblFARSAudit](
	[FARSAuditID] [int] IDENTITY(1,1) NOT NULL,
	[Type] [char](1) COLLATE SQL_Latin1_General_CP1_CI_AS NOT NULL,
	[SSN] [char](9) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
	[ContractorID] [char](10) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
	[DCFPurp] [char](1) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
	[EvalDate] [smalldatetime] NULL,
	[ProvID] [char](10) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
	[FieldName] [varchar](50) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
	[OldValue] [varchar](50) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
	[NewValue] [varchar](50) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
	[ChangeDate] [smalldatetime] NOT NULL,
	[UserName] [varchar](30) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
 CONSTRAINT [PK_tblFARSAudit] PRIMARY KEY CLUSTERED 
(
	[FARSAuditID] ASC
)WITH (PAD_INDEX  = OFF, STATISTICS_NORECOMPUTE  = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS  = ON, ALLOW_PAGE_LOCKS  = ON) ON [PRIMARY]
) ON [PRIMARY]

GO
SET ANSI_PADDING OFF

TG
Master Smack Fu Yak Hacker

6065 Posts

Posted - 2010-01-22 : 11:03:48

I didn't have any problems with your code. I created the 2 tables and the trigger, then inserted a couple rows and updated a couple columns on all the rows. The audit table correctly contained all changed columns for all changed rows. ??

Be One with the Optimizer
TG

spizotfl
Starting Member

8 Posts

Posted - 2010-01-22 : 11:44:49

Any thoughts on what could cause the observed behavior?

spizotfl
Starting Member

8 Posts

Posted - 2010-01-22 : 11:48:24

The Application runs under a Network Service like account. All database access is handled through stored procedures. Is there any chance that there is some sort of permissions issue and, if so, what permissions should I look at? Unfortunately we do not have any body here who really know SQL Server, so I have to try and figure this all out myself.

Kristen
Test

22859 Posts

Posted - 2010-01-22 : 11:56:11

"Is there any chance that there is some sort of permissions issue"

Yes, your trigger is using dynamic SQL, so user will have to have INSERT permission on that table.

FWIW I *definitely* would not do it that way :(

... just store the data that has changed, without attempting to massage it into a differnet format / table / which columns changed etc., and deal with the presentation when you report on it.

Triggers need to be as-fast-as-a-fast-thing .... copying the data into local #TEMP tables and using dynamic SQL to insert it is going to be slow ... I reckon.

spizotfl
Starting Member

8 Posts

Posted - 2010-01-22 : 12:03:56

Do you have a suggestion on how to just get the changed data? My original thought involved inserting the full record into the audit table on insert/update/delete but I was told that it would be too "hard" for people to see what had changed.

I thought having INSERT permissions would be important and tried setting it on the audit table but it still doesn't work on 9 of the 10.

spizotfl
Starting Member

8 Posts

Posted - 2010-01-22 : 12:16:57

Just reading some more and came across this post (http://www.sqlteam.com/forums/topic.asp?TOPIC_ID=52356) that I have read a half dozen times since I started working on this....

TG said "I believe the issue is that the user selecting values out of information_schema owned views need to have appropriate permissions to the (subject) objects themselvles."

Took a look at the regular data tables and the one is auditing properly lists the Application's account as having INSERT permissions to it. Allowing INSERT on tblFARS to the application account allows the update to be logged in the audit table. I just removed the INSERT permission and the update was still logged.... I am very confused.

spizotfl
Starting Member

8 Posts

Posted - 2010-01-22 : 12:18:20

Scratch that last bit about it working once I removed the permission.

spizotfl
Starting Member

8 Posts

Posted - 2010-01-22 : 12:23:10

It also works if I give the account SELECT permissions on the table.

Kristen
Test

22859 Posts

Posted - 2010-01-22 : 12:27:10

Yes, I would go with insert the full row to an Audit Table.

You can then just do

INSERT INTO MyTable_AUDIT
SELECT GetDate(), ... any other "extra" columns, ... *
FROM deleted

(We only store the Deleted data on UPDATE and DELETE. I figure the current data is in the table, so I don't need that, and I save some bytes

. Others prefer to save the INSERTED data. We have one _AUDIT table for each real table)

So long as you remember to change / add columns the the _AUDIT table when you change the Main table then the SELECT * will include your new columns (and if you forget the trigger will break, so it will come to light immediately)

yes, I agree, seeing what has changed is harder.

We display the data in a Grid, all columns horizontally, and we colour-code data that has changed from one row (edit) to the next.

One thing this does do (which just storing "Column X changed from Foo to Bar" does NOT do) is to show the data in context. e.g.:

"On the change of 25th November when the Address was changed from THIS to THAT what was the country setting?"

assuming that the country setting did NOT change on 25th November, but did change at some point, and is the source of some confusion, then its hard to determine what it was at a particular point in time.

People usually need to see the data, in context, to form opinions about "Why" etc. and to determine "Ah! I remember why we did that ..."

I have done audit triggers that record PK fields, Column Name & Before/After values. Rather than using dynamic SQL we mechanically generated the code for each trigger (they were quite big, but I don't suppose that matters). Its not that hard to loop round the column names, and data types, and generated the code.

But it was a nightmare to report on, the triggers were huge, and they were slower than "Just stuff the DELETED into an audit table"

One thing that one audit table, per real table, does badly is to help work out what also changed at the same time (in other tables). e.g. an Sproc saves the Name record and also the Address record at the same time.

Given an _Audit table as per my example, you could then use that as a basis to generate "sequential edits" as a background / slack-time task, if you need them that way.

This method may need the user to have INSERT permissions on the _AUDIT table, not sure offhand whether SProc EXECUTE is enough.

Subscribe to SQLTeam.com

SQLTeam.com Articles via RSS

SQLTeam.com Weblog via RSS

- Advertisement -

Resources