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;