Re: pg_upgrade version checking questions

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
Cc: Christoph Berg <myon(at)debian(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Tomasz Szypowski <tomasz(dot)szypowski(at)gmail(dot)com>
Subject: Re: pg_upgrade version checking questions
Date: 2019-07-25 14:33:44
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

> On 24 Jul 2019, at 22:32, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
> On 2019-07-23 17:30, Daniel Gustafsson wrote:
>> The reason for moving is that we print default values in usage(), and that
>> requires the value to be computed before calling usage(). We already do this
>> for resolving environment values in parseCommandLine(). If we do it in setup,
>> then we’d have to split out resolving the new_cluster.bindir into it’s own
>> function exposed to option.c, or do you have any other suggestions there?
> I think doing nontrivial work in order to print default values in
> usage() is bad practice, because in unfortunate cases it would even
> prevent you from calling --help. Also, in this case, it would probably
> very often exceed the typical line length of --help output and create
> some general ugliness. Writing something like "(default: same as this
> pg_upgrade)" would probably achieve just about the same.

Fair enough, those are both excellent points. I’ve shuffled the code around to
move back the check for exec_path to setup (albeit earlier than before due to
where we perform directory checking). This does mean that the directory
checking in the options parsing must learn to cope with missing directories,
which is a bit unfortunate since it’s already doing a few too many things IMHO.
To ensure dogfooding, I also removed the use of -B in ‘make check’ for
pg_upgrade, which should bump the coverage.

Also spotted a typo in a pg_upgrade file header in a file touched by this, so
included it in this thread too as a 0004.

Thanks again for reviewing, much appreciated!

cheers ./daniel

Attachment Content-Type Size
0004-Fix-typo-in-pg_upgrade-file-header-v4.patch application/octet-stream 571 bytes
0003-Default-new-bindir-to-exec_path-v4.patch application/octet-stream 7.4 KB
0002-Check-all-used-executables-v4.patch application/octet-stream 1.5 KB
0001-Only-allow-upgrades-by-the-same-exact-version-new-v4.patch application/octet-stream 2.2 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2019-07-25 14:44:13 Re: Initdb failure
Previous Message Rafia Sabih 2019-07-25 14:10:47 Re: Initdb failure