Hi Thomas,

On 2017/05/16 9:12, Thomas Munro wrote:
> Hi hackers,
> While testing the patch I'm writing for the transition table open
> item, I noticed that we can leak Relation objects like this:
> postgres=# create table parent (a text, b int) partition by list (a);
> postgres=# create table child partition of parent for values in ('AAA');
> postgres=# create or replace function my_trigger_func() returns
> trigger language plpgsql as $$ begin raise notice 'hello'; return
> null; end; $$;
> postgres=# create trigger child_trigger after insert or update or
> delete on child for each row execute procedure my_trigger_func();
> postgres=# copy parent (a, b) from stdin;
> Enter data to be copied followed by a newline.
> End with a backslash and a period on a line by itself.
>>> AAA 42
>>> \.
> NOTICE:  hello
> WARNING:  relcache reference leak: relation "child" not closed
> COPY 1

Thanks for the test case and the patch.

> The leaked object is created by ExecGetTriggerResultRel, called by
> afterTriggerInvokeEvents.  That function relies on someone, usually
> ExecEndPlan or EvalPlanQualEnd, to clean up es_trig_target_relations.
> Shouldn't copy.c do the same thing (see attached)?  Or perhaps there
> should there be an ExecCleanupTriggerState(estate) used by all places?

I vote for ExecCleanupTriggerState(estate).  After your patch, there will
be 4 places, including afterTriggerInvokeEvents(), ExecEndPlan(), and
EvalPlanQualEnd(), that repeat the same block of code.


Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to