On Wed, Apr 21, 2021 at 7:38 PM Michael Paquier <mich...@paquier.xyz> wrote: > On Wed, Apr 21, 2021 at 04:21:52PM +0900, Amit Langote wrote: > > So I had started last night by adding some tests for this in > > 003_constraints.pl because there are already some replica BR trigger > > tests there. I like your suggestion to have some tests around > > partitions, so added some in 013_partition.pl too. Let me know what > > you think. > > Thanks, cool!
Thanks for looking. > + IF (NEW.bid = 4 AND NEW.id = OLD.id) THEN > + RETURN NEW; > + ELSE > + RETURN NULL; > + END IF; > Nit: the indentation is a bit off here. Hmm, I checked that I used 4 spaces for indenting, but maybe you're concerned that the whole thing is indented unnecessarily relative to the parent ELSIF block? > +CREATE FUNCTION log_tab_fk_ref_upd() RETURNS TRIGGER AS \$\$ > +BEGIN > + CREATE TABLE IF NOT EXISTS public.tab_fk_ref_op_log (tgtab text, > tgop text, tgwhen text, tglevel text, oldbid int, newbid int); > + INSERT INTO public.tab_fk_ref_op_log SELECT TG_RELNAME, TG_OP, > TG_WHEN, TG_LEVEL, OLD.bid, NEW.bid; > + RETURN NULL; > +END; > Let's use only one function here, then you can just do something like > that and use NEW and OLD as you wish conditionally: > IF (TG_OP = 'INSERT') THEN > INSERT INTO tab_fk_ref_op_log blah; > ELSIF (TG_OP = 'UPDATE') THEN > INSERT INTO tab_fk_ref_op_log blah_; > END IF; > > The same remark applies to the two files where the tests have been > introduced. That's certainly better with fewer lines. > Why don't you create the table beforehand rather than making it part > of the trigger function? Makes sense too. > +CREATE TRIGGER tab_fk_ref_log_ins_after_trg > [...] > +CREATE TRIGGER tab_fk_ref_log_upd_after_trg > No need for two triggers either once there is only one function. Right. > + "SELECT * FROM tab_fk_ref_op_log ORDER BY tgop, newbid;"); > I would add tgtab and tgwhen to the ORDER BY here, just to be on the > safe side, and apply the same rule everywhere. Your patch is already > consistent regarding that and help future debugging, that's good. Okay, done. -- Amit Langote EDB: http://www.enterprisedb.com
fix_relcache_leak_in_lrworker_v8.patch
Description: Binary data
