On 3/4/26 22:41, Fujii Masao wrote:
>
> Regarding the regression test, if the purpose is to verify the GUC hook
> for recovery_target_xid, it might be simpler to test whether
> "ALTER SYSTEM SET recovery_target_xid TO ..." succeeds or fails as expected
> as follows, rather than starting the server with that setting. That said,
> since recovery_target_timeline is already tested in a similar way, I understand
> why you followed the same pattern here. So I'm ok with the current approach.
I wrote the tests for recovery_target_timeline but I was not too
satisfied with them because starting Postgres is fairly expensive.
>
> my ($result, $stdout, $stderr) = $node_primary->psql('postgres',
> "ALTER SYSTEM SET recovery_target_xid TO 'bogus'");
> like(
> $stderr,
> qr/is not a valid number/,
> "invalid recovery_target_xid (bogus value)");
>
> If we think it's better to use ALTER SYSTEM SET for testing invalid
> recovery_target_xxx settings to keep the regression tests simpler,
> we can revisit this later and address it in a separate patch.
I've updated the patch to do it this way. Not only is it faster but you
get a better message when the expected value is incorrect.
I can update the tests for recovery_target_timeline in a separate patch.
Regards,
-David
