From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | David Cook <divergentdave(at)gmail(dot)com> |
Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: [PATCH] pgrowlocks: Make mode names consistent with docs |
Date: | 2023-09-26 21:42:13 |
Message-ID: | ZRNQNXe25hNJGbTs@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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.
From | Date | Subject | |
---|---|---|---|
Next Message | Karl O. Pinc | 2023-09-26 21:50:20 | Re: [PGdocs] fix description for handling pf non-ASCII characters |
Previous Message | Peter Eisentraut | 2023-09-26 21:29:23 | Re: Better help output for pgbench -I init_steps |