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/