Re: creating audit tables

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(at)cshl(dot)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(at)cshl(dot)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(at)cshl(dot)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(at)cshl(dot)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(at)cshl(dot)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(at)cshl(dot)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(at)cshl(dot)org
GMOD Coordinator (http://www.gmod.org/) 216-392-3087
Cold Spring Harbor Laboratory

Scott Cain <cain(at)cshl(dot)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(at)cshl(dot)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(at)cshl(dot)org
GMOD Coordinator (http://www.gmod.org/) 216-392-3087
Cold Spring Harbor Laboratory