Re: [COMMITTERS] pgsql: Fix traversal of half-frozen update chains

On Fri, Oct 6, 2017 at 8:29 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> Fix traversal of half-frozen update chains

I have a question about this (this is taken from the master branch):

> bool
> HeapTupleUpdateXmaxMatchesXmin(TransactionId xmax, HeapTupleHeader htup)
> {
>     TransactionId   xmin = HeapTupleHeaderGetXmin(htup);
>
>     /*
>      * If the xmax of the old tuple is identical to the xmin of the new one,
>      * it's a match.
>      */
>     if (TransactionIdEquals(xmax, xmin))
>         return true;
>
>     /*
>      * If the Xmin that was in effect prior to a freeze matches the Xmax,
>      * it's good too.
>      */
>     if (HeapTupleHeaderXminFrozen(htup) &&
>         TransactionIdEquals(HeapTupleHeaderGetRawXmin(htup), xmax))
>         return true;
>
>     /*
>      * When a tuple is frozen, the original Xmin is lost, but we know it's a
>      * committed transaction.  So unless the Xmax is InvalidXid, we don't know
>      * for certain that there is a match, but there may be one; and we must
>      * return true so that a HOT chain that is half-frozen can be walked
>      * correctly.
>      *
>      * We no longer freeze tuples this way, but we must keep this in order to
>      * interpret pre-pg_upgrade pages correctly.
>      */
>     if (TransactionIdEquals(xmin, FrozenTransactionId) &&
>         TransactionIdIsValid(xmax))
>         return true;
>
>     return false;
> }

Wouldn't this last "if" test, to cover the pg_upgrade case, be better
targeted by comparing *raw* xmin to FrozenTransactionId? You're using
the potentially distinct xmin value returned by
HeapTupleHeaderGetXmin() for the test here. I think we should be
directly targeting tuples frozen on or before 9.4 (prior to
pg_upgrade) instead.

Have I missed something?

-- 
Peter Geoghegan


-- 
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers