Re: doubt about FullTransactionIdAdvance()

Hi, Andres

On Oct 24, 2022, 01:16 +0800, Andres Freund <andres(at)anarazel(dot)de>, wrote:
> Hi,
>
> On 2022-10-22 11:32:47 +0800, Zhang Mingli wrote:
> > Hi, hackers
> >
> > I don't quite understand FullTransactionIdAdvance(), correct me if I’m wrong.
> >
> >
> > /*
> >  * Advance a FullTransactionId variable, stepping over xids that would appear
> >  * to be special only when viewed as 32bit XIDs.
> >  */
> > static inline void
> > FullTransactionIdAdvance(FullTransactionId *dest)
> > {
> > dest->value++;
> >
> > /* see FullTransactionIdAdvance() */
> > if (FullTransactionIdPrecedes(*dest, FirstNormalFullTransactionId))
> >  return;
> >
> > while (XidFromFullTransactionId(*dest) < FirstNormalTransactionId)
> >  dest->value++;
> > }
> >
> > #define XidFromFullTransactionId(x) ((x).value)
> > #define FullTransactionIdPrecedes(a, b) ((a).value < (b).value)
> >
> > Can the codes reach line: while (XidFromFullTransactionId(*dest) < FirstNormalTransactionId)?
> > As we will return if (FullTransactionIdPrecedes(*dest, FirstNormalFullTransactionId)), and the two judgements seem equal.
> > Another confusion is the comments: /* see FullTransactionIdAdvance() */, is its own  itself.
> > Could anyone explain this? Thanks in advance.
>
> FullTransactionId is 64bit. An "old school" xid is 32bit. The first branch is
> to protect against the special fxids that are actually below
> FirstNormalFullTransactionId:
>
> if (FullTransactionIdPrecedes(*dest, FirstNormalFullTransactionId))
> return;
>
> The second branch is to protect against 64bit xids that would yield a 32bit
> xid below FirstNormalTransactionId after truncating to 32bit:
>
> while (XidFromFullTransactionId(*dest) < FirstNormalTransactionId)
> dest->value++;
>
> E.g. we don't want to modify the 64bit xid 0 (meaning InvalidTransactionId) as
> it has special meaning. But we have to make sure that e.g. the 64bit xid
> 0x100000000 won't exist, as it'd also result in InvalidTransactionId if
> truncated to 32bit.
>
>
> However, it looks like this comment:
> /* see FullTransactionIdAdvance() */
> if (FullTransactionIdPrecedes(*dest, FirstNormalFullTransactionId))
> return;
>
> is bogus, and it's my fault. Looks like it's intending to reference
> FullTransactionIdRetreat().
>
> Greetings,
>
> Andres Freund
Now I get it, thanks for your explanation.

Regards,
Zhang Mingli