| From: | David Steele <david(at)pgbackrest(dot)org> |
|---|---|
| To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Hüseyin Demir <huseyin(dot)d3r(at)gmail(dot)com> |
| Cc: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Improve checks for GUC recovery_target_xid |
| Date: | 2026-03-04 05:11:48 |
| Message-ID: | 9e8b7ee9-4a16-477a-baa5-0cdf37a04798@pgbackrest.org |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 2/27/26 08:12, Fujii Masao wrote:
> On Fri, Feb 20, 2026 at 2:42 PM David Steele <david(at)pgbackrest(dot)org> wrote:
>
> + GUC_check_errdetail("\"%s\" without epoch must greater than or equal to %u.",
>
> "must greater" shiould be "must be greater"?
Fixed in v2 attached to the prior email.
> "without epoch" seems not necessary to me.
I guess that depends on whether or not we error if the epoch is present,
see below.
> + /*
> + * 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.
This was my first instinct, but it causes our integration tests to fail
because pg_current_xact_id() returns the xid with epoch. You can fix
this by casting pg_current_xact_id()::xid but this seems like a pretty
big change in usage. I'm OK with it but we'd definitely need to update
the documentation to match.
What do you think?
Regards
-David
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-03-04 05:31:29 | Re: BUG: Former primary node might stuck when started as a standby |
| Previous Message | David Steele | 2026-03-04 05:07:22 | Re: Improve checks for GUC recovery_target_xid |