Re: [PATCH] pg_upgrade: add --initdb option to create the new cluster automatically

My review. Overall, this looks really nice and a much needed feature. I
will definitely be using this a lot in the future :)

It cleanly compiled on top of 97a18c22 (from Fri Jul 10 22:34:24 2026), and
I was able to successfully do the upgrade via the new --initdb flag for
versions 11 through 18. Version 10 did not work, but that is not the fault
of this patch. But the output for completeness, since we still support
upgrading from v10:

Inspecting old cluster locale for new cluster creation
invalid connection option "max_protocol_version"

I also verified that it correctly maintains the checksum setting, the
locale, weird WAL segment sizes, and the new icu and builtin locale
options. However, the tests do none of those checks, and they really ought
to, as that's an important part of what this feature provides.

> Extra initdb options can be passed via the existing -O flag and will be
> forwarded to the initdb invocation.

I don't like this part, as there is not a 100% overlap between what
postmaster accepts and what initdb accepts. The -c options are fine, but
things like -O "-B 12345" cause a failure. I'd simply not add the -O
options here: if people really need them, they can run pg_upgrade without
the --initdb flag. Alternatively, only allow -c items, but that's a real
pain to code that.

> +static void create_new_cluster_via_initdb(void);

Nice, I love descriptive function names.

> + * resolve_new_bindir()

Good idea, well executed.

> +/* Refuse to overwrite an existing cluster. */

Move this up to just after the other check that initdb is present?

> + snprintf(verfile, sizeof(verfile), "%s/PG_VERSION",

I suppose this is slightly better than just checking if the dir is empty or
not, as initdb itself will later fail if non-empty but does not contain
PG_VERSION. So a weak +1. Sort of has test coverage (but see below)

> + * Verify that initdb is present and executable before doing any work.

Also not tested at all (may be overkill to do so, but pointing out for
completeness)

> +# Copyright (c) 2022-2025, PostgreSQL Global Development Group

s/2025/2026/ - or perhaps just say "2026" as this is a new file

> +# --initdb must refuse to clobber an already-populated data directory.
> +command_fails(

Would be nice if we confirmed this was failing due to the new PG_VERSION
check, versus the initdb "ain't gonna write to non empty dir" error.

Cheers,
Greg