> On Oct 14, 2026, at 06:02, Heikki Linnakangas <[email protected]> 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/
