public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Jeff Davis <[email protected]>
Cc: [email protected]
Subject: Re: pgsql: pg_upgrade: Fix inconsistency in memory freeing
Date: Sun, 30 Mar 2025 07:03:01 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
On Sat, Mar 29, 2025 at 09:24:58AM -0700, Jeff Davis wrote:
> This seems to have broken the pg_upgrade test when olddump/oldinstall
> are set to PG14 or earlier:
>
> stderr:
> # Failed test 'check that locales in new cluster match original
> cluster'
> # at
> /home/jdavis/git/postgresql/src/bin/pg_upgrade/t/002_pg_upgrade.pl line
> 506.
> # got: '0|c|C|C|NULL'
> # expected: '0|c|C|C|'
> # Looks like you failed 1 test of 16.
>
> Looking at the commit, it seems you are escaping NULL as a literal.
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?
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?
--
Michael
Attachments:
[text/x-diff] upgrade-free-locale.patch (1.4K, 2-upgrade-free-locale.patch)
download | inline diff:
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 9295e46aed3e..89baec821ec5 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -441,7 +441,6 @@ set_locale_and_encoding(void)
char *datcollate_literal;
char *datctype_literal;
char *datlocale_literal = NULL;
- char *datlocale_src;
DbLocaleInfo *locale = old_cluster.template0;
prep_status("Setting locale and encoding for new cluster");
@@ -455,10 +454,12 @@ set_locale_and_encoding(void)
datctype_literal = PQescapeLiteral(conn_new_template1,
locale->db_ctype,
strlen(locale->db_ctype));
- datlocale_src = locale->db_locale ? locale->db_locale : "NULL";
- datlocale_literal = PQescapeLiteral(conn_new_template1,
- datlocale_src,
- strlen(datlocale_src));
+ if (locale->db_locale)
+ datlocale_literal = PQescapeLiteral(conn_new_template1,
+ locale->db_locale,
+ strlen(locale->db_locale));
+ else
+ datlocale_literal = pg_strdup("NULL");
/* update template0 in new cluster */
if (GET_MAJOR_VERSION(new_cluster.major_version) >= 1700)
@@ -502,7 +503,10 @@ set_locale_and_encoding(void)
PQfreemem(datcollate_literal);
PQfreemem(datctype_literal);
- PQfreemem(datlocale_literal);
+ if (locale->db_locale)
+ PQfreemem(datlocale_literal);
+ else
+ pg_free(datlocale_literal);
PQfinish(conn_new_template1);
[application/pgp-signature] signature.asc (833B, 3-signature.asc)
download
view thread (8+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: pgsql: pg_upgrade: Fix inconsistency in memory freeing
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox