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 <br...@momjian.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 +
