Re: Table refer leak in logical replication

On Mon, Apr 19, 2021 at 3:02 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Mon, Apr 19, 2021 at 02:33:10PM +0530, Amit Kapila wrote:
> > On Mon, Apr 19, 2021 at 12:32 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> >> FWIW, I agree with fixing this bug of 1375422c in as least scary
> >> manner as possible. Hou-san proposed that we add the ResultRelInfo
> >> that apply_handle_{insert|update|delete} initialize themselves to
> >> es_opened_result_relations. I would prefer that only
> >> ExecInitResultRelation() add anything to es_opened_result_relations()
> >> to avoid future maintenance problems. Instead, a fix as simple as the
> >> Hou-san's proposed fix would be to add a ExecCloseResultRelations()
> >> call at the end of each of apply_handle_{insert|update|delete}.
> >
> > Yeah, that will work too but might look a bit strange. BTW, how that
> > is taken care of for ExecuteTruncateGuts? I mean we do add rels there
> > like Hou-San's patch without calling ExecCloseResultRelations, the
> > rels are probably closed when we close the relation in worker.c but
> > what about memory for the list?
>
> TRUNCATE relies on FreeExecutorState() for that, no?
>

I am not sure about that because it doesn't seem to be allocated in
es_query_cxt. Note, we switch to oldcontext in the
CreateExecutorState.

--
With Regards,
Amit Kapila.