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 57d47e8601443d592982e6faffb7855391790b95..945684333198f16e535ee0e94b6ae851e71194af 100644 (file)
--- 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;