diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 7217e96d6a..a04763207f 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -190,9 +190,6 @@ static const int MultiXactStatusLock[MaxMultiXactStatus + 1] = /* Get the LockTupleMode for a given MultiXactStatus */ #define TUPLOCK_from_mxstatus(status) \ (MultiXactStatusLock[(status)]) -/* Get the is_update bit for a given MultiXactStatus */ -#define ISUPDATE_from_mxstatus(status) \ - ((status) > MultiXactStatusForUpdate) /* ---------------------------------------------------------------- * heap support routines @@ -2588,6 +2585,27 @@ compute_infobits(uint16 infomask, uint16 infomask2) XLHL_KEYS_UPDATED : 0); } +/* + * Given two versions of the same t_infomask for a tuple, compare them and + * return whether the relevant status for a tuple Xmax has changed. This is + * used after a buffer lock has been released and reacquired: we want to ensure + * that the tuple state continues to be the same it was when we previously + * examined it. + * + * Note the Xmax field itself must be compared separately. + */ +static inline bool +xmax_infomask_changed(uint16 new_infomask, uint16 old_infomask) +{ + const uint16 interesting = + HEAP_XMAX_IS_MULTI | HEAP_XMAX_LOCK_ONLY | HEAP_LOCK_MASK; + + if ((new_infomask & interesting) != (old_infomask & interesting)) + return true; + + return false; +} + /* * heap_delete - delete a tuple * @@ -2725,7 +2743,7 @@ l1: * update this tuple before we get to this point. Check for xmax * change, and start over if so. */ - if (!(tp.t_data->t_infomask & HEAP_XMAX_IS_MULTI) || + if (xmax_infomask_changed(tp.t_data->t_infomask, infomask) || !TransactionIdEquals(HeapTupleHeaderGetRawXmax(tp.t_data), xwait)) goto l1; @@ -2751,7 +2769,7 @@ l1: * other xact could update this tuple before we get to this point. * Check for xmax change, and start over if so. */ - if ((tp.t_data->t_infomask & HEAP_XMAX_IS_MULTI) || + if (xmax_infomask_changed(tp.t_data->t_infomask, infomask) || !TransactionIdEquals(HeapTupleHeaderGetRawXmax(tp.t_data), xwait)) goto l1; @@ -3278,7 +3296,7 @@ l2: * update this tuple before we get to this point. Check for xmax * change, and start over if so. */ - if (!(oldtup.t_data->t_infomask & HEAP_XMAX_IS_MULTI) || + if (xmax_infomask_changed(oldtup.t_data->t_infomask, infomask) || !TransactionIdEquals(HeapTupleHeaderGetRawXmax(oldtup.t_data), xwait)) goto l2; @@ -3332,7 +3350,7 @@ l2: * recheck the locker; if someone else changed the tuple while * we weren't looking, start over. */ - if ((oldtup.t_data->t_infomask & HEAP_XMAX_IS_MULTI) || + if (xmax_infomask_changed(oldtup.t_data->t_infomask, infomask) || !TransactionIdEquals( HeapTupleHeaderGetRawXmax(oldtup.t_data), xwait)) @@ -3353,7 +3371,7 @@ l2: * some other xact could update this tuple before we get to * this point. Check for xmax change, and start over if so. */ - if ((oldtup.t_data->t_infomask & HEAP_XMAX_IS_MULTI) || + if (xmax_infomask_changed(oldtup.t_data->t_infomask, infomask) || !TransactionIdEquals( HeapTupleHeaderGetRawXmax(oldtup.t_data), xwait)) @@ -4357,7 +4375,7 @@ l3: * over. */ LockBuffer(*buffer, BUFFER_LOCK_EXCLUSIVE); - if (!(tuple->t_data->t_infomask & HEAP_XMAX_IS_MULTI) || + if (xmax_infomask_changed(tuple->t_data->t_infomask, infomask) || !TransactionIdEquals(HeapTupleHeaderGetRawXmax(tuple->t_data), xwait)) { @@ -4376,7 +4394,7 @@ l3: LockBuffer(*buffer, BUFFER_LOCK_EXCLUSIVE); /* if the xmax changed in the meantime, start over */ - if ((tuple->t_data->t_infomask & HEAP_XMAX_IS_MULTI) || + if (xmax_infomask_changed(tuple->t_data->t_infomask, infomask) || !TransactionIdEquals( HeapTupleHeaderGetRawXmax(tuple->t_data), xwait)) @@ -4444,7 +4462,7 @@ l3: * could update this tuple before we get to this point. Check * for xmax change, and start over if so. */ - if (!(tuple->t_data->t_infomask & HEAP_XMAX_IS_MULTI) || + if (xmax_infomask_changed(tuple->t_data->t_infomask, infomask) || !TransactionIdEquals( HeapTupleHeaderGetRawXmax(tuple->t_data), xwait)) @@ -4500,7 +4518,7 @@ l3: * some other xact could update this tuple before we get to * this point. Check for xmax change, and start over if so. */ - if ((tuple->t_data->t_infomask & HEAP_XMAX_IS_MULTI) || + if (xmax_infomask_changed(tuple->t_data->t_infomask, infomask) || !TransactionIdEquals( HeapTupleHeaderGetRawXmax(tuple->t_data), xwait)) diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c index d4ad6787a5..459f59cb4e 100644 --- a/src/backend/access/transam/multixact.c +++ b/src/backend/access/transam/multixact.c @@ -457,7 +457,7 @@ MultiXactIdExpand(MultiXactId multi, TransactionId xid, MultiXactStatus status) for (i = 0, j = 0; i < nmembers; i++) { if (TransactionIdIsInProgress(members[i].xid) || - ((members[i].status > MultiXactStatusForUpdate) && + (ISUPDATE_from_mxstatus(members[i].status) && TransactionIdDidCommit(members[i].xid))) { newMembers[j].xid = members[i].xid; @@ -713,6 +713,22 @@ MultiXactIdCreateFromMembers(int nmembers, MultiXactMember *members) return multi; } + /* Verify that there is a single update Xid among the given members. */ + { + int i; + bool has_update = false; + + for (i = 0; i < nmembers; i++) + { + if (ISUPDATE_from_mxstatus(members[i].status)) + { + if (has_update) + elog(ERROR, "new multixact has more than one updating member"); + has_update = true; + } + } + } + /* * Assign the MXID and offsets range to use, and make sure there is space * in the OFFSETs and MEMBERs files. NB: this routine does diff --git a/src/include/access/multixact.h b/src/include/access/multixact.h index 9729f27508..1f048e8ed5 100644 --- a/src/include/access/multixact.h +++ b/src/include/access/multixact.h @@ -48,6 +48,10 @@ typedef enum #define MaxMultiXactStatus MultiXactStatusUpdate +/* does a status value correspond to a tuple update? */ +#define ISUPDATE_from_mxstatus(status) \ + ((status) > MultiXactStatusForUpdate) + typedef struct MultiXactMember {