On Mon, Apr 19, 2021 at 3:25 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Mon, Apr 19, 2021 at 3:12 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Mon, Apr 19, 2021 at 3:02 PM Michael Paquier <mich...@paquier.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 <amitlangot...@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? > > > > > > > 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. > > > > I have just checked that the memory for the list is allocated in > ApplyMessageContext. So, it appears a memory leak to me unless I am > missing something. >
It seems like the memory will be freed after we apply the truncate because we reset the ApplyMessageContext after applying each message, so maybe we don't need to bother about it. -- With Regards, Amit Kapila.