Re: Fix for consume_xids advancing XIDs incorrectly

Hi Fujii-san,

Thank you very much for the review and for the suggested comment.

You are right: if the transaction already has a top-level XID and the
next XID is already at least the target, consume_xids_until() returns
the next XID without assigning it, so the old comment was not
accurate.

The attached v3 updates the comment of consume_xids_until() as you
suggested. This is the only change from v2:

/*
* Advance XID allocation so that the next XID is at least the target.
* This may consume XIDs beyond the target.
*
* Ensures that the current transaction has a top-level XID.
* Returns the last XID assigned by this call, or the next XID observed
* if no XID was assigned.
*/

The code is unchanged. I applied v3 to current master (8d87367b39e),
built it with assertions, and compared it with the module from master,
each call in a new transaction:

- consume_xids(1), next XID 666: v3 returns 666. With the master
module (next XID 682) it returns 683, so two XIDs were consumed.
- consume_xids(10), next XID 668: v3 returns 677 (10 XIDs). Master
(next XID 685) returns 695 (11 XIDs).
- consume_xids(0): v3 raises "invalid nxids argument: 0"; master returns an XID.
- The case from your mail, consume_xids_until(pg_current_xact_id()) in
a transaction that already has XID 678: it returns 679, the next XID.
Master behaves the same, which matches the new comment.

I did not run the xid_wraparound TAP tests (PG_TEST_EXTRA) for this version.

Thanks,
João

Em qua., 11 de nov. de 2026 às 05:52, Fujii Masao
<masao(dot)fujii(at)gmail(dot)com> escreveu:
>
> On Wed, Nov 11, 2026 at 7:53 AM João Rito <joaovictor(dot)rito(at)gmail(dot)com> wrote:
> > Following Michael's request in January, I picked this one up since the
> > thread has been idle for a while. I rebased the work onto current master
> > and addressed the review comments Sawada-san posted on 2024-10-31.
>
> Thanks for updating the patch! LGTM, except for one comment.
>
> > I kept the "Returns the last XID assigned by this function" comment on
> > consume_xids() and consume_xids_until(), as Fujii-san asked.
>
> If the transaction already has a top-level XID and the next XID is
> at least the target, consume_xids_until() returns that next XID
> without assigning it. So isn't it better to update the comment for
> consume_xids_until(), for example, as follows?
>
> /*
> * Advance XID allocation so that the next XID is at least the target.
> * This may consume XIDs beyond the target.
> *
> * Ensures that the current transaction has a top-level XID.
> * Returns the last XID assigned by this call, or the next XID observed
> * if no XID was assigned.
> */
>
> Regards,
>
> --
> Fujii Masao