Thread: Fix PGresult leak in pg_dump during binary upgrade

While looking at pg_dump performance today I noticed that pg_dump fails to
clear query results in binary_upgrade_set_pg_class_oids during binary upgrade
mode.  9a974cbcba00 moved the query to the outer block, but left the PQclear
and query buffer destruction in the is_index conditional, making it not always
be executed.  353708e1fb2d fixed the leak of the query buffer but left the
PGresult leak.  The attached fixes the PGresult leak which when upgrading large
schemas can be non-trivial.

This needs to be backpatched down to v15.

--
Daniel Gustafsson

Daniel Gustafsson <daniel@yesql.se> writes:
> While looking at pg_dump performance today I noticed that pg_dump fails to
> clear query results in binary_upgrade_set_pg_class_oids during binary upgrade
> mode.  9a974cbcba00 moved the query to the outer block, but left the PQclear
> and query buffer destruction in the is_index conditional, making it not always
> be executed.  353708e1fb2d fixed the leak of the query buffer but left the
> PGresult leak.  The attached fixes the PGresult leak which when upgrading large
> schemas can be non-trivial.

+1 --- in 353708e1f I was just fixing what Coverity complained about.
I wonder why it missed this; it does seem to understand that PGresult
leaks are a thing.  But anyway, I missed it too.

            regards, tom lane

> On 15 May 2024, at 20:46, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Daniel Gustafsson <daniel@yesql.se> writes:
>> While looking at pg_dump performance today I noticed that pg_dump fails to
>> clear query results in binary_upgrade_set_pg_class_oids during binary upgrade
>> mode.  9a974cbcba00 moved the query to the outer block, but left the PQclear
>> and query buffer destruction in the is_index conditional, making it not always
>> be executed.  353708e1fb2d fixed the leak of the query buffer but left the
>> PGresult leak.  The attached fixes the PGresult leak which when upgrading large
>> schemas can be non-trivial.
>
> +1 --- in 353708e1f I was just fixing what Coverity complained about.
> I wonder why it missed this; it does seem to understand that PGresult
> leaks are a thing.  But anyway, I missed it too.

Done, backpatched to v15. Thanks for review!

--
Daniel Gustafsson