Re: pg_upgrade version checking questions

On Mon, Mar 18, 2019 at 07:35:17PM -0400, Tom Lane wrote:
> While poking around trying to find an explanation for the pg_upgrade
> failure described here:
> https://www.postgresql.org/message-id/flat/CACmJi2JUhGo2ZxqDkh-EPHNjEN1ZA1S64uHLJFWHBhUuV4492w%40mail.gmail.com
> I noticed a few things that seem a bit fishy about pg_upgrade.
> I can't (yet) connect any of these to Tomasz' problem, but:
>
> 1. check_bin_dir() does validate_exec() for pg_dumpall and pg_dump,
> but not for pg_restore, though pg_upgrade surely calls that too.
> For that matter, it's not validating initdb and vacuumdb, though
> it's grown dependencies on those as well. Seems like there's little
> point in checking these if we're not going to check all of them.

Yes, adding those checks would be nice. I guess I never suspected there
would be mixed-version binaries in that directory.

> 2. check_cluster_versions() insists that the target version be the
> same major version as pg_upgrade itself, but is that really good enough?
> As things stand, it looks like pg_upgrade 11.3 would happily use pg_dump
> 11.1, or vice versa. With this rule, we cannot safely make any fixes
> in minor releases that rely on synchronized changes in the behavior of
> pg_upgrade and pg_dump/pg_dumpall/pg_restore. I've not gone looking
> to see if we've already made such changes in the past, but even if we
> never have, that's a rather tight-looking straitjacket. I think we
> should insist that the new_cluster.bin_version be an exact match
> to pg_upgrade's own PG_VERSION_NUM.

Again, I never considered minor-version changes, so yeah, forcing minor
version matching makes sense.

> 3. Actually, I'm kind of wondering why pg_upgrade has a --new-bindir
> option at all, rather than just insisting on finding the new-version
> executables in the same directory it is in. This seems like, at best,
> a hangover from before it got into core. Even if you don't want to
> remove the option, we could surely provide a useful default setting
> based on find_my_exec. (I'm amused to notice that pg_upgrade
> currently takes the trouble to find out its own path, and then does
> precisely nothing with the information.)

Good point. You are right that when it was outside of the source tree,
and even in /contrib, that would not have worked easily. Makes sense to
at least default to the same directory as pg_upgrade.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +