Hi, Thanks, looks saner than my version ;)
On 2013-11-29 01:00:35 -0300, Alvaro Herrera wrote: > ! /* > ! * FIXME this calls TransactionIdDidAbort() > internally, > ! * falsifying the claim in the comment at the > top ... > ! */ > ! update_xid = HeapTupleGetUpdateXid(tuple); Yea. I think we should remove that behaviour from HeapTupleGetUpdateXid() - we don't need to rely on it here. > ! /* > ! * XXX we rely here on HeapTupleGetUpdateXid > returning > ! * Invalid for aborted updates. > ! */ > ! if (!TransactionIdIsValid(update_xid)) > ! freeze_xmax = true; I've just added this case because HeapTupleGetUpdateXid() currently can return InvalidTransactionId - I'd be perfectly fine to just place the aborted xid in there. > ! * FIXME -- what if it's a committed update > which has recent > ! * new locker transaction? The tuple wouldn't > have been > ! * removed in that case > (HeapTupleSatisfiesVacuum returns > ! * RECENTLY_DEAD). > ! */ Afaics we should be protected against that by virtue of GetOldestMultiXactId(). > *************** > *** 5645,5668 **** heap_tuple_needs_freeze(HeapTupleHeader tuple, > TransactionId cutoff_xid, > ! /* we don't care about lockers */ > ! if (members[i].status != > MultiXactStatusNoKeyUpdate || > ! members[i].status == > MultiXactStatusUpdate) > ! continue; Dangerous typo alert. Should also be replaced by ISUPDATE_from_mxstatus(). Alternatively, if we decide to make HeapTupleGetUpdateXid() not make that DidAbort() check, we can simply get rid of doing the loop ourselves alltogethers. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
