Re: Bogus error handling in pg_upgrade

On Sun, Dec 29, 2013 at 12:25:04AM -0500, Tom Lane wrote:
> A credulous person might suppose that this chunk of code is designed
> to abort if pg_resetxlog fails:
>
> prep_status("Setting next transaction ID for new cluster");
> exec_prog(UTILITY_LOG_FILE, NULL, true,
> "\"%s/pg_resetxlog\" -f -x %u \"%s\"",
> new_cluster.bindir, old_cluster.controldata.chkpnt_nxtxid,
> new_cluster.pgdata);
> check_ok();
>
> In point of fact, it does no such thing, but blithely continues
> (even though pg_resetxlog has corrupted things horribly before failing).

Well, exec_prog() does this:

result = system(cmd);

if (result != 0)

So, is pg_resetxlog returning a zero value? I am guessing it is.

> check_ok() is particularly badly named, since it contains not one iota
> of error checking. misleadingly_claim_ok() would be a better name.
>
> If this isn't broken-by-design, I'd like an explanation why not.

It is probably because it came from initdb.c, but I always read check_ok
as report_ok. Should I rename it?

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

+ Everyone has their own god. +