Re: pg_upgrade fails with a config-only datadir and no --new-bindir

> On Oct 14, 2026, at 06:02, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>
> While reviewing Bohyun's pg_upgrade --initdb patch, I noticed this small existing bug:
>
> ----
> $ mkdir data-new-cfg
> $ touch data-new-cfg/postgresql.conf
> $ ./bin/pg_upgrade --old-bindir=./bin -D ./data-new-cfg -d ./data
> Finding the real data directory for the target cluster sh: 1: (null)/postgres: not found
>
> could not get data directory using "(null)/postgres" -D "data-new-cfg" -C data_directory: No such file or directory
> Failure, exiting
> ----
>
> In this scenario, we have a combination of no --new-bindir option, so that it is to be derived from the path of pg_upgrade itself, and a --new-datadir (-D) option that points to a configuration-only directory.
>
> That command was doomed to fail as the new datadir was empty except for the config file, but a "(null)" is a sign of passing a NULL to sprintf(), and we shouldn't be doing that.
>
> I think the ordering of checks in main() and setup() is the culprit here. main() calls adjust_data_dir() first, which does this:
>
>> snprintf(cmd, sizeof(cmd), "\"%s/postgres\" -D \"%s\" -C data_directory",
>> cluster->bindir, cluster->pgconfig);
>
> to get the path to the data directory from the config. But cluster->bindir isn't initialized yet.
>
> Attached patch reorders the checks so that the binary directory is resolved and checked before adjust_data_dir(). I think this bug is present on all supported versions, but I'm not sure this is worth backpatching. Although it doesn't seem very risky to backpatch either.
>
> - Heikki
> <0001-Fix-order-of-bin-and-data-dir-checks-in-pg_upgrade.patch>

The fix looks correct to me. It makes sense to move check_bin_dirs() before adjust_data_dir(&new_cluster), since the latter may need to execute the postgres binary.

However, the updated test in 002_pg_upgrade.pl doesn’t seem to cover the bug being fixed. I tried to ran the updated test against master, and it still passed. Is that intentional?

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/