Re: pgsql: pg_upgrade: Fix inconsistency in memory freeing - Mailing list pgsql-committers

From Jeff Davis
Subject Re: pgsql: pg_upgrade: Fix inconsistency in memory freeing
Date
Msg-id 8e540416ec3f0a5850264505bf4c72844d32176a.camel@j-davis.com
Whole thread Raw
In response to Re: pgsql: pg_upgrade: Fix inconsistency in memory freeing  (Michael Paquier <michael@paquier.xyz>)
Responses Re: pgsql: pg_upgrade: Fix inconsistency in memory freeing
Re: pgsql: pg_upgrade: Fix inconsistency in memory freeing
List pgsql-committers
On Sun, 2025-03-30 at 07:03 +0900, Michael Paquier wrote:
> Thanks for the report.  It would be possible to switch to a second
> approach here, where we use pg_free() if we don't have a
> locale->db_locale to make sure that the memory is freed in its
> correct
> context, like in the attached.  What do you think?

Why pg_strdup() the "NULL" at all in that case? Usually I see that done
so that there doesn't need to be a conditional when freeing, but here
there's a conditional anyway.

Perhaps something like the attached?

> This test has been added in v16 via 9637badd9f92, with the buildfarm
> not complaining.  Could it be possible to improve the situation so as
> we would know about 002_pg_upgrade.pl failing for such cross-upgrades
> like what you are doing here?

Ideally the buildfarm would do cross-version upgrades the same way as
002_pg_upgrade.pl, in which case the test would have failed in the
buildfarm. But as it is, the only way to test the cross-version
upgrades in 002_pg_upgrade.pl is to initiate them manually by setting
olddump/oldinstall.

There are some ways that the buildfarm test is better, for example it
can go back more versions successfully. I'm not sure what all of the
reasons are, though.

Regards,
    Jeff Davis


Attachment

pgsql-committers by date:

Previous
From: Tomas Vondra
Date:
Subject: pgsql: Remove incidental md5() function use from test
Next
From: Andres Freund
Date:
Subject: pgsql: Add errhint_internal()