Re: Table refer leak in logical replication

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@gmail.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?  FWIW, I'd rather
agree to use what has been proposed with es_opened_result_relations
like TRUNCATE does rather than attempt to use ExecInitResultRelation()
combined with potentially asymmetric calls to
ExecCloseResultRelations().
--
Michael