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);
> CREATE TABLE
> postgres=# create table child partition of parent for values in ('AAA');
> CREATE TABLE
> postgres=# create or replace function my_trigger_func() returns
> trigger language plpgsql as $$ begin raise notice 'hello'; return
> null; end; $$;
> CREATE FUNCTION
> postgres=# create trigger child_trigger after insert or update or
> delete on child for each row execute procedure my_trigger_func();
> CREATE TRIGGER
> 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.
Thanks,
Amit
