Server = SQL Server 2000 SP3a
Client = Access 2000 SP3 (.adp)
Hi, can anyone advise me on creating an audit process to account for any
changes in a table, I thought I'd come up with quite a good idea and started
to do it, but now Im stuck. There are several tables which have 20-40
columns each, the plan was to create an audit table with:
CREATE TABLE [dbo].[tbl_Audit] (
[A_TABLE] [varchar] (50) NOT NULL,
[A_YEAR] [char] (4) NOT NULL ,
[A_VERSION] [varchar] (10) NOT NULL ,
[A_COST_CENTRE] [varchar] (6) NOT NULL ,
[A_ACCOUNT] [varchar] (8) NOT NULL ,
[A_COLUMN] [varchar] (50) NOT NULL ,
[A_OLD_VALUE] [varchar] (1000) NOT NULL ,
[A_NEW_VALUE] [varchar] (1000) NOT NULL ,
[A_DATE] [datetime] NULL ,
[A_USER] [varchar] (50) NULL
) ON [PRIMARY]
GO
This could then be used as a generic audit table, as the table is logged,
the identifying rows are logged (year, version, cost_centre, account), then
the column name, its old and new values.
This was going to be populated by a trigger which fired after an update, it
would loop though the column names and check the values from the inserted
table to the actual table to see if they were different, if they were
different then insert into audit.
I had started to write the trigger, but hit a problem early on, which is I'd
like to create one trigger which fits all tables with only minor tweaking, s
o
I thought I'd use a cursor to loop through the column names, but this means
(I think) using a dynamic sql string, but this seems to fail as the inserted
table goes out of scope, eg when the trigger below fires, it errors with:
Server: Msg 208, Level 16, State 1, Line 2
Invalid object name 'inserted'.
This I assume is because the inserted table is not in scope of the dynamic
SQL string @.SQL1 (below)
alter trigger tg_Estimates_Audit_Update
on dbo.tbl_estimates
after update
as
SET NOCOUNT ON
SET ANSI_NULLS ON
SET ANSI_WARNINGS ON
declare
@.COLUMN varchar(50),
@.SQL1 nvarchar(750),
@.USER varchar(30),
@.TIME datetime
set @.USER = (select suser_sname())
set @.TIME = (select getdate())
declare TableCursor cursor local static for
select COLUMN_NAME from information_schema.columns
where table_name = 'tbl_estimates' and column_name like '%ADJ%'
open TableCursor
fetch next from TableCursor into @.COLUMN
while @.@.fetch_status = 0
begin
set @.SQL1 = '
insert into tbl_audit (
a_table,
a_year,
a_version,
a_cost_centre,
a_account,
a_column,
a_old_value,
a_new_value,
a_date,
a_user )
select
''tbl_estimates'' as a_table,
a.est_year,
a.est_version,
a.est_cost_centre,
a.est_account,
'''+@.COLUMN+''' as est_column,
b.'+@.COLUMN+' as est_old_value,
a.'+@.COLUMN+' as est_new_value,
'''+cast(@.TIME as varchar)+''' as est_date,
'''+@.USER+''' as est_user
from inserted a
inner join tbl_estimates b on
a.est_year = b.est_year and
a.est_version = b.est_version and
a.est_cost_centre = b.est_cost_centre and
a.est_account = b.est_account
where
a.'+@.COLUMN+' != b.'+@.COLUMN
exec sp_executesql @.SQL1
fetch next from TableCursor into @.COLUMN
end
close TableCursor
deallocate TableCursor
I then tried to insert the data from inserted into a #temp table, but again
I think I would need to use exec sp_executesql to do this as surely the
#temp_table would have to be #temp_table+'@.USERNAME' otherwise would there
not be possibly duplicated data if other people are editing the table and
causing the trigger to fire at the same time....or maybe not?
Thanks for reading that! Can anyone offer any ideas as to how I might get
round this, or even a totally different approach if the one I've chosen is
just plain daft.
I thought I was being clever, but maybe thats where Im going wrong :)
Many thanks.
Steve'oNever put cursors in triggers. They just turn your set-based updates
into slow, inefficient and over-complex row-by-row operations.
Dynamic SQL is usually a bad idea too. The security implications of
dynamic code make it particularly unsuitable for audit processes. A
better method to keep this "generic" IMO is to generate static trigger
code automatically at design time using the information_schema.
Your trigger code can look something like the following, for example.
CREATE TRIGGER trg_foo_audit ON foo FOR INSERT, UPDATE, DELETE
AS
INSERT INTO audit_table (col1, col2, ...)
SELECT D.col1, D.col2, ...
FROM Inserted AS I
JOIN Deleted AS D
ON I.key_col = D.key_col
WHERE COALESCE(I.col1,'') <> COALESCE(D.col1,'')
AND COALESCE(I.col2,'') <> COALESCE(D.col2,'')
Hope this helps.
David Portas
SQL Server MVP
--|||Hi David, thanks for the reply!
With the dynamic sql I was just passing the column name and the username of
the person who update the data, all the variables are sql functions, how is
this bad security? Im not running the alter trigger statement each time,
sorry if I forgot to change that line, once I've written the trigger it
should stay static, with just the variables changing within the dynamic @.SQL
1
When you said:
>A
> better method to keep this "generic" IMO is to generate static trigger
> code automatically at design time using the information_schema.
Is that not what I had done? Using a curosr to loop through the
information.schema?
The slowness is an issue, it does make it much slower, as I've managed to
get a bit more working and can see its not good as it takes 1 second between
updates now.
From what your saying though, I have to write a long trigger per table, as
there are 20-40 columns which could be updated, so truck loads of OR..OR..OR
.
I was trying to keep a short single trigger which could quickly be re-used
if other tables need auditing.
Thanks again!
Steve.
"David Portas" wrote:
> Never put cursors in triggers. They just turn your set-based updates
> into slow, inefficient and over-complex row-by-row operations.
> Dynamic SQL is usually a bad idea too. The security implications of
> dynamic code make it particularly unsuitable for audit processes. A
> better method to keep this "generic" IMO is to generate static trigger
> code automatically at design time using the information_schema.
> Your trigger code can look something like the following, for example.
> CREATE TRIGGER trg_foo_audit ON foo FOR INSERT, UPDATE, DELETE
> AS
> INSERT INTO audit_table (col1, col2, ...)
> SELECT D.col1, D.col2, ...
> FROM Inserted AS I
> JOIN Deleted AS D
> ON I.key_col = D.key_col
> WHERE COALESCE(I.col1,'') <> COALESCE(D.col1,'')
> AND COALESCE(I.col2,'') <> COALESCE(D.col2,'')
> Hope this helps.
> --
> David Portas
> SQL Server MVP
> --
>|||> how is this bad security?
Putting dynamic SQL in a trigger means that every user who has to
update the data must be granted update permissions on the tables,
including the audit table. In other words you lose all the potential
benefits of securing your data through stored procedures and your audit
trail could be wide open to abuse. Whether that's a real issue may
depend on how your middle tier is implemented but if your data access
was suitably abstracted thorugh middle tier code (on a web server, say)
you probably wouldn't need a trigger anyway - you could just call an SP
to do the work. As you've used triggers I'm guessing you haven't
implemented a middle tier in such a way as to prevent users getting
direct access to tables. If I'm wrong then I'm not sure why you would
use a trigger to do this at all.
Read the following article for more information on the issues around
dynamic code:
http://www.sommarskog.se/dynamic_sql.html
> Is that not what I had done? Using a curosr to loop through the
> information.schema?
Yes, but I'm proposing that you don't do it in a trigger. Instead, do
it at *design time* in order to generate the (static) trigger code.
David Portas
SQL Server MVP
--|||I've been having problems replying all day, "MS is having problems with your
request".
Thanks David, for taking the time to explain what to most people is probably
obvious. I guess this is the problem with developing solutions with no
formal training, after a year the essential basics are missing and I've got
bad habbits, I shall read that link!
Could I then in theory use the after update trigger to fire a sproc which
does the updating of the tables, would this work? In that the user does not
have permission on the audit table, but the sproc is run by an account which
can only update the audit table and is NOT on the windows network (we use
windows authentication).
Or should I be reading more on the security chapters now too :) (I think I
know the answer to that one).
Currently there is no middle tier, just Access 2000 projects (.adp). Some
of our users are on dialup vpn connections, so server side work seemed to
make sense at the time.
"David Portas" wrote:
> Putting dynamic SQL in a trigger means that every user who has to
> update the data must be granted update permissions on the tables,
> including the audit table. In other words you lose all the potential
> benefits of securing your data through stored procedures and your audit
> trail could be wide open to abuse. Whether that's a real issue may
> depend on how your middle tier is implemented but if your data access
> was suitably abstracted thorugh middle tier code (on a web server, say)
> you probably wouldn't need a trigger anyway - you could just call an SP
> to do the work. As you've used triggers I'm guessing you haven't
> implemented a middle tier in such a way as to prevent users getting
> direct access to tables. If I'm wrong then I'm not sure why you would
> use a trigger to do this at all.
> Read the following article for more information on the issues around
> dynamic code:
> http://www.sommarskog.se/dynamic_sql.html
>
> Yes, but I'm proposing that you don't do it in a trigger. Instead, do
> it at *design time* in order to generate the (static) trigger code.
> --
> David Portas
> SQL Server MVP
> --
>|||Sorry, me again, having gone through your posts again Im
as to what
your suggesting. ie
Automatically? I don't understand, design time is surely now, how would I
generate it automatically now and why?
Surely automatically implies at run-time, not design time?
Your example of :
INSERT INTO audit_table (col1, col2, ...)
SELECT D.col1, D.col2, ...
FROM Inserted AS I
JOIN Deleted AS D
ON I.key_col = D.key_col
WHERE COALESCE(I.col1,'') <> COALESCE(D.col1,'')
AND COALESCE(I.col2,'') <> COALESCE(D.col2,'')
How does this account for possibly more than 1 table with 20-40 columns
each? Surely this method means you have to specifically name ALL the column
s
which could be affected, and you have to have an audit table per source tabl
e
or you have a massive audit table which has most of its columns blank, as in
this example the columns need to match (insert into blah select blah).
The route I was looking at meant the only 1 audit table and only 1 trigger
no matter how many tables are to be audited (just change the source table
name and it will work).
My audit table only had one column for the column name, and the column name
was used as actual data rather than the crosstab look of boat loads of
columns to fit every circumstance.
Apologies, as Im sure I must have simply missunderstood, is there any chance
of a more detailed explanation of what "automatically" means and how your
example accounts for >1 table and loads of columns?
Thanks again.
Steve
"Steve'o" wrote:
> I've been having problems replying all day, "MS is having problems with yo
ur
> request".
> Thanks David, for taking the time to explain what to most people is probab
ly
> obvious. I guess this is the problem with developing solutions with no
> formal training, after a year the essential basics are missing and I've go
t
> bad habbits, I shall read that link!
> Could I then in theory use the after update trigger to fire a sproc which
> does the updating of the tables, would this work? In that the user does n
ot
> have permission on the audit table, but the sproc is run by an account whi
ch
> can only update the audit table and is NOT on the windows network (we use
> windows authentication).
> Or should I be reading more on the security chapters now too :) (I think
I
> know the answer to that one).
> Currently there is no middle tier, just Access 2000 projects (.adp). Some
> of our users are on dialup vpn connections, so server side work seemed to
> make sense at the time.
> "David Portas" wrote:
>|||Steve'o wrote:
> Sorry, me again, having gone through your posts again Im
as to wh
at
> your suggesting. ie
>
>
> Automatically? I don't understand, design time is surely now, how would I
> generate it automatically now and why?
> Surely automatically implies at run-time, not design time?
>
What I imagine he's suggesting is that you move your automatic discovery
of table columns out of the trigger and into some code which writes a
script to create the triggers. Your triggers currently are operating
inefficiently because they are written to dynamic discover information
(the columns) at runtime which isn't dynamic at runtime. Every single
time the triggers fire, they suffer a penalty by having to rediscover
the same columns over and over again.
What's preferable is to write some small program or somewhat less small
SQL script which reads the information about your tables and their
columns from INFORMATION_SCHEMA views or other system info source and
then THAT would assemble and output a SQL script containing dynamically
generated CREATE TRIGGER instructions. The trigger code itself would
then be static and efficient because you've done the column discovery
externally.
For an example you can look at our OmniAudit program to create audit
triggers (hey, I did tell you how to do it yourself first). Just looking
at the triggers created by the trial version will give you some ideas
for your own trigger-generating process.
http://www.krell-software.com/omniaudit
Steve Troxell
http://www.krell-software.com
|||