pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feed[PATCH] pg_upgrade: report the reason for failing to open the cluster version file
11+ messages / 6 participants
[nested] [flat]
Commits from this thread:
2020-03-19 6802942fa2 2247a1ea5f 9f62ea27e3 c9c042dded 7df528312d 2c9864e516 pg_upgrade: make get_major_server_version() err msg consistent
2020-02-26 4109bb5de4 Include error code in message from pg_upgrade
* [PATCH] pg_upgrade: report the reason for failing to open the cluster version file
@ 2020-02-25 23:14 Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-02-25 23:31 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Daniel Gustafsson <daniel@yesql.se>
0 siblings, 1 reply; 11+ messages in thread
From: ilmari@ilmari.org @ 2020-02-25 23:14 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org
Hi Hackers,
The other day I was helping someone with pg_upgrade on IRC, and they got
a rather unhelpful error message:
ERROR: could not open version file /path/to/new/cluster/PG_VERSION
It would have saved some minutes of debugging time if that had included
the reason why the open failed, so here's a patch to do so.
- ilmari
--
- Twitter seems more influential [than blogs] in the 'gets reported in
the mainstream press' sense at least. - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
to a mainstream media article. - Calle Dybedahl
Attachments:
0001-pg_upgrade-add-m-to-version-file-open-failure-messag.patch (text/x-diff, 1005B) download
From 84b99582a3447213f666c2f6f52c22ef518d82d5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Tue, 25 Feb 2020 18:07:14 +0000
Subject: [PATCH] pg_upgrade: add %m to version file open failure message
---
src/bin/pg_upgrade/server.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/bin/pg_upgrade/server.c b/src/bin/pg_upgrade/server.c
index e244256501..c9b56ae175 100644
--- a/src/bin/pg_upgrade/server.c
+++ b/src/bin/pg_upgrade/server.c
@@ -164,7 +164,7 @@ 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)
--
2.22.0
* Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file
2020-02-25 23:14 [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
@ 2020-02-25 23:31 ` Daniel Gustafsson <daniel@yesql.se>
2020-02-25 23:55 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Gustafsson @ 2020-02-25 23:31 UTC (permalink / raw)
To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; +Cc: pgsql-hackers@lists.postgresql.org
> On 26 Feb 2020, at 00:14, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:
> It would have saved some minutes of debugging time if that had included
> the reason why the open failed, so here's a patch to do so.
+1 on the attached patch. A quick skim across the similar error reportings in
pg_upgrade doesn't turn up any others which lack the more detailed information.
> - pg_fatal("could not open version file: %s\n", ver_filename);
> + pg_fatal("could not open version file \"%s\": %m\n", ver_filename);
A few lines further down from this we report an error in case we are unable to
parse the file in question:
pg_fatal("could not parse PG_VERSION file from %s\n", cluster->pgdata);
Should the pgdata argument be quoted there as well, like \"%s\", to make it
consistent for how we report filenames and directories in pg_upgrade?
cheers ./daniel
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file
2020-02-25 23:14 [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-02-25 23:31 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Daniel Gustafsson <daniel@yesql.se>
@ 2020-02-25 23:55 ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-02-26 01:48 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 11+ messages in thread
From: ilmari@ilmari.org @ 2020-02-25 23:55 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; +Cc: pgsql-hackers@lists.postgresql.org
Daniel Gustafsson <daniel@yesql.se> writes:
> A few lines further down from this we report an error in case we are unable to
> parse the file in question:
>
> pg_fatal("could not parse PG_VERSION file from %s\n", cluster->pgdata);
>
> Should the pgdata argument be quoted there as well, like \"%s\", to make it
> consistent for how we report filenames and directories in pg_upgrade?
Good point, I agree we should. Updated patch attached.
- ilmari
--
"A disappointingly low fraction of the human race is,
at any given time, on fire." - Stig Sandbeck Mathisen
Attachments:
v2-0001-pg_upgrade-add-m-to-version-file-open-failure-mes.patch (text/x-diff, 1.2K) download
From a503ffdb1499e73bfb75373a9af2e766e279beeb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Tue, 25 Feb 2020 18:07:14 +0000
Subject: [PATCH v2] pg_upgrade: add %m to version file open failure message
Also quote the file name in the error messages.
---
src/bin/pg_upgrade/server.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/bin/pg_upgrade/server.c b/src/bin/pg_upgrade/server.c
index e244256501..be604d3351 100644
--- a/src/bin/pg_upgrade/server.c
+++ b/src/bin/pg_upgrade/server.c
@@ -164,11 +164,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 PG_VERSION file from \"%s\"\n", cluster->pgdata);
fclose(version_fd);
--
2.22.0
* Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file
2020-02-25 23:14 [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-02-25 23:31 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Daniel Gustafsson <daniel@yesql.se>
2020-02-25 23:55 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
@ 2020-02-26 01:48 ` Michael Paquier <michael@paquier.xyz>
2020-02-26 08:56 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Daniel Gustafsson <daniel@yesql.se>
0 siblings, 1 reply; 11+ messages in thread
From: Michael Paquier @ 2020-02-26 01:48 UTC (permalink / raw)
To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; +Cc: Daniel Gustafsson <daniel@yesql.se>; pgsql-hackers@lists.postgresql.org
On Tue, Feb 25, 2020 at 11:55:06PM +0000, Dagfinn Ilmari Mannsåker wrote:
> @@ -164,11 +164,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);
Here I think that it would be better to just use "could not open
file" as we know that we are dealing with a version file already
thanks to 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 PG_VERSION file from \"%s\"\n", cluster->pgdata);
>
> fclose(version_fd);
No objection to this one.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file
2020-02-25 23:14 [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-02-25 23:31 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Daniel Gustafsson <daniel@yesql.se>
2020-02-25 23:55 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-02-26 01:48 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Michael Paquier <michael@paquier.xyz>
@ 2020-02-26 08:56 ` Daniel Gustafsson <daniel@yesql.se>
2020-02-26 09:06 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Magnus Hagander <magnus@hagander.net>
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Gustafsson @ 2020-02-26 08:56 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; pgsql-hackers@lists.postgresql.org
> On 26 Feb 2020, at 02:48, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Tue, Feb 25, 2020 at 11:55:06PM +0000, Dagfinn Ilmari Mannsåker wrote:
>> @@ -164,11 +164,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);
>
> Here I think that it would be better to just use "could not open
> file" as we know that we are dealing with a version file already
> thanks to ver_filename.
Isn't that a removal of detail with very little benefit? Not everyone running
pg_upgrade will know internal filenames, and the ver_filename contains the
pgdata path as well which might provide additional clues in case this goes
wrong.
cheers ./daniel
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file
2020-02-25 23:14 [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-02-25 23:31 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Daniel Gustafsson <daniel@yesql.se>
2020-02-25 23:55 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-02-26 01:48 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Michael Paquier <michael@paquier.xyz>
2020-02-26 08:56 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Daniel Gustafsson <daniel@yesql.se>
@ 2020-02-26 09:06 ` Magnus Hagander <magnus@hagander.net>
2020-02-26 09:35 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 11+ messages in thread
From: Magnus Hagander @ 2020-02-26 09:06 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; +Cc: Michael Paquier <michael@paquier.xyz>; Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
On Wed, Feb 26, 2020 at 9:56 AM Daniel Gustafsson <daniel@yesql.se> wrote:
>
> > On 26 Feb 2020, at 02:48, Michael Paquier <michael@paquier.xyz> wrote:
> >
> > On Tue, Feb 25, 2020 at 11:55:06PM +0000, Dagfinn Ilmari Mannsåker wrote:
> >> @@ -164,11 +164,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);
> >
> > Here I think that it would be better to just use "could not open
> > file" as we know that we are dealing with a version file already
> > thanks to ver_filename.
>
> Isn't that a removal of detail with very little benefit? Not everyone running
> pg_upgrade will know internal filenames, and the ver_filename contains the
> pgdata path as well which might provide additional clues in case this goes
> wrong.
+1, seems like that would be a regression in value.
Committed as per Dagfinn's v2.
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file
2020-02-25 23:14 [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-02-25 23:31 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Daniel Gustafsson <daniel@yesql.se>
2020-02-25 23:55 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-02-26 01:48 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Michael Paquier <michael@paquier.xyz>
2020-02-26 08:56 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Daniel Gustafsson <daniel@yesql.se>
2020-02-26 09:06 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Magnus Hagander <magnus@hagander.net>
@ 2020-02-26 09:35 ` Michael Paquier <michael@paquier.xyz>
2020-02-26 15:55 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 11+ messages in thread
From: Michael Paquier @ 2020-02-26 09:35 UTC (permalink / raw)
To: Magnus Hagander <magnus@hagander.net>; +Cc: Daniel Gustafsson <daniel@yesql.se>; Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
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.
> Committed as per Dagfinn's v2.
Anyway, too late :)
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file
2020-02-25 23:14 [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-02-25 23:31 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Daniel Gustafsson <daniel@yesql.se>
2020-02-25 23:55 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-02-26 01:48 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Michael Paquier <michael@paquier.xyz>
2020-02-26 08:56 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Daniel Gustafsson <daniel@yesql.se>
2020-02-26 09:06 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Magnus Hagander <magnus@hagander.net>
2020-02-26 09:35 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Michael Paquier <michael@paquier.xyz>
@ 2020-02-26 15:55 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-02-26 18:32 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
0 siblings, 1 reply; 11+ messages in thread
From: Tom Lane @ 2020-02-26 15:55 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Magnus Hagander <magnus@hagander.net>; Daniel Gustafsson <daniel@yesql.se>; Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
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);
The wording as it stands:
pg_fatal("could not parse PG_VERSION file from \"%s\"\n", cluster->pgdata);
could be criticized on more grounds than just that it's pointlessly
different from the adjacent message: it doesn't follow the style guideline
about saying what each mentioned object is. You could fix that maybe with
s/from/from directory/, but I think this construction is unfortunate and
overly verbose already.
regards, tom lane
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file
2020-02-25 23:14 [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-02-25 23:31 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Daniel Gustafsson <daniel@yesql.se>
2020-02-25 23:55 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-02-26 01:48 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Michael Paquier <michael@paquier.xyz>
2020-02-26 08:56 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Daniel Gustafsson <daniel@yesql.se>
2020-02-26 09:06 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Magnus Hagander <magnus@hagander.net>
2020-02-26 09:35 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Michael Paquier <michael@paquier.xyz>
2020-02-26 15:55 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Tom Lane <tgl@sss.pgh.pa.us>
@ 2020-02-26 18:32 ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-03-19 19:23 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Bruce Momjian <bruce@momjian.us>
0 siblings, 1 reply; 11+ messages in thread
From: ilmari@ilmari.org @ 2020-02-26 18:32 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Michael Paquier <michael@paquier.xyz>; Magnus Hagander <magnus@hagander.net>; Daniel Gustafsson <daniel@yesql.se>; PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
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.
- ilmari
--
- Twitter seems more influential [than blogs] in the 'gets reported in
the mainstream press' sense at least. - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
to a mainstream media article. - Calle Dybedahl
Attachments:
0001-Make-pg_upgrade-s-version-file-parsing-error-message.patch (text/x-diff, 910B) download
From a761148357420fdf81589e28df701cc1fde4cb87 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Wed, 26 Feb 2020 18:29:03 +0000
Subject: [PATCH] Make pg_upgrade's version file parsing error message
consistent
---
src/bin/pg_upgrade/server.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/bin/pg_upgrade/server.c b/src/bin/pg_upgrade/server.c
index 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);
--
2.20.1
* Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file
2020-02-25 23:14 [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-02-25 23:31 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Daniel Gustafsson <daniel@yesql.se>
2020-02-25 23:55 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-02-26 01:48 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Michael Paquier <michael@paquier.xyz>
2020-02-26 08:56 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Daniel Gustafsson <daniel@yesql.se>
2020-02-26 09:06 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Magnus Hagander <magnus@hagander.net>
2020-02-26 09:35 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Michael Paquier <michael@paquier.xyz>
2020-02-26 15:55 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Tom Lane <tgl@sss.pgh.pa.us>
2020-02-26 18:32 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
@ 2020-03-19 19:23 ` Bruce Momjian <bruce@momjian.us>
2020-03-19 19:40 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
0 siblings, 1 reply; 11+ messages in thread
From: Bruce Momjian @ 2020-03-19 19:23 UTC (permalink / raw)
To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Michael Paquier <michael@paquier.xyz>; Magnus Hagander <magnus@hagander.net>; Daniel Gustafsson <daniel@yesql.se>; PostgreSQL Developers <pgsql-hackers@lists.postgresql.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.c
index 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.c
index 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.c
index 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.c
index 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.c
index 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.c
index 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);
* Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file
2020-02-25 23:14 [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-02-25 23:31 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Daniel Gustafsson <daniel@yesql.se>
2020-02-25 23:55 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-02-26 01:48 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Michael Paquier <michael@paquier.xyz>
2020-02-26 08:56 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Daniel Gustafsson <daniel@yesql.se>
2020-02-26 09:06 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Magnus Hagander <magnus@hagander.net>
2020-02-26 09:35 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Michael Paquier <michael@paquier.xyz>
2020-02-26 15:55 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Tom Lane <tgl@sss.pgh.pa.us>
2020-02-26 18:32 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-03-19 19:23 ` Re: [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Bruce Momjian <bruce@momjian.us>
@ 2020-03-19 19:40 ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
0 siblings, 0 replies; 11+ messages in thread
From: ilmari@ilmari.org @ 2020-03-19 19:40 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Michael Paquier <michael@paquier.xyz>; Magnus Hagander <magnus@hagander.net>; Daniel Gustafsson <daniel@yesql.se>; PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Bruce Momjian <bruce@momjian.us> writes:
> 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:
Thanks!
- ilmari
--
- Twitter seems more influential [than blogs] in the 'gets reported in
the mainstream press' sense at least. - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
to a mainstream media article. - Calle Dybedahl
^ permalink raw reply [nested|flat] 11+ messages in thread
end of thread, other threads:[~2020-03-19 19:40 UTC | newest]
Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-02-25 23:14 [PATCH] pg_upgrade: report the reason for failing to open the cluster version file Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-02-25 23:31 ` Daniel Gustafsson <daniel@yesql.se>
2020-02-25 23:55 ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-02-26 01:48 ` Michael Paquier <michael@paquier.xyz>
2020-02-26 08:56 ` Daniel Gustafsson <daniel@yesql.se>
2020-02-26 09:06 ` Magnus Hagander <magnus@hagander.net>
2020-02-26 09:35 ` Michael Paquier <michael@paquier.xyz>
2020-02-26 15:55 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-02-26 18:32 ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-03-19 19:23 ` Bruce Momjian <bruce@momjian.us>
2020-03-19 19:40 ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
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