Re: [PATCH] pgrowlocks: Make mode names consistent with docs

On Thu, Sep  7, 2023 at 12:58:29PM -0400, Bruce Momjian wrote:
> You are right something is wrong.  However, I looked at your patch and I
> am thinking we need to go the other way and add "For" in the upper
> block, rather than removing it in the lower one.  I have two reasons. 
> Looking at the code block:
> 
>     case MultiXactStatusUpdate:
> 	snprintf(buf, NCHARS, "Update");
> 	break;
>     case MultiXactStatusNoKeyUpdate:
> 	snprintf(buf, NCHARS, "No Key Update");
> 	break;
>     case MultiXactStatusForUpdate:
> 	snprintf(buf, NCHARS, "For Update");
> 	break;
>     case MultiXactStatusForNoKeyUpdate:
> 	snprintf(buf, NCHARS, "For No Key Update");
> 	break;
>     case MultiXactStatusForShare:
> 	snprintf(buf, NCHARS, "Share");
> 	break;
>     case MultiXactStatusForKeyShare:
> 	snprintf(buf, NCHARS, "Key Share");
> 	break;
> 
> You will notice there are "For" and non-"For" versions of "Update" and
> "No Key Update".  Notice that "For" appears in the macro names for the
> "For" macro versions of update, but "For" does not appear in the "Share"
> and "Key Share" versions, though the macro has "For".
> 
> Second, notice that the "For" and non-"For" match in the block below
> that, which suggests it is correct, and the non-"For" block is later:
> 
>     values[Atnum_modes] = palloc(NCHARS);
>     if (infomask & HEAP_XMAX_LOCK_ONLY)
>     {
> 	if (HEAP_XMAX_IS_SHR_LOCKED(infomask))
> 	snprintf(values[Atnum_modes], NCHARS, "{For Share}");
> 	else if (HEAP_XMAX_IS_KEYSHR_LOCKED(infomask))
> 	snprintf(values[Atnum_modes], NCHARS, "{For Key Share}");
> 	else if (HEAP_XMAX_IS_EXCL_LOCKED(infomask))
> 	{
> 	    if (tuple->t_data->t_infomask2 & HEAP_KEYS_UPDATED)
> 		snprintf(values[Atnum_modes], NCHARS, "{For Update}");
> 	    else
> 		snprintf(values[Atnum_modes], NCHARS, "{For No Key Update}");
> 	}
> 	else
> 	/* neither keyshare nor exclusive bit it set */
> 	snprintf(values[Atnum_modes], NCHARS,
> 		 "{transient upgrade status}");
>     }
>     else
>     {
> 	if (tuple->t_data->t_infomask2 & HEAP_KEYS_UPDATED)
> 	snprintf(values[Atnum_modes], NCHARS, "{Update}");
> 	else
> 	snprintf(values[Atnum_modes], NCHARS, "{No Key Update}");
>     }
> 
> I therefore suggest this attached patch, which should be marked as an
> incompatibility in PG 17.

Patch applied to master.

-- 
  Bruce Momjian          https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.