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
