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

On Fri, Feb 14, 2020 at 6:31 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> On Wed, Jan 29, 2020 at 12:43 AM Fujii Masao <masao.fujii@oss.nttdata.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