Andres Freund wrote:
> On 2013-12-12 18:24:34 -0300, Alvaro Herrera wrote:
> > + else if (TransactionIdIsValid(update_xid) && !has_lockers)
> > + {
> > + /*
> > + * If there's a single member and it's an update, pass it back alone
> > + * without creating a new Multi. (XXX we could do this when there's a
> > + * single remaining locker, too, but that would complicate the API too
> > + * much; moreover, the case with the single updater is more
> > + * interesting, because those are longer-lived.)
> > + */
> > + Assert(nnewmembers == 1);
> > + *flags |= FRM_RETURN_IS_XID;
> > + if (update_committed)
> > + *flags |= FRM_MARK_COMMITTED;
> > + xid = update_xid;
> > + }
>
> Afaics this will cause HEAP_KEYS_UPDATED to be reset, is that
> problematic? I don't really remember what it's needed for TBH...
So we do reset HEAP_KEYS_UPDATED, and in general that bit seems critical
for several things. So it should be kept when a Xmax is carried over
from the pre-frozen version of the tuple. But while reading through
that, I realize that we should also be keeping HEAP_HOT_UPDATED in that
case. And particularly we should never clear HEAP_ONLY_TUPLE.
So I think heap_execute_freeze_tuple() is wrong, because it's resetting
the whole infomask to zero, and then setting it to only the bits that
heap_prepare_freeze_tuple decided that it needed set. That seems bogus
to me. heap_execute_freeze_tuple() should only clear a certain number
of bits, and then possibly set some of the same bits; but the remaining
flags should remain untouched. So HEAP_KEYS_UPDATED, HEAP_UPDATED and
HEAP_HOT_UPDATED should be untouched by heap_execute_freeze_tuple;
heap_prepare_freeze_tuple needn't worry about querying those bits at
all.
Only when FreezeMultiXactId returns FRM_INVALIDATE_XMAX, and when the
Xmax is not a multi and it gets removed, should those three flags be
removed completely.
HEAP_ONLY_TUPLE should be untouched by the freezing protocol.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services