Re: Table refer leak in logical replication

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!

+ 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.

+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.

Why don't you create the table beforehand rather than making it part
of the trigger function?

+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.

+ "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.
--
Michael