Re: Fixing memory leaks in postgres_fdw

Hi,

On Sat, May 24, 2025 at 10:10 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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