diff options
| author | Alvaro Herrera <alvherre@alvh.no-ip.org> | 2013-11-29 16:08:06 -0300 |
|---|---|---|
| committer | Alvaro Herrera <alvherre@alvh.no-ip.org> | 2013-11-29 21:47:21 -0300 |
| commit | 1ce150b7bb14105ddc190c5f1acf2ae1a9b2854a (patch) | |
| tree | dd072a896cb56b945a500c9ce1e5dfff8bd1d954 /src/backend/access/heap/heapam.c | |
| parent | 1df0122daa6510eed4146033379a5055f66f5a8e (diff) | |
| download | postgresql-1ce150b7bb14105ddc190c5f1acf2ae1a9b2854a.tar.gz postgresql-1ce150b7bb14105ddc190c5f1acf2ae1a9b2854a.zip | |
Don't TransactionIdDidAbort in HeapTupleGetUpdateXid
It is dangerous to do so, because some code expects to be able to see what's
the true Xmax even if it is aborted (particularly while traversing HOT
chains). So don't do it, and instead rely on the callers to verify for
abortedness, if necessary.
Several race conditions and bugs fixed in the process. One isolation test
changes the expected output due to these.
This also reverts commit c235a6a589b, which is no longer necessary.
Backpatch to 9.3, where this function was introduced.
Andres Freund
Diffstat (limited to 'src/backend/access/heap/heapam.c')
| -rw-r--r-- | src/backend/access/heap/heapam.c | 21 |
1 files changed, 10 insertions, 11 deletions
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 31496a3063e..31518d58bf9 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -3181,7 +3181,11 @@ l2: if (!HEAP_XMAX_IS_LOCKED_ONLY(oldtup.t_data->t_infomask)) update_xact = HeapTupleGetUpdateXid(oldtup.t_data); - /* there was no UPDATE in the MultiXact; or it aborted. */ + /* + * There was no UPDATE in the MultiXact; or it aborted. No + * TransactionIdIsInProgress() call needed here, since we called + * MultiXactIdWait() above. + */ if (!TransactionIdIsValid(update_xact) || TransactionIdDidAbort(update_xact)) can_continue = true; @@ -5441,6 +5445,9 @@ GetMultiXactIdHintBits(MultiXactId multi, uint16 *new_infomask, * Given a multixact Xmax and corresponding infomask, which does not have the * HEAP_XMAX_LOCK_ONLY bit set, obtain and return the Xid of the updating * transaction. + * + * Caller is expected to check the status of the updating transaction, if + * necessary. */ static TransactionId MultiXactIdGetUpdateXid(TransactionId xmax, uint16 t_infomask) @@ -5465,19 +5472,11 @@ MultiXactIdGetUpdateXid(TransactionId xmax, uint16 t_infomask) for (i = 0; i < nmembers; i++) { /* Ignore lockers */ - if (members[i].status == MultiXactStatusForKeyShare || - members[i].status == MultiXactStatusForShare || - members[i].status == MultiXactStatusForNoKeyUpdate || - members[i].status == MultiXactStatusForUpdate) + if (!ISUPDATE_from_mxstatus(members[i].status)) continue; - /* ignore aborted transactions */ - if (TransactionIdDidAbort(members[i].xid)) - continue; - /* there should be at most one non-aborted updater */ + /* there can be at most one updater */ Assert(update_xact == InvalidTransactionId); - Assert(members[i].status == MultiXactStatusNoKeyUpdate || - members[i].status == MultiXactStatusUpdate); update_xact = members[i].xid; #ifndef USE_ASSERT_CHECKING |