On Sun, Apr 03, 2022 at 09:31:32PM +0200, Michael Banck wrote:
> The patch applies, make is ok, make check is ok, pg_rewind TAP tests
> are ok.
+ appendPQExpBufferStr(postgres_cmd, " --config_file=");
+ appendShellString(postgres_cmd, config_file);
Nit. This is a valid option for the postgres command, but I'd rather
use a -c switch instead, mostly as a matter of self-documentation.
In the tests, the only difference between the modes "archive_cli" and
"archive" is the extra option given to the pg_rewind command, and
that's a simple redirect to what pg_rewind would choose by default
anyway. A more elegant solution would be to have an extra option in
RewindTest::run_pg_rewind(), where any caller of the routine can feed
extra options to the pg_rewind command. Now, in this case, we are
not going to lose any coverage if the existing "archive" mode is
changed so as we move away postgresql.conf from the target data folder
and just use --config-file by default, no? I would make the choice of
simplicity, by giving up on "archive_cli" and using
primary-postgresql.conf.tmp as value for --config-file. There could
be an argument for using --config-file for all the modes, as well.
--
Michael