On Fri, Mar 09, 2007 at 06:50:39PM -0500, Tom Lane wrote: > Bill Moseley <moseley@xxxxxxxx> writes: > > I'm asking for a sanity check: > > > And then an audit table: > > > create table template_history ( > > id SERIAL PRIMARY KEY, > > template_id integer NOT NULL REFERENCES template ON DELETE CASCADE, > > path text NOT NULL, > > content text NOT NULL, > > last_updated_time timestamp(0) with time zone NOT NULL > > ); > > Why would you want ON DELETE CASCADE? Or for that matter to have a > foreign key here at all? Surely the point of an audit table is to > remember history. If the audit entries all disappear the instant > the main-table entry is deleted, it's not much of an audit tool. In this case the templates are short lived so only want to track history while the "live" record is around. That is, it's expected that the template and all its history will get wiped out once in a while. Still, I agree with you that it would be better to use a different approach and not cascade delete. The foreign key is there so can find the history related to the primary record. That's what ties the history records together (the path can change during the life of the template). > > > My trigger is very simple: > > > CREATE OR REPLACE FUNCTION audit_template() RETURNS TRIGGER AS ' > > BEGIN > > INSERT INTO template_history > > ( template_id, path, content, last_updated_time, person ) > > select > > id, path, content, last_updated_time, person > > from > > template where id = 1; > > RETURN NEW; > > END' > > language 'plpgsql'; > > This is not going to work because the row's not there yet. This is a BEFORE *UPDATE* trigger, not a BEFORE INSERT, so the row is there. The audit table is written when the primary record changes and the old version is written to the audit table, not the new version. Yes, it's more common to write the audit for every insert and update (so the most recent version is also in the audit table). > (I won't bother pointing out the thinko in the WHERE clause); Darn cut-n-paste errors > and even if it did > work it'd be unnecessarily inefficient. Just use the NEW row that's > passed to the trigger: > > INSERT INTO template_history(...) VALUES(NEW.id, NEW.path, ...) Ok, but as the id is a sequence. I need to test if NEW.id is set after the insert -- seems like not, IIRC, and I'd need to use curval(). > If you have other BEFORE triggers on this table that can change the > NEW row, then it might be better to make this an AFTER trigger so it can > be sure the NEW row it sees won't change anymore. But AFTER triggers > are distinctly less efficient, so if you're not intending to add more > triggers then using a BEFORE trigger is probably the way to go. It's just that pesky sequence I need access to after the insert happens. Thanks for the tips, Tom. -- Bill Moseley moseley@xxxxxxxx
