Re: Improve checks for GUC recovery_target_xid

On Fri, Feb 20, 2026 at 2:42 PM David Steele <david(at)pgbackrest(dot)org> wrote:
>
> Hackers,
>
> Currently check_recovery_target_xid() converts invalid values to 0. So,
> for example, the following configuration added to postgresql.conf
> followed by a startup:
>
> recovery_target_xid = 'bogus'
> recovery_target_xid = '1.1'
> recovery_target_xid = '0'
>
> ... does not generate an error but recovery does not complete. There are
> many values that can prevent recovery from completing but we should at
> least catch obvious misconfiguration by the user.

+1

> The origin of the problem is that we do not perform a range check in the
> GUC value passed-in for recovery_target_xid. This commit improves the
> situation by using adding end checking to strtou64() and by providing
> stricter range checks. Some test cases are added for the cases of an
> incorrect or a lower-bound timeline value, checking the sanity of the
> reports based on the contents of the server logs.
>
> Also add a comment that truncation of the input value is expected since
> users will generally be using the output from pg_current_xact_id() (or
> similar) to set recovery_target_xid (just as our tests do).
>
> This patch was promised in [1] -- here at last!

Thanks for the patch!

+ GUC_check_errdetail("\"%s\" without epoch must greater than or equal to %u.",

"must greater" shiould be "must be greater"?

"without epoch" seems not necessary to me.

+ /*
+ * This cast will remove the epoch, if any
+ */
+ xid = (TransactionId) strtou64(*newval, &endp, 0);

Would it be better to use strtouint32_strict() instead of strtou64()?
That would allow us to detect invalid XID values larger than 2^32 and
report an error, similar to what pg_resetwal -x does.

Regards,

--
Fujii Masao