Re: Fixing memory leaks in postgres_fdw - Mailing list pgsql-hackers

From Etsuro Fujita
Subject Re: Fixing memory leaks in postgres_fdw
Date
Msg-id CAPmGK17iOh6W25UswGcn5k75jEukSgLMpiaJEATcAZQTec7xxg@mail.gmail.com
Whole thread Raw
In response to Fixing memory leaks in postgres_fdw  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
Hi,

On Sat, May 24, 2025 at 10:10 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> The DirectModify code path relies on PG_TRY blocks to ensure that it
> releases the PGresult for the foreign modify command, but that can't
> work because (at least in cases with RETURNING data) the PGresult has
> to survive across successive calls to postgresIterateDirectModify.
> If an error occurs in the query in between those steps, we have no
> opportunity to clean up.

Ugh.

> I thought of fixing this by using a memory context reset callback
> to ensure that the PGresult is cleaned up when the executor's context
> goes away, and that seems to work nicely (see 0001 attached).
> However, I feel like this is just a POC, because now that we have that
> concept we might be able to use it elsewhere in postgres_fdw to
> eliminate most or even all of its reliance on PG_TRY.  That should be
> faster as well as much less bug-prone.  But I'm putting it up at this
> stage for comments, in case anyone thinks this is not the direction to
> head in.

I think that that is a good idea; +1 for removing the reliance not
only in DirectModify but in other places.  I think that that would be
also useful if extending batch INSERT to cases with RETURNING data in
postgres_fdw.

Thanks for working on this!

Best regards,
Etsuro Fujita



pgsql-hackers by date:

Previous
From: * Neustradamus *
Date:
Subject: Please update your contact list: It must be pgsql-hackers@lists.postgresql.org
Next
From: Alexander Korotkov
Date:
Subject: Re: Slot's restart_lsn may point to removed WAL segment after hard restart unexpectedly