Thread: creating audit tables
Hello, I am trying to create audit tables for all of the tables in my database. The function, table and trigger create statements are below. Apparently, I am not doing it quite right, because I get these messages when I try to run the create statements below: CREATE FUNCTION CREATE FUNCTION CREATE TABLE CREATE TABLE GRANT ERROR: function audit_update() does not exist ERROR: function audit_delete() does not exist Why do I get a message that the functions don't exist when they were just successfully created? Thanks much, Scott Here's the ddl: CREATE FUNCTION audit_update(varchar) RETURNS trigger AS ' DECLARE audit_table varchar; table_name varchar; BEGIN table_name = $1; audit_table = ''audit_'' || table_name; INSERT INTO audit_table VALUES (SELECT OLD.*,now(),''U'' FROM table_name); return NEW; END ' LANGUAGE plpgsql; CREATE FUNCTION audit_delete(varchar) RETURNS trigger AS ' DECLARE audit_table varchar; table_name varchar; BEGIN table_name = $1; audit_table = ''audit_'' || table_name; INSERT INTO audit_table VALUES (SELECT OLD.*,now(),''D'' FROM table_name); return OLD; END ' LANGUAGE plpgsql; create table tableinfo ( tableinfo_id serial not null, primary key (tableinfo_id), name varchar(30) not null, primary_key_column varchar(30) null, is_view int not null default 0, view_on_table_id int null, superclass_table_id int null, is_updateable int not null default 1, modification_date date not null default now(), constraint tableinfo_c1 unique (name) ); CREATE TABLE audit_tableinfo ( tableinfo_id integer, name varchar, primary_key_column varchar, is_view integer, view_on_table_id integer, superclass_table_id integer, is_updateable integer, modification_date date, transaction_date timestamp not null, transaction_type char not null ); GRANT ALL on audit_tableinfo to PUBLIC; CREATE TRIGGER tableinfo_audit_u BEFORE UPDATE ON tableinfo FOR EACH ROW EXECUTE PROCEDURE audit_update('tableinfo'); CREATE TRIGGER tableinfo_audit_d BEFORE DELETE ON tableinfo FOR EACH ROW EXECUTE PROCEDURE audit_delete('tableinfo'); -- ------------------------------------------------------------------------ Scott Cain, Ph. D. cain@cshl.org GMOD Coordinator (http://www.gmod.org/) 216-392-3087 Cold Spring Harbor Laboratory
Scott Cain <cain@cshl.org> writes: > I am trying to create audit tables for all of the tables in my > database. The function, table and trigger create statements are below. > Apparently, I am not doing it quite right, because I get these messages > when I try to run the create statements below: Trigger functions don't take any explicit parameters. Everything they need they get through specialized mechanisms (in plpgsql, it's special variables like tgargv). regards, tom lane
OK, I've reworked my function and I can now create my functions and triggers; however, when I try to do a test update, I get the following error: ERROR: syntax error at or near "$1" at character 14 CONTEXT: PL/pgSQL function "audit_update" line 7 at SQL statement Which I think corresponds to 'audit_table' in the INSERT line below: CREATE FUNCTION audit_update() RETURNS trigger AS ' DECLARE audit_table text; table_name text; BEGIN table_name = TG_RELNAME; audit_table = ''audit_'' || table_name; INSERT INTO audit_table VALUES (SELECT OLD.*,now(),''U'' FROM table_name); return NEW; END ' LANGUAGE plpgsql; I am trying to dynamically construct the audit table's name from the TG_RELNAME variable (the audit table is always named as the name of the original table with 'audit_' prepended to it). Is this not a valid thing to do? Thanks, Scott On Wed, 2004-10-13 at 23:59, Tom Lane wrote: > Scott Cain <cain@cshl.org> writes: > > I am trying to create audit tables for all of the tables in my > > database. The function, table and trigger create statements are below. > > Apparently, I am not doing it quite right, because I get these messages > > when I try to run the create statements below: > > Trigger functions don't take any explicit parameters. Everything they > need they get through specialized mechanisms (in plpgsql, it's special > variables like tgargv). > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 4: Don't 'kill -9' the postmaster -- ------------------------------------------------------------------------ Scott Cain, Ph. D. cain@cshl.org GMOD Coordinator (http://www.gmod.org/) 216-392-3087 Cold Spring Harbor Laboratory
I think you want to EXECUTE that sql so it doesn't get compiled into the function. http://www.postgresql.org/docs/7.4/static/plpgsql-statements.html#PLPGSQL-STATEMENTS-EXECUTING-DYN - Ian >>> Scott Cain <cain@cshl.org> 10/14/04 8:01 AM >>> OK, I've reworked my function and I can now create my functions and triggers; however, when I try to do a test update, I get the following error: ERROR: syntax error at or near "$1" at character 14 CONTEXT: PL/pgSQL function "audit_update" line 7 at SQL statement Which I think corresponds to 'audit_table' in the INSERT line below: CREATE FUNCTION audit_update() RETURNS trigger AS ' DECLARE audit_table text; table_name text; BEGIN table_name = TG_RELNAME; audit_table = ''audit_'' || table_name; INSERT INTO audit_table VALUES (SELECT OLD.*,now(),''U'' FROM table_name); return NEW; END ' LANGUAGE plpgsql; I am trying to dynamically construct the audit table's name from the TG_RELNAME variable (the audit table is always named as the name of the original table with 'audit_' prepended to it). Is this not a valid thing to do? Thanks, Scott On Wed, 2004-10-13 at 23:59, Tom Lane wrote: > Scott Cain <cain@cshl.org> writes: > > I am trying to create audit tables for all of the tables in my > > database. The function, table and trigger create statements are below. > > Apparently, I am not doing it quite right, because I get these messages > > when I try to run the create statements below: > > Trigger functions don't take any explicit parameters. Everything they > need they get through specialized mechanisms (in plpgsql, it's special > variables like tgargv). > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 4: Don't 'kill -9' the postmaster -- ------------------------------------------------------------------------ Scott Cain, Ph. D. cain@cshl.org GMOD Coordinator (http://www.gmod.org/) 216-392-3087 Cold Spring Harbor Laboratory ---------------------------(end of broadcast)--------------------------- TIP 7: don't forget to increase your free space map settings
I feel like I am getting very close, but I am still not quite there. I rewrote the trigger function below to use execute, but now I get the following error: ERROR: OLD used in query that is not in a rule CONTEXT: PL/pgSQL function "audit_update" line 5 at execute statement It seems that I am not able to use OLD in this context, but that is exactly what I need to do, to get the contents of the old row in the original table to put it in the audit table. Here is the function now: CREATE FUNCTION audit_update() RETURNS trigger AS ' DECLARE audit_table text; BEGIN audit_table = ''audit_''||TG_RELNAME; EXECUTE ''INSERT INTO '' ||quote_ident(audit_table) ||'' VALUES ('' ||OLD.* ||'','' ||now() ||'',''''U'''')''; return NEW; END ' LANGUAGE plpgsql; Thanks again, Scott On Thu, 2004-10-14 at 11:16, Ian Harding wrote: > I think you want to EXECUTE that sql so it doesn't get compiled into the > function. > > http://www.postgresql.org/docs/7.4/static/plpgsql-statements.html#PLPGSQL-STATEMENTS-EXECUTING-DYN > > - Ian > > >>> Scott Cain <cain@cshl.org> 10/14/04 8:01 AM >>> > OK, I've reworked my function and I can now create my functions and > triggers; however, when I try to do a test update, I get the following > error: > > ERROR: syntax error at or near "$1" at character 14 > CONTEXT: PL/pgSQL function "audit_update" line 7 at SQL statement > > Which I think corresponds to 'audit_table' in the INSERT line below: > > CREATE FUNCTION audit_update() RETURNS trigger > AS ' > DECLARE > audit_table text; > table_name text; > BEGIN > table_name = TG_RELNAME; > audit_table = ''audit_'' || table_name; > INSERT INTO audit_table VALUES (SELECT OLD.*,now(),''U'' FROM > table_name); > return NEW; > END > ' > LANGUAGE plpgsql; > > I am trying to dynamically construct the audit table's name from the > TG_RELNAME variable (the audit table is always named as the name of the > original table with 'audit_' prepended to it). Is this not a valid > thing to do? > > Thanks, > Scott > > On Wed, 2004-10-13 at 23:59, Tom Lane wrote: > > Scott Cain <cain@cshl.org> writes: > > > I am trying to create audit tables for all of the tables in my > > > database. The function, table and trigger create statements are > below. > > > Apparently, I am not doing it quite right, because I get these > messages > > > when I try to run the create statements below: > > > > Trigger functions don't take any explicit parameters. Everything they > > need they get through specialized mechanisms (in plpgsql, it's special > > variables like tgargv). > > > > regards, tom lane > > > > ---------------------------(end of > broadcast)--------------------------- > > TIP 4: Don't 'kill -9' the postmaster -- ------------------------------------------------------------------------ Scott Cain, Ph. D. cain@cshl.org GMOD Coordinator (http://www.gmod.org/) 216-392-3087 Cold Spring Harbor Laboratory
Scott Cain wrote: > I feel like I am getting very close, but I am still not quite there. I > rewrote the trigger function below to use execute, but now I get the > following error: > > ERROR: OLD used in query that is not in a rule > CONTEXT: PL/pgSQL function "audit_update" line 5 at execute statement > > It seems that I am not able to use OLD in this context, but that is > exactly what I need to do, to get the contents of the old row in the > original table to put it in the audit table. Here is the function now: > > CREATE FUNCTION audit_update() RETURNS trigger > AS ' > DECLARE > audit_table text; > BEGIN > audit_table = ''audit_''||TG_RELNAME; > EXECUTE ''INSERT INTO '' > ||quote_ident(audit_table) > ||'' VALUES ('' > ||OLD.* > ||'','' > ||now() > ||'',''''U'''')''; > return NEW; > END > ' > LANGUAGE plpgsql; Looks like people were fixing your errors, not looking at what you were trying to do. Apologies, but it's easy to fixate on an error message. Unless something is changing in 8.0 you're using the wrong tool for the job here. Plpgsql isn't good at dynamic queries, and can't unwrap OLD for you. Try a different language - tcl would be an obvious choice. -- Richard Huxton Archonet Ltd
Heck! So much for feeling close. It is somewhat frustrating to me that such an obviously useful tool (having and using audit tables) should be so difficult to implement. I thought I had a reasonable chance of doing it in plpgsql because I've written functions in that before--I have no idea how to do it in tkl. If someone would show me a simple example for doing this for one table, I will happily make available the script I am writing that will generate audit tables and the functions and triggers for using them automatically, given any ddl file. It is based on the Perl module SQL::Translator. Thanks, Scott On Thu, 2004-10-14 at 14:07, Richard Huxton wrote: > Scott Cain wrote: > > I feel like I am getting very close, but I am still not quite there. I > > rewrote the trigger function below to use execute, but now I get the > > following error: > > > > ERROR: OLD used in query that is not in a rule > > CONTEXT: PL/pgSQL function "audit_update" line 5 at execute statement > > > > It seems that I am not able to use OLD in this context, but that is > > exactly what I need to do, to get the contents of the old row in the > > original table to put it in the audit table. Here is the function now: > > > > CREATE FUNCTION audit_update() RETURNS trigger > > AS ' > > DECLARE > > audit_table text; > > BEGIN > > audit_table = ''audit_''||TG_RELNAME; > > EXECUTE ''INSERT INTO '' > > ||quote_ident(audit_table) > > ||'' VALUES ('' > > ||OLD.* > > ||'','' > > ||now() > > ||'',''''U'''')''; > > return NEW; > > END > > ' > > LANGUAGE plpgsql; > > Looks like people were fixing your errors, not looking at what you were > trying to do. Apologies, but it's easy to fixate on an error message. > > Unless something is changing in 8.0 you're using the wrong tool for the > job here. Plpgsql isn't good at dynamic queries, and can't unwrap OLD > for you. Try a different language - tcl would be an obvious choice. -- ------------------------------------------------------------------------ Scott Cain, Ph. D. cain@cshl.org GMOD Coordinator (http://www.gmod.org/) 216-392-3087 Cold Spring Harbor Laboratory
Here's what I do... It's not pretty but it works. create table auditlog ( auditwhen timestamp not null default CURRENT_TIMESTAMP, auditwhat char(10) not null, audittable varchar not null, auditkeyval int not null, auditfield varchar not null, oldval text null, newval text null); CREATE OR REPLACE FUNCTION "tsp_audit_atrig" () RETURNS trigger AS ' if {[string match $TG_op INSERT]} { foreach field $TG_relatts { if {[info exists NEW($field)]} { set sql "insert into auditlog (auditwhat, audittable, auditkeyval, " append sql "auditfield, newval) " append sql "values (''INSERT'', ''$1'', ''$NEW($2)'', ''$field'', " append sql "''$NEW($field)'')" spi_exec "$sql" } } } elseif {[string match $TG_op DELETE]} { foreach field $TG_relatts { if {[info exists OLD($field)]} { set sql "insert into auditlog (auditwhat, audittable, auditkeyval, " append sql "auditfield, oldval) " append sql "values (''DELETE'', ''$1'', ''$OLD($2)'', ''$field'', " append sql "''$OLD($field)'')" spi_exec "$sql" } } } elseif {[string match $TG_op UPDATE]} { foreach field $TG_relatts { # Was data changed or is this the key field? if {([info exists NEW($field)] && [info exists OLD($field)] && ![string match $OLD($field) $NEW($field)])} { set sql "insert into auditlog (auditwhat, audittable, auditkeyval, " append sql "auditfield, oldval, newval) " append sql "values (''UPDATE'', ''$1'', ''$NEW($2)'', ''$field'', " append sql "''$OLD($field)'', ''$NEW($field)'')" spi_exec "$sql" # Is this a field replacing a null? } elseif {[info exists NEW($field)] && ![info exists OLD($field)]} { set sql "insert into auditlog (auditwhat, audittable, auditkeyval, " append sql "auditfield, newval) " append sql "values (''UPDATE'', ''$1'', ''$NEW($2)'', ''$field'', " append sql "''$NEW($field)'')" spi_exec "$sql" # Is this a field being replaced with null? } elseif {![info exists NEW($field)] && [info exists OLD($field)]} { set sql "insert into auditlog (auditwhat, audittable, auditkeyval, " append sql "auditfield, oldval) " append sql "values (''UPDATE'', ''$1'', ''$NEW($2)'', ''$field'', " append sql "''$OLD($field)'')" spi_exec "$sql" } } } return "OK" ' LANGUAGE 'pltcl'; drop trigger trig_timecardaudit_atrig on timecard; CREATE TRIGGER "trig_timecardaudit_atrig" AFTER INSERT OR DELETE OR UPDATE ON "timec ard" FOR EACH ROW EXECUTE PROCEDURE "tsp_audit_atrig" ('timecard', 'timecardid'); Ian Harding Programmer/Analyst II Tacoma-Pierce County Health Department iharding@tpchd.org Phone: (253) 798-3549 Pager: (253) 754-0002 >>> Scott Cain <cain@cshl.org> 10/14/04 11:19 AM >>> Heck! So much for feeling close. It is somewhat frustrating to me that such an obviously useful tool (having and using audit tables) should be so difficult to implement. I thought I had a reasonable chance of doing it in plpgsql because I've written functions in that before--I have no idea how to do it in tkl. If someone would show me a simple example for doing this for one table, I will happily make available the script I am writing that will generate audit tables and the functions and triggers for using them automatically, given any ddl file. It is based on the Perl module SQL::Translator. Thanks, Scott On Thu, 2004-10-14 at 14:07, Richard Huxton wrote: > Scott Cain wrote: > > I feel like I am getting very close, but I am still not quite there. I > > rewrote the trigger function below to use execute, but now I get the > > following error: > > > > ERROR: OLD used in query that is not in a rule > > CONTEXT: PL/pgSQL function "audit_update" line 5 at execute statement > > > > It seems that I am not able to use OLD in this context, but that is > > exactly what I need to do, to get the contents of the old row in the > > original table to put it in the audit table. Here is the function now: > > > > CREATE FUNCTION audit_update() RETURNS trigger > > AS ' > > DECLARE > > audit_table text; > > BEGIN > > audit_table = ''audit_''||TG_RELNAME; > > EXECUTE ''INSERT INTO '' > > ||quote_ident(audit_table) > > ||'' VALUES ('' > > ||OLD.* > > ||'','' > > ||now() > > ||'',''''U'''')''; > > return NEW; > > END > > ' > > LANGUAGE plpgsql; > > Looks like people were fixing your errors, not looking at what you were > trying to do. Apologies, but it's easy to fixate on an error message. > > Unless something is changing in 8.0 you're using the wrong tool for the > job here. Plpgsql isn't good at dynamic queries, and can't unwrap OLD > for you. Try a different language - tcl would be an obvious choice. -- ------------------------------------------------------------------------ Scott Cain, Ph. D. cain@cshl.org GMOD Coordinator (http://www.gmod.org/) 216-392-3087 Cold Spring Harbor Laboratory ---------------------------(end of broadcast)--------------------------- TIP 4: Don't 'kill -9' the postmaster
Scott Cain <cain@cshl.org> writes: > Heck! So much for feeling close. It is somewhat frustrating to me that > such an obviously useful tool (having and using audit tables) should be > so difficult to implement. The only really reasonable way to implement this is as a C function anyway. I think anything involving a PL language is going to be a huge performance drag, if you intend to put it on essentially every table. There are some pretty closely related examples in contrib/spi/, though I don't see anything that does *exactly* what you want. If you came up with something that does, I think it'd be reasonable to add it to that set of examples ... regards, tom lane
Hi Tom, You are probably right that the performance will become an issue. I do have a working solution using plpgsql, though, so I will at least try it out for a while. For anyone who is interested, I created a template file (using the perl module Template.pm syntax) that works with the perl module SQL::Translator to examine my ddl file and create from it the audit tables and the functions and triggers to make them work. The template file copied below, and SQL::Translator is available from CPAN and from http://sqlfairy.sourceforge.net/ . Thanks, Scott ---------------------------------------------- --audit tables generated from -- % sqlt -f PostgreSQL -t TTSchema --template add-audits.tmpl nofuncs.sql > \ -- audits.sql [% FOREACH table IN schema.get_tables %] DROP TABLE audit_[% table.name %]; CREATE TABLE audit_[% table.name %] ( [% FOREACH field IN table.get_fields %] [% field.name %] [% IF field.data_type == 'serial'; 'int'; ELSE; field.data_type; END %][% IF field.size AND (field.data_type== 'char' OR field.data_type == 'varchar') %]([% field.size.join(', ') %])[% END %], [% END %] transaction_date timestamp not null default now(), transaction_type char(1) not null ); GRANT ALL on audit_[% table.name %] to PUBLIC; CREATE OR REPLACE FUNCTION audit_update_delete_[% table.name %]() RETURNS trigger AS ' DECLARE [% FOREACH field IN table.get_fields %][% field.name %]_var [% IF field.data_type == 'serial'; 'int'; ELSE; field.data_type;END %][% IF field.size AND (field.data_type == 'char' OR field.data_type == 'varchar') %]([% field.size.join(',') %])[% END %]; [% END %] transaction_type_var char; BEGIN [% FOREACH field IN table.get_fields %][% field.name %]_var = OLD.[% field.name %]; [% END %] IF TG_OP = ''DELETE'' THEN transaction_type_var = ''D''; ELSE transaction_type_var = ''U''; END IF; INSERT INTO audit_[% table.name %] ( [% FOREACH field IN table.get_fields %] [% field.name %], [% END %] transaction_type ) VALUES ( [% FOREACH field IN table.get_fields %] [% field.name %]_var, [% END %] transaction_type_var ); IF TG_OP = ''DELETE'' THEN return null; ELSE return NEW; END IF; END ' LANGUAGE plpgsql; DROP TRIGGER [% table.name %]_audit_ud ON [% table.name %]; CREATE TRIGGER [% table.name %]_audit_ud BEFORE UPDATE OR DELETE ON [% table.name %] FOR EACH ROW EXECUTE PROCEDURE audit_update_delete_[% table.name %] (); [% END %] On Fri, 2004-10-15 at 11:02, Tom Lane wrote: > Scott Cain <cain@cshl.org> writes: > > Heck! So much for feeling close. It is somewhat frustrating to me that > > such an obviously useful tool (having and using audit tables) should be > > so difficult to implement. > > The only really reasonable way to implement this is as a C function > anyway. I think anything involving a PL language is going to be a huge > performance drag, if you intend to put it on essentially every table. > > There are some pretty closely related examples in contrib/spi/, though > I don't see anything that does *exactly* what you want. If you came up > with something that does, I think it'd be reasonable to add it to that > set of examples ... > > regards, tom lane -- ------------------------------------------------------------------------ Scott Cain, Ph. D. cain@cshl.org GMOD Coordinator (http://www.gmod.org/) 216-392-3087 Cold Spring Harbor Laboratory
Hmm. You have an audit_ table for each table that is audited. I chose to have one big ugly audit table for all audited tables. I wonder which is more flexible/useful. Right off the bat I can see that if you add or rename a column you would need to add or rename a column in your audit_ table and re-produce the functions/triggers. I guess dropped columns would just show nulls from then on. Column name changes lose history of the field name too. Queries are a PITA with my schema, I can see where they would be easier with yours. I can imagine a pivot function that would make life easier with my schema though. Any thoughts would be appreciated, I might take a hack at this in C. - Ian >>> Scott Cain <cain@cshl.org> 10/15/04 8:27 AM >>> Hi Tom, You are probably right that the performance will become an issue. I do have a working solution using plpgsql, though, so I will at least try it out for a while. For anyone who is interested, I created a template file (using the perl module Template.pm syntax) that works with the perl module SQL::Translator to examine my ddl file and create from it the audit tables and the functions and triggers to make them work. The template file copied below, and SQL::Translator is available from CPAN and from http://sqlfairy.sourceforge.net/ . Thanks, Scott ---------------------------------------------- --audit tables generated from -- % sqlt -f PostgreSQL -t TTSchema --template add-audits.tmpl nofuncs.sql > \ -- audits.sql [% FOREACH table IN schema.get_tables %] DROP TABLE audit_[% table.name %]; CREATE TABLE audit_[% table.name %] ( [% FOREACH field IN table.get_fields %] [% field.name %] [% IF field.data_type == 'serial'; 'int'; ELSE; field.data_type; END %][% IF field.size AND (field.data_type == 'char' OR field.data_type == 'varchar') %]([% field.size.join(', ') %])[% END %], [% END %] transaction_date timestamp not null default now(), transaction_type char(1) not null ); GRANT ALL on audit_[% table.name %] to PUBLIC; CREATE OR REPLACE FUNCTION audit_update_delete_[% table.name %]() RETURNS trigger AS ' DECLARE [% FOREACH field IN table.get_fields %][% field.name %]_var [% IF field.data_type == 'serial'; 'int'; ELSE; field.data_type; END %][% IF field.size AND (field.data_type == 'char' OR field.data_type == 'varchar') %]([% field.size.join(', ') %])[% END %]; [% END %] transaction_type_var char; BEGIN [% FOREACH field IN table.get_fields %][% field.name %]_var = OLD.[% field.name %]; [% END %] IF TG_OP = ''DELETE'' THEN transaction_type_var = ''D''; ELSE transaction_type_var = ''U''; END IF; INSERT INTO audit_[% table.name %] ( [% FOREACH field IN table.get_fields %] [% field.name %], [% END %] transaction_type ) VALUES ( [% FOREACH field IN table.get_fields %] [% field.name %]_var, [% END %] transaction_type_var ); IF TG_OP = ''DELETE'' THEN return null; ELSE return NEW; END IF; END ' LANGUAGE plpgsql; DROP TRIGGER [% table.name %]_audit_ud ON [% table.name %]; CREATE TRIGGER [% table.name %]_audit_ud BEFORE UPDATE OR DELETE ON [% table.name %] FOR EACH ROW EXECUTE PROCEDURE audit_update_delete_[% table.name %] (); [% END %] On Fri, 2004-10-15 at 11:02, Tom Lane wrote: > Scott Cain <cain@cshl.org> writes: > > Heck! So much for feeling close. It is somewhat frustrating to me that > > such an obviously useful tool (having and using audit tables) should be > > so difficult to implement. > > The only really reasonable way to implement this is as a C function > anyway. I think anything involving a PL language is going to be a huge > performance drag, if you intend to put it on essentially every table. > > There are some pretty closely related examples in contrib/spi/, though > I don't see anything that does *exactly* what you want. If you came up > with something that does, I think it'd be reasonable to add it to that > set of examples ... > > regards, tom lane -- ------------------------------------------------------------------------ Scott Cain, Ph. D. cain@cshl.org GMOD Coordinator (http://www.gmod.org/) 216-392-3087 Cold Spring Harbor Laboratory
Hi Ian, I created one audit table for each table in the database just because that seemed to me to be the sensible thing to do. The reason we want audit tables is so that we can ask the question: "what was the state of the database 6 months ago" and the easiest way to answer that question is with shadow tables where I can write the same queries I do now, just changing (slightly) the table name and adding a date check to the where clause. Using a big, unified table makes it much harder to ask that sort of question, unless you spend a fair amount of effort making views to simulate the real audit tables I already have. I don't see any advantage to us in using a unified table. Scott On Fri, 2004-10-15 at 13:00, Ian Harding wrote: > Hmm. You have an audit_ table for each table that is audited. I chose > to have one big ugly audit table for all audited tables. I wonder which > is more flexible/useful. > > Right off the bat I can see that if you add or rename a column you would > need to add or rename a column in your audit_ table and re-produce the > functions/triggers. I guess dropped columns would just show nulls from > then on. Column name changes lose history of the field name too. > > Queries are a PITA with my schema, I can see where they would be easier > with yours. I can imagine a pivot function that would make life easier > with my schema though. > > Any thoughts would be appreciated, I might take a hack at this in C. > > - Ian > > >>> Scott Cain <cain@cshl.org> 10/15/04 8:27 AM >>> > Hi Tom, > > You are probably right that the performance will become an issue. I do > have a working solution using plpgsql, though, so I will at least try it > out for a while. > > For anyone who is interested, I created a template file (using the perl > module Template.pm syntax) that works with the perl module > SQL::Translator to examine my ddl file and create from it the audit > tables and the functions and triggers to make them work. The template > file copied below, and SQL::Translator is available from CPAN and from > http://sqlfairy.sourceforge.net/ . > > Thanks, > Scott > > ---------------------------------------------- > --audit tables generated from > -- % sqlt -f PostgreSQL -t TTSchema --template add-audits.tmpl > nofuncs.sql > \ > -- audits.sql > > > > [% FOREACH table IN schema.get_tables %] > DROP TABLE audit_[% table.name %]; > CREATE TABLE audit_[% table.name %] ( [% FOREACH field IN > table.get_fields %] > [% field.name %] [% IF field.data_type == 'serial'; 'int'; ELSE; > field.data_type; END %][% IF field.size AND (field.data_type == 'char' > OR field.data_type == 'varchar') %]([% field.size.join(', ') %])[% END > %], [% END %] > transaction_date timestamp not null default now(), > transaction_type char(1) not null > ); > GRANT ALL on audit_[% table.name %] to PUBLIC; > > > > CREATE OR REPLACE FUNCTION audit_update_delete_[% table.name %]() > RETURNS trigger AS > ' > DECLARE > [% FOREACH field IN table.get_fields %][% field.name %]_var [% IF > field.data_type == 'serial'; 'int'; ELSE; field.data_type; END %][% IF > field.size AND (field.data_type == 'char' OR field.data_type == > 'varchar') %]([% field.size.join(', ') %])[% END %]; > [% END %] > transaction_type_var char; > BEGIN > [% FOREACH field IN table.get_fields %][% field.name %]_var = > OLD.[% field.name %]; > [% END %] > IF TG_OP = ''DELETE'' THEN > transaction_type_var = ''D''; > ELSE > transaction_type_var = ''U''; > END IF; > > > > INSERT INTO audit_[% table.name %] ( [% FOREACH field IN > table.get_fields %] > [% field.name %], [% END %] > transaction_type > ) VALUES ( [% FOREACH field IN table.get_fields %] > [% field.name %]_var, [% END %] > transaction_type_var > ); > > > > IF TG_OP = ''DELETE'' THEN > return null; > ELSE > return NEW; > END IF; > END > ' > LANGUAGE plpgsql; > > > > DROP TRIGGER [% table.name %]_audit_ud ON [% table.name %]; > CREATE TRIGGER [% table.name %]_audit_ud > BEFORE UPDATE OR DELETE ON [% table.name %] > FOR EACH ROW > EXECUTE PROCEDURE audit_update_delete_[% table.name %] (); > > > > [% END %] > > > On Fri, 2004-10-15 at 11:02, Tom Lane wrote: > > Scott Cain <cain@cshl.org> writes: > > > Heck! So much for feeling close. It is somewhat frustrating to me > that > > > such an obviously useful tool (having and using audit tables) should > be > > > so difficult to implement. > > > > The only really reasonable way to implement this is as a C function > > anyway. I think anything involving a PL language is going to be a > huge > > performance drag, if you intend to put it on essentially every table. > > > > There are some pretty closely related examples in contrib/spi/, though > > I don't see anything that does *exactly* what you want. If you came > up > > with something that does, I think it'd be reasonable to add it to that > > set of examples ... > > > > regards, tom lane -- ------------------------------------------------------------------------ Scott Cain, Ph. D. cain@cshl.org GMOD Coordinator (http://www.gmod.org/) 216-392-3087 Cold Spring Harbor Laboratory
Ah, time travel. I don't think it will be quite that easy since if there was no modification of a record on that day, there will be no data returned, and if there were several modifications on that day, there will be several records returned. I think you will need a correlated subquery for each table looking for the max(<timestamp field>) where <= <your search condition timestamp>. There used to be a time travel module somewhere. My audit is more interested in "What is the history of my timecard for payperiod 7" which is very different that what you are doing. Good luck!! - Ian >>> Scott Cain <cain@cshl.org> 10/15/04 11:53 AM >>> Hi Ian, I created one audit table for each table in the database just because that seemed to me to be the sensible thing to do. The reason we want audit tables is so that we can ask the question: "what was the state of the database 6 months ago" and the easiest way to answer that question is with shadow tables where I can write the same queries I do now, just changing (slightly) the table name and adding a date check to the where clause. Using a big, unified table makes it much harder to ask that sort of question, unless you spend a fair amount of effort making views to simulate the real audit tables I already have. I don't see any advantage to us in using a unified table. Scott On Fri, 2004-10-15 at 13:00, Ian Harding wrote: > Hmm. You have an audit_ table for each table that is audited. I chose > to have one big ugly audit table for all audited tables. I wonder which > is more flexible/useful. > > Right off the bat I can see that if you add or rename a column you would > need to add or rename a column in your audit_ table and re-produce the > functions/triggers. I guess dropped columns would just show nulls from > then on. Column name changes lose history of the field name too. > > Queries are a PITA with my schema, I can see where they would be easier > with yours. I can imagine a pivot function that would make life easier > with my schema though. > > Any thoughts would be appreciated, I might take a hack at this in C. > > - Ian > > >>> Scott Cain <cain@cshl.org> 10/15/04 8:27 AM >>> > Hi Tom, > > You are probably right that the performance will become an issue. I do > have a working solution using plpgsql, though, so I will at least try it > out for a while. > > For anyone who is interested, I created a template file (using the perl > module Template.pm syntax) that works with the perl module > SQL::Translator to examine my ddl file and create from it the audit > tables and the functions and triggers to make them work. The template > file copied below, and SQL::Translator is available from CPAN and from > http://sqlfairy.sourceforge.net/ . > > Thanks, > Scott > > ---------------------------------------------- > --audit tables generated from > -- % sqlt -f PostgreSQL -t TTSchema --template add-audits.tmpl > nofuncs.sql > \ > -- audits.sql > > > > [% FOREACH table IN schema.get_tables %] > DROP TABLE audit_[% table.name %]; > CREATE TABLE audit_[% table.name %] ( [% FOREACH field IN > table.get_fields %] > [% field.name %] [% IF field.data_type == 'serial'; 'int'; ELSE; > field.data_type; END %][% IF field.size AND (field.data_type == 'char' > OR field.data_type == 'varchar') %]([% field.size.join(', ') %])[% END > %], [% END %] > transaction_date timestamp not null default now(), > transaction_type char(1) not null > ); > GRANT ALL on audit_[% table.name %] to PUBLIC; > > > > CREATE OR REPLACE FUNCTION audit_update_delete_[% table.name %]() > RETURNS trigger AS > ' > DECLARE > [% FOREACH field IN table.get_fields %][% field.name %]_var [% IF > field.data_type == 'serial'; 'int'; ELSE; field.data_type; END %][% IF > field.size AND (field.data_type == 'char' OR field.data_type == > 'varchar') %]([% field.size.join(', ') %])[% END %]; > [% END %] > transaction_type_var char; > BEGIN > [% FOREACH field IN table.get_fields %][% field.name %]_var = > OLD.[% field.name %]; > [% END %] > IF TG_OP = ''DELETE'' THEN > transaction_type_var = ''D''; > ELSE > transaction_type_var = ''U''; > END IF; > > > > INSERT INTO audit_[% table.name %] ( [% FOREACH field IN > table.get_fields %] > [% field.name %], [% END %] > transaction_type > ) VALUES ( [% FOREACH field IN table.get_fields %] > [% field.name %]_var, [% END %] > transaction_type_var > ); > > > > IF TG_OP = ''DELETE'' THEN > return null; > ELSE > return NEW; > END IF; > END > ' > LANGUAGE plpgsql; > > > > DROP TRIGGER [% table.name %]_audit_ud ON [% table.name %]; > CREATE TRIGGER [% table.name %]_audit_ud > BEFORE UPDATE OR DELETE ON [% table.name %] > FOR EACH ROW > EXECUTE PROCEDURE audit_update_delete_[% table.name %] (); > > > > [% END %] > > > On Fri, 2004-10-15 at 11:02, Tom Lane wrote: > > Scott Cain <cain@cshl.org> writes: > > > Heck! So much for feeling close. It is somewhat frustrating to me > that > > > such an obviously useful tool (having and using audit tables) should > be > > > so difficult to implement. > > > > The only really reasonable way to implement this is as a C function > > anyway. I think anything involving a PL language is going to be a > huge > > performance drag, if you intend to put it on essentially every table. > > > > There are some pretty closely related examples in contrib/spi/, though > > I don't see anything that does *exactly* what you want. If you came > up > > with something that does, I think it'd be reasonable to add it to that > > set of examples ... > > > > regards, tom lane -- ------------------------------------------------------------------------ Scott Cain, Ph. D. cain@cshl.org GMOD Coordinator (http://www.gmod.org/) 216-392-3087 Cold Spring Harbor Laboratory ---------------------------(end of broadcast)--------------------------- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to majordomo@postgresql.org so that your message can get through to the mailing list cleanly
Have you thought about unifying the audit + the current table and add from/to datestamps for every record? Example: from_dt to_dt value 9/1/2004 9/30/2004 ABC 9/30/2004 10/5/2004 XYZ 10/6/2004 12/31/9999 123 This would let you use the following query on the same table whether you wanted historic values or current values. SELECT * FROM table WHERE from_dt >= as_of_date AND to_dt <= as_of_date Scott Cain wrote: > Hi Ian, > > I created one audit table for each table in the database just because > that seemed to me to be the sensible thing to do. The reason we want > audit tables is so that we can ask the question: "what was the state of > the database 6 months ago" and the easiest way to answer that question > is with shadow tables where I can write the same queries I do now, just > changing (slightly) the table name and adding a date check to the where > clause. Using a big, unified table makes it much harder to ask that > sort of question, unless you spend a fair amount of effort making views > to simulate the real audit tables I already have. I don't see any > advantage to us in using a unified table. > > Scott
Oh yes. I do that a lot for attributes that need a history (last name, which changes when you get married, etc) It is a bit more complicated for queries though, since I use null to indicate an unknown end date instead of the Y2K problem solution below. -Ian >>> William Yu <wyu@talisys.com> 10/15/04 12:46 PM >>> Have you thought about unifying the audit + the current table and add from/to datestamps for every record? Example: from_dt to_dt value 9/1/2004 9/30/2004 ABC 9/30/2004 10/5/2004 XYZ 10/6/2004 12/31/9999 123 This would let you use the following query on the same table whether you wanted historic values or current values. SELECT * FROM table WHERE from_dt >= as_of_date AND to_dt <= as_of_date Scott Cain wrote: > Hi Ian, > > I created one audit table for each table in the database just because > that seemed to me to be the sensible thing to do. The reason we want > audit tables is so that we can ask the question: "what was the state of > the database 6 months ago" and the easiest way to answer that question > is with shadow tables where I can write the same queries I do now, just > changing (slightly) the table name and adding a date check to the where > clause. Using a big, unified table makes it much harder to ask that > sort of question, unless you spend a fair amount of effort making views > to simulate the real audit tables I already have. I don't see any > advantage to us in using a unified table. > > Scott ---------------------------(end of broadcast)--------------------------- TIP 4: Don't 'kill -9' the postmaster
12/31/9999 would be Y10K problem. :) Dunno about you but I sorta don't care if somebody has to rewrite my app 8000 years from now. :) Ian Harding wrote: > Oh yes. I do that a lot for attributes that need a history (last name, > which changes when you get married, etc) It is a bit more complicated > for queries though, since I use null to indicate an unknown end date > instead of the Y2K problem solution below. > > -Ian > > >>>>William Yu <wyu@talisys.com> 10/15/04 12:46 PM >>> > > Have you thought about unifying the audit + the current table and add > from/to datestamps for every record? > > Example: > > from_dt to_dt value > 9/1/2004 9/30/2004 ABC > 9/30/2004 10/5/2004 XYZ > 10/6/2004 12/31/9999 123