Re: Should we add xid_current() or a int8->xid cast?

On Fri, Feb 14, 2020 at 6:31 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
> On Wed, Jan 29, 2020 at 12:43 AM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
> > /*
> > - * do a TransactionId -> txid conversion for an XID near the given epoch
> > + * Do a TransactionId -> fxid conversion for an XID that is known to precede
> > + * the given 'next_fxid'.
> > */
> > -static txid
> > -convert_xid(TransactionId xid, const TxidEpoch *state)
> > +static FullTransactionId
> > +convert_xid(TransactionId xid, FullTransactionId next_fxid)
> >
> > As the comment suggests, this function assumes that "xid" must
> > precede "next_fxid". But there is no check for the assumption.
> > Isn't it better to add, e.g., an assertion checking that?
> > Or convert_xid() should handle the case where "xid" follows
> > "next_fxid" like the orignal convert_xid() does. That is, don't
> > apply the following change.
> >
> > - if (xid > state->last_xid &&
> > - TransactionIdPrecedes(xid, state->last_xid))
> > + if (xid > next_xid)
> > epoch--;
> > - else if (xid < state->last_xid &&
> > - TransactionIdFollows(xid, state->last_xid))
> > - epoch++;

I don't think it is reachable. I have renamed the function to
widen_snapshot_xid() and rewritten the comments to explain the logic.

The other changes in this version:

* updated OIDs to avoid collisions
* added btequalimage to btree/xid8_ops