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

Thanks, Demir, for v5 and the work on the orphan-directory cleanup and

the --check dry run, and thanks Daniel and Greg for the continued

review. Attached is v6. It builds on Demir's v5 and folds in the

remaining points below. Per Daniel's suggestion it is now split:

0001 - docs only: the manual "Initialize the new cluster" clarification

0002 - the --initdb feature (Demir's v5 plus the fixes below)

Responses are all inline.

On Tue, Aug 12, 2026, Daniel Gustafsson wrote:

> + appendPQExpBuffer(&cmd, " -U \"%s\"", os_info.user);

>

> This will inject the username specified in the (untrusted input)

> commandline into the initdb command and execute it. [...] it seems

> like a Booby Tables pattern [...]

>

The initdb command is now assembled with appendShellString()

for every externally-derived value (i.e., the data directory, username,

bindir, and the encoding/locale strings), so shell metacharacters can

no longer break out of an argument.

> Regardless of the rest of the patch, I think we should apply

> something like this to the docs. I propose you extract this portion

> into a separate 0001 in this patchset so that it can be considered

> individually.

>

0001 is docs-only and stands on its own. It also corrects the

earlier implication that encoding and locale must match -- pg_upgrade

copies those from the old cluster, so only the WAL segment size and the

data-checksum setting actually need to match, which 0001 now states

with a pointer to pg_controldata.

On Wed, Aug 13, 2026, Greg Sabino Mullane wrote:

> I see your point. Maybe we don't allow --check with --initdb at all?

>

v5 resolved this as a dry run rather than a hard block. --check

--initdb reports the initdb command it would run and validates the

preconditions (correct new-binary version, empty target directory, old

cluster reachable) without creating the new cluster. v6 keeps that,

and the docs now describe it.

> I'm not clear what pointing this out for --initdb only accomplishes.

> A wrong -B is going to cause problems regardless of the mode.

>

I agree, and I believe this meets Demir's point about orphan directories.

v5 checks the new-binary version up front, so a wrong -B fails before
anything is

written. An atexit handler removes the created directory if a later

check fails, up until the point where real data has been copied. So

there is no need for a -B note specific to --initdb in the docs.

v6 carries both.

> Eliminating what exactly? And why would a vacuum or checkpoint cause

> problems?

>

The old server is started here with the same start_postmaster() that

pg_upgrade uses elsewhere -- including -b, which disables autovacuum --

only to read template0's encoding and locale, then stopped before the

new cluster is created. So autovacuum will not run, and a checkpoint

would be harmless. It is a brief, read-only start using pg_upgrade's

existing mechanism, so it adds no risk beyond the old-server starts

pg_upgrade already performs. The docs now mention this brief start.

On Sun, Aug 17, 2026, Hüseyin Demir wrote:

> a log message in the new --check --initdb dry-run path had a trailing

> newline character it shouldn't have had [...] Fixed [...]

>

Carried into v6.

Beyond the review points, v6 also tightens two things I noticed while

reworking this:

- The empty-directory guard now rejects any non-empty target directory,

not just one already containing a PG_VERSION file. This closes a gap

in the orphan-cleanup path. The cleanup handler removes the whole new

data directory on failure, so it must not run against a directory that

already held the user's files.

- Because --initdb reads template0 by briefly starting the old server, it

now checks in advance that the old server is shut down and fails with the

usual "shut down that postmaster" message, rather than failing later

when pg_resetwal refuses to run against a live server.

Branch on GitHub: github.com/LeeBohyun/postgres/tree/pg_upgrade_initdb

Best regards,

Bohyun

On Mon, Aug 17, 2026 at 9:44 AM Hüseyin Demir <huseyin(dot)d3r(at)gmail(dot)com> wrote:

> > Patch attached. I've renamed the patch file to
> > v5-0001-pg_upgrade-initdb.patch to match our naming convention. No
> > content change, just the filename. Happy to walk through any of this
> > in more detail.
> >
>
> One more thing. While checking this against the CI build I found one
> more small bug: a log message in the new --check --initdb dry-run path
> had a trailing newline character it shouldn't have had, which tripped
> an internal safety check and caused a crash — but only on that
> stricter CI build, not on a normal build. Fixed by removing the extra
> newline. v5 is now confirmed working on both build types.
>