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

Thanks both for the reviews. v4 attached, addressing all of Greg's
second-pass points and Daniel's documentation point.

*Greg:*

-

resolve_new_bindir(): dropped the last two comment lines.
-

prep_status text is now "Examining old cluster settings".
-

Test: since checksums are now the default, the old cluster is created
with --no-data-checksums (a genuinely non-default value) and --locale=C.
The post-upgrade checks confirm the new cluster inherited both, along with
the WAL segment size, encoding, collation, ctype, and locale provider.
-

Test: replaced open()/print() with append_conf, and the port/socket
settings now follow the framework's TCP-vs-Unix logic ($use_tcp), so
Windows works too.
-

-O: dropped support with --initdb entirely, as you originally suggested.
The partial "-c only" forwarding still broke on quoted values with spaces.
pg_upgrade now rejects -O + --initdb during option parsing, with a test and
a doc note.

*Daniel:*

-

Reworked the manual-initdb docs: the vague "use compatible initdb flags"
is now the specific settings that must match (WAL segment size, checksums,
encoding, locale), with a pointer to pg_controldata.

On in-tree vs. wrapper: these settings have to come from the old cluster's
control data, which pg_upgrade already reads, a wrapper would have to
rediscover the same information, and getting it wrong is exactly the
failure --initdb is meant to prevent.

The patch is pgindent-clean and the full pg_upgrade TAP suite passes
(including the new t/009_initdb_option.pl).

Registered in PG20-2. Branch on GitHub:
https://github.com/LeeBohyun/postgres/tree/pg_upgrade_initdb

Best regards,

Bohyun

On Thu, Jul 16, 2026 at 8:17 PM Greg Sabino Mullane <htamfids(at)gmail(dot)com>
wrote:

> Second review pass.
>
> Cleanly compiles and all tests pass when applied to 637aa273e (Thu Jul 16
> 11:56:01 2026 -0400)
>
> /*
> * resolve_new_bindir()
> *
> * Idempotent helper: if new_cluster.bindir has not been set by the user
> via
> * -B, derive it from the path of the currently executing pg_upgrade
> binary.
> * Called early by create_new_cluster_via_initdb() so that the initdb path
> * is available before verify_directories() runs.
> */
>
> Do we really need the last two lines? This is also called during the
> "normal" path, not just via create_new_cluster_via_initdb, so not sure why
> we are pointing it out here.
>
>
> > prep_status("Inspecting old cluster locale for new cluster creation");
>
> We are doing a lot more than locale though. How about:
>
> prep_status("Examining old cluster settings");
>
>
> > # Use non-default settings that --initdb must carry over to the new
> cluster
> > # (derived from the old cluster's pg_control): data checksums and a
> non-default
> > # WAL segment size. We check below that the new cluster inherits them.
> > my $oldnode = PostgreSQL::Test::Cluster->new('old_node');
> > $oldnode->init(extra => [ '--data-checksums', '--wal-segsize' => '2' ]);
>
> But checkums ARE the default now, so for a non-default setting, we should
> do --no-data-checksums. Would be nice to check locale as well. That gets
> tricky, but maybe --locale=C is nicely portable and pretty non-default?
>
>
> > my $conf = $newnode->data_dir . '/postgresql.conf';
> > open(my $fh, '>>', $conf) or die "could not open $conf: $!";
>
> Easier to use $newnode->append_conf
>
>
> > print $fh "listen_addresses = ''\n";
> > print $fh "unix_socket_directories = '" . $newnode->host . "'\n";
>
> What about Win32? See .e.g $use_tcp in Cluster.pm
>
> > * Forward only "-c name=value" options from -O to initdb. initdb accepts
>
> I still think we just bail on supporting -O. The new code is an
> improvement, but fails if there are legitimate spaces in any of the args:
>
> $ pg_upgrade --initdb -O "-c transaction_isolation='repeatable read'"
>
> Creating new cluster with initdb ignoring
> non-"-c" option(s) passed via -O; only "-c" settings are forwarded to
> initdb for --initdb
> sh: 1: Syntax error: Unterminated quoted string
>
> *failure*
>
>
> Cheers,
> Greg
>