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
signature.asc
Description: PGP signature