Re: Improve checks for GUC recovery_target_xid

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