Thread: doubt about FullTransactionIdAdvance()

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