Re: pg_upgrade version check improvements and small fixes

Dan McGee wrote:
> Not sure what the normal process is for patches, but I put together a
> few small patches for pg_upgrade after trying to use it earlier today
> and staring a non-helpful error message before I finally figured out
> what was going on.

Thanks for the detailed report and patches. Let me address each one
individually.

> 0001 is just a simple typo fix, but didn't want to mix it in with the rest.

I applied this message capitalization fix to 9.0, 9.1, and master (9.2).

> 0002 moves a function around to be declared in the only place it is
> needed, and prevents a "sh: /oasdfpt/pgsql-8.4/bin/pg_config: No such
> file or directory" error message when you give it a bogus bindir.

You are right that I was calling pg_ctl before I had validated the bin
directory, and you were right that the function wasn't in an ideal
C file.

What I did was to move the function, and bundle all the
pg_upgrade_support test into that single function, so the function now
either errors out or returns void.

Patch attached and applied to 9.1 and master. Good catch.

> 0003 is what I really wanted to solve, which was my failure with
> pg_upgrade. The call to pg_ctl didn't succeed because the binaries
> didn't match the data directory, thus resulting in this:
>
> $ pg_upgrade --check -d /tmp/olddata -D /tmp/newdata -b /usr/bin/ -B /usr/bin/
> Performing Consistency Checks
> -----------------------------
> Checking old data directory (/tmp/olddata) ok
> Checking old bin directory (/usr/bin) ok
> Checking new data directory (/tmp/newdata) ok
> Checking new bin directory (/usr/bin) ok
> pg_resetxlog: pg_control exists but is broken or unknown version; ignoring it
> Trying to start old server .................ok
>
> Unable to start old postmaster with the command: "/usr/bin/pg_ctl" -l
> "/dev/null" -D "/tmp/olddata" -o "-p 5432 -c autovacuum=off -c
> autovacuum_freeze_max_age=2000000000" start >> "/dev/null" 2>&1
> Perhaps pg_hba.conf was not set to "trust".
>
> The error had nothing to do with "trust" at all; it was simply that I
> tried to use 9.0 binaries with an 8.4 data directory. My patch checks
> for this and ensures that the -D bindir is the correct version, just
> as the -B datadir has to be the correct version.

I had not thought about testing if the bin and data directory were the
same major version, but you are right that it generates an odd error if
they are not.

I changed your sscanf format string because the version can be just
"9.2dev" and there is no need for the minor version. I saw no reason
to test if the binary version matches the pg_upgrade version because we
already test the cluster version, and we test the cluster version is the
same as the binary version.

Patch attached and applied to 9.1 and master.

> I'm not on the mailing list nor do I have a lot of free time to keep
> up with normal development, but if there are quick things I can do to
> get these patches in let me know.

All done!

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

+ It's impossible for everything to be true. +