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 <bruce(at)momjian(dot)us> https://momjian.us
EDB https://enterprisedb.com

Only you can decide what is important to you.