From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Thu, 31 Jan 2013 22:12:35 +0000 (-0300)
Subject: Restrict infomask bits to set on multixacts
X-Git-Tag: REL9_3_BETA1~396
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b78647a0e6f7b110273e98601f26d3d1db0ad931;p=postgresql

Restrict infomask bits to set on multixacts

We must only set the bit(s) for the strongest lock held in the tuple;
otherwise, a multixact containing members with exclusive lock and
key-share lock will behave as though only a share lock is held.

This bug was introduced in commit 0ac5ad5134, somewhere along
development, when we allowed a singleton FOR SHARE lock to be
implemented without a MultiXact by using a multi-bit pattern.
I overlooked that GetMultiXactIdHintBits() needed to be tweaked as well.
Previously, we could have the bits for FOR KEY SHARE and FOR UPDATE
simultaneously set and it wouldn't cause a problem.

Per report from digoal@126.com
---

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 57d47e8601..9456843331 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -3269,7 +3269,13 @@ l2:
 							  &xmax_old_tuple, &infomask_old_tuple,
 							  &infomask2_old_tuple);
 
-	/* And also prepare an Xmax value for the new copy of the tuple */
+	/*
+	 * And also prepare an Xmax value for the new copy of the tuple.  If there
+	 * was no xmax previously, or there was one but all lockers are now gone,
+	 * then use InvalidXid; otherwise, get the xmax from the old tuple.  (In
+	 * rare cases that might also be InvalidXid and yet not have the
+	 * HEAP_XMAX_INVALID bit set; that's fine.)
+	 */
 	if ((oldtup.t_data->t_infomask & HEAP_XMAX_INVALID) ||
 		(checked_lockers && !locker_remains))
 		xmax_new_tuple = InvalidTransactionId;
@@ -3283,6 +3289,12 @@ l2:
 	}
 	else
 	{
+		/*
+		 * If we found a valid Xmax for the new tuple, then the infomask bits
+		 * to use on the new tuple depend on what was there on the old one.
+		 * Note that since we're doing an update, the only possibility is that
+		 * the lockers had FOR KEY SHARE lock.
+		 */
 		if (oldtup.t_data->t_infomask & HEAP_XMAX_IS_MULTI)
 		{
 			GetMultiXactIdHintBits(xmax_new_tuple, &infomask_new_tuple,
@@ -5161,6 +5173,7 @@ GetMultiXactIdHintBits(MultiXactId multi, uint16 *new_infomask,
 	uint16	bits = HEAP_XMAX_IS_MULTI;
 	uint16	bits2 = 0;
 	bool	has_update = false;
+	LockTupleMode	strongest = LockTupleKeyShare;
 
 	/*
 	 * We only use this in multis we just created, so they cannot be values
@@ -5170,32 +5183,47 @@ GetMultiXactIdHintBits(MultiXactId multi, uint16 *new_infomask,
 
 	for (i = 0; i < nmembers; i++)
 	{
+		LockTupleMode	mode;
+
+		/*
+		 * Remember the strongest lock mode held by any member of the
+		 * multixact.
+		 */
+		mode = TUPLOCK_from_mxstatus(members[i].status);
+		if (mode > strongest)
+			strongest = mode;
+
+		/* See what other bits we need */
 		switch (members[i].status)
 		{
 			case MultiXactStatusForKeyShare:
-				bits |= HEAP_XMAX_KEYSHR_LOCK;
-				break;
 			case MultiXactStatusForShare:
-				bits |= HEAP_XMAX_SHR_LOCK;
-				break;
 			case MultiXactStatusForNoKeyUpdate:
-				bits |= HEAP_XMAX_EXCL_LOCK;
 				break;
+
 			case MultiXactStatusForUpdate:
-				bits |= HEAP_XMAX_EXCL_LOCK;
 				bits2 |= HEAP_KEYS_UPDATED;
 				break;
+
 			case MultiXactStatusNoKeyUpdate:
-				bits |= HEAP_XMAX_EXCL_LOCK;
 				has_update = true;
 				break;
+
 			case MultiXactStatusUpdate:
-				bits |= HEAP_XMAX_EXCL_LOCK;
 				bits2 |= HEAP_KEYS_UPDATED;
 				has_update = true;
 				break;
 		}
 	}
+
+	if (strongest == LockTupleExclusive ||
+		strongest == LockTupleNoKeyExclusive)
+		bits |= HEAP_XMAX_EXCL_LOCK;
+	else if (strongest == LockTupleShare)
+		bits |= HEAP_XMAX_SHR_LOCK;
+	else if (strongest == LockTupleKeyShare)
+		bits |= HEAP_XMAX_KEYSHR_LOCK;
+
 	if (!has_update)
 		bits |= HEAP_XMAX_LOCK_ONLY;