From: Bruce Momjian <bruce@momjian.us>
To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Magnus Hagander <magnus@hagander.net>
Cc: Daniel Gustafsson <daniel@yesql.se>
Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Subject: Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file
Date: Thu, 19 Mar 2020 15:23:04 -0400
Message-ID: <20200319192304.GA28431@momjian.us> (raw)
In-Reply-To: <87k149w673.fsf@wibble.ilmari.org>
References: <87pne2w98h.fsf@wibble.ilmari.org>
<44115C15-EE49-4BDE-A976-2901384B5E60@yesql.se>
<87mu96w7c5.fsf@wibble.ilmari.org>
<20200226014844.GF17153@paquier.xyz>
<2760811A-9AF4-4725-8A8A-061940CCF962@yesql.se>
<CABUevEygyP_o9LmEWbSGzP3Wu-sstGSrQE9kq_WpuOEy1FOm5g@mail.gmail.com>
<20200226093551.GA106669@paquier.xyz>
<9878.1582732525@sss.pgh.pa.us>
<87k149w673.fsf@wibble.ilmari.org>
On Wed, Feb 26, 2020 at 06:32:00PM +0000, Dagfinn Ilmari Mannsåker wrote:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>
> > Michael Paquier <michael@paquier.xyz> writes:
> >> On Wed, Feb 26, 2020 at 10:06:38AM +0100, Magnus Hagander wrote:
> >>> +1, seems like that would be a regression in value.
> >
> >> Having more generic messages is less work for translators, we have
> >> PG_VERSION in the file name, and that's more complicated to translate
> >> in both French and Japanese. No idea about other languages.
> >
> > Just looking at the committed diff, it seems painfully obvious that these
> > two messages were written by different people who weren't talking to each
> > other. Why aren't they more alike? Given
> >
> > pg_fatal("could not open version file \"%s\": %m\n", ver_filename);
> >
> > (which seems fine to me), I think the second ought to be
> >
> > pg_fatal("could not parse version file \"%s\"\n", ver_filename);
>
> Good point. Patch attached.
Patch applied, and other adjustments:
This patch fixes the error message in get_major_server_version()
to be "could not parse version file", and uses the full file path
name, rather than just the data directory path.
Also, commit 4109bb5de4 added the cause of the failure to the
"could not open" error message, and improved quoting. This patch
backpatches the "could not open" cause to PG 12, where it was
first widely used, and backpatches the quoting fix in that patch
to all supported releases.
Because some of the branches are different, I am attaching the applied
multi-version patch.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +
Attachments:fix.diff (text/x-diff, 4.6K) download
9.5:
diff --git a/src/bin/pg_upgrade/server.c b/src/bin/pg_upgrade/server.cindex da3aefca82..c59da91f9d 100644--- a/src/bin/pg_upgrade/server.c+++ b/src/bin/pg_upgrade/server.c@@ -164,12 +164,12 @@ get_major_server_version(ClusterInfo *cluster)
snprintf(ver_filename, sizeof(ver_filename), "%s/PG_VERSION",
cluster->pgdata);
if ((version_fd = fopen(ver_filename, "r")) == NULL)
- pg_fatal("could not open version file: %s\n", ver_filename);+ pg_fatal("could not open version file \"%s\"\n", ver_filename);
if (fscanf(version_fd, "%63s", cluster->major_version_str) == 0 ||
sscanf(cluster->major_version_str, "%d.%d", &integer_version,
&fractional_version) != 2)
- pg_fatal("could not get version from %s\n", cluster->pgdata);+ pg_fatal("could not parse version file \"%s\"\n", ver_filename);
fclose(version_fd);
9.6:
diff --git a/src/bin/pg_upgrade/server.c b/src/bin/pg_upgrade/server.cindex 1cd606a847..862a50c254 100644--- a/src/bin/pg_upgrade/server.c+++ b/src/bin/pg_upgrade/server.c@@ -165,12 +165,12 @@ get_major_server_version(ClusterInfo *cluster)
snprintf(ver_filename, sizeof(ver_filename), "%s/PG_VERSION",
cluster->pgdata);
if ((version_fd = fopen(ver_filename, "r")) == NULL)
- pg_fatal("could not open version file: %s\n", ver_filename);+ pg_fatal("could not open version file \"%s\"\n", ver_filename);
if (fscanf(version_fd, "%63s", cluster->major_version_str) == 0 ||
sscanf(cluster->major_version_str, "%d.%d", &integer_version,
&fractional_version) != 2)
- pg_fatal("could not get version from %s\n", cluster->pgdata);+ pg_fatal("could not parse version file \"%s\"\n", ver_filename);
fclose(version_fd);
10:
diff --git a/src/bin/pg_upgrade/server.c b/src/bin/pg_upgrade/server.cindex 5563a5020b..abcb4b176b 100644--- a/src/bin/pg_upgrade/server.c+++ b/src/bin/pg_upgrade/server.c@@ -165,11 +165,11 @@ get_major_server_version(ClusterInfo *cluster)
snprintf(ver_filename, sizeof(ver_filename), "%s/PG_VERSION",
cluster->pgdata);
if ((version_fd = fopen(ver_filename, "r")) == NULL)
- pg_fatal("could not open version file: %s\n", ver_filename);+ pg_fatal("could not open version file \"%s\"\n", ver_filename);
if (fscanf(version_fd, "%63s", cluster->major_version_str) == 0 ||
sscanf(cluster->major_version_str, "%d.%d", &v1, &v2) < 1)
- pg_fatal("could not parse PG_VERSION file from %s\n", cluster->pgdata);+ pg_fatal("could not parse version file \"%s\"\n", ver_filename);
fclose(version_fd);
11:
diff --git a/src/bin/pg_upgrade/server.c b/src/bin/pg_upgrade/server.cindex fccc21836a..d2391137a8 100644--- a/src/bin/pg_upgrade/server.c+++ b/src/bin/pg_upgrade/server.c@@ -165,11 +165,11 @@ get_major_server_version(ClusterInfo *cluster)
snprintf(ver_filename, sizeof(ver_filename), "%s/PG_VERSION",
cluster->pgdata);
if ((version_fd = fopen(ver_filename, "r")) == NULL)
- pg_fatal("could not open version file: %s\n", ver_filename);+ pg_fatal("could not open version file \"%s\"\n", ver_filename);
if (fscanf(version_fd, "%63s", cluster->major_version_str) == 0 ||
sscanf(cluster->major_version_str, "%d.%d", &v1, &v2) < 1)
- pg_fatal("could not parse PG_VERSION file from %s\n", cluster->pgdata);+ pg_fatal("could not parse version file \"%s\"\n", ver_filename);
fclose(version_fd);
12:
diff --git a/src/bin/pg_upgrade/server.c b/src/bin/pg_upgrade/server.cindex 6ad4b14e16..0b5967c49b 100644--- a/src/bin/pg_upgrade/server.c+++ b/src/bin/pg_upgrade/server.c@@ -165,11 +165,11 @@ get_major_server_version(ClusterInfo *cluster)
snprintf(ver_filename, sizeof(ver_filename), "%s/PG_VERSION",
cluster->pgdata);
if ((version_fd = fopen(ver_filename, "r")) == NULL)
- pg_fatal("could not open version file: %s\n", ver_filename);+ pg_fatal("could not open version file \"%s\": %m\n", ver_filename);
if (fscanf(version_fd, "%63s", cluster->major_version_str) == 0 ||
sscanf(cluster->major_version_str, "%d.%d", &v1, &v2) < 1)
- pg_fatal("could not parse PG_VERSION file from %s\n", cluster->pgdata);+ pg_fatal("could not parse version file \"%s\"\n", ver_filename);
fclose(version_fd);
head:
diff --git a/src/bin/pg_upgrade/server.c b/src/bin/pg_upgrade/server.cindex be604d3351..f669bb4e8a 100644--- a/src/bin/pg_upgrade/server.c+++ b/src/bin/pg_upgrade/server.c@@ -168,7 +168,7 @@ get_major_server_version(ClusterInfo *cluster)
if (fscanf(version_fd, "%63s", cluster->major_version_str) == 0 ||
sscanf(cluster->major_version_str, "%d.%d", &v1, &v2) < 1)
- pg_fatal("could not parse PG_VERSION file from \"%s\"\n", cluster->pgdata);+ pg_fatal("could not parse version file \"%s\"\n", ver_filename);
fclose(version_fd);
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: pgsql-hackers@postgresql.org
Cc: bruce@momjian.us, ilmari@ilmari.org, tgl@sss.pgh.pa.us, michael@paquier.xyz, magnus@hagander.net, daniel@yesql.se, pgsql-hackers@lists.postgresql.org
Subject: Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file
In-Reply-To: <20200319192304.GA28431@momjian.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox