MIME-Version: 1.0
References: <f14463ab-990b-4ae9-a177-998d2677aae0@pgbackrest.org>
In-Reply-To: <f14463ab-990b-4ae9-a177-998d2677aae0@pgbackrest.org>
From: Fujii Masao <masao.fujii@gmail.com>
Date: Fri, 27 Feb 2026 10:12:43 +0900
Message-ID: <CAHGQGwEnakTrosMp3Y=Trya4MTC-h8Lqx3FDU1mOALRQ0rz59Q@mail.gmail.com>
Subject: Re: Improve checks for GUC recovery_target_xid
To: David Steele <david@pgbackrest.org>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAHGQGwEnakTrosMp3Y%3DTrya4MTC-h8Lqx3FDU1mOALRQ0rz59Q%40mail.gmail.com>
Precedence: bulk

On Fri, Feb 20, 2026 at 2:42=E2=80=AFPM David Steele <david@pgbackrest.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 =3D 'bogus'
> recovery_target_xid =3D '1.1'
> recovery_target_xid =3D '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 =3D (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,

--=20
Fujii Masao