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

Thanks for the review, Greg. Glad it's useful, and thanks for trying it
across the version range.

Attached is v2, which addresses your points:

- Added test coverage for the settings the feature is really about. The
test now initdb's the old cluster with data checksums and a non-default WAL
segment size, and after the upgrade checks that the new cluster inherited
the checksum setting, WAL segment size, encoding, collation, ctype, and
locale provider.

- The overwrite test now confirms the failure comes from pg_upgrade's own
PG_VERSION check ("already contains a database system") rather than
initdb's non-empty-directory error.

- Added a test for the "initdb not present" case (pointing --new-bindir at
an empty dir), since it was easy enough to cover.

- Reworked the -O handling. Rather than dropping it entirely, I took your
alternative: only "-c name=value" options are forwarded to initdb now, and
anything else is skipped with a warning. So -O "-c work_mem=..." works, and
-O "-B 12345" no longer breaks initdb.

That keeps GUC pass-through working without pretending initdb accepts the
full postmaster option set.

- Moved the "refuse to overwrite" check up to right after the
initdb-present check.

- Fixed the copyright year on the new test file.

On the v10 failure ("invalid connection option max_protocol_version"):
that comes from stock pg_upgrade, which adds max_protocol_version=3.0 to
the connection string for pre-PG11 servers (server.c). It's not introduced
by --initdb, the same connection is made in the normal flow, so I've left
it out of scope for this patch.

The patch is pgindent-clean and the TAP test passes. For convenience, the
branch is also on GitHub:

github.com/LeeBohyun/postgres/tree/pg_upgrade_initdb

Best regards,

Bohyun

On Fri, Jul 10, 2026 at 7:24 PM Greg Sabino Mullane <htamfids(at)gmail(dot)com>
wrote:

> 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
>
>