Re: Fix incorrect buffer lock description in pg_visibility comment
Hi,
On Tue, Dec 23, 2025 at 6:18 PM Chao Li <li.evan.chao@gmail.com> wrote:
Hi Hackers,
While reviewing the patch [1], though I couldn’t raise a comment for it, I noticed a comment error in the file pg_visibility.c, where a buffer lock is held in shared mode but the code comment mentioned exclusive mode. I am filing a small patch to correct the comment.
I only changed "exclusively" to "shared", the format changed was done by pgindent.
Yeah, it seems like a typo since the first commit of pg_visiblity,
e472ce9624e0. I think we can backpatch it to all supported versions.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Import Notes
Reply to msg id not found: CAEoWx2kkhxgfp=kinPMetnwHaa0JjR6YBkO_0gg0oiy6mu7Zjw@mail.gmail.comReference msg id not found: CAEoWx2kkhxgfp=kinPMetnwHaa0JjR6YBkO_0gg0oiy6mu7Zjw@mail.gmail.com
On Mon, Dec 29, 2025 at 10:33 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
Hi,
On Tue, Dec 23, 2025 at 6:18 PM Chao Li <li.evan.chao@gmail.com> wrote:
Hi Hackers,
While reviewing the patch [1], though I couldn’t raise a comment for it, I noticed a comment error in the file pg_visibility.c, where a buffer lock is held in shared mode but the code comment mentioned exclusive mode. I am filing a small patch to correct the comment.
I only changed "exclusively" to "shared", the format changed was done by pgindent.
Yeah, it seems like a typo since the first commit of pg_visiblity,
e472ce9624e0. I think we can backpatch it to all supported versions.
I find that "shared locked" sounds unnatural to me. How about
rephrasing to "... we're holding the buffer locked in shared mode"?
I've attached the patch.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Attachments:
0001-pg_visibility-Fix-incorrect-buffer-lock-description.patchapplication/octet-stream; name=0001-pg_visibility-Fix-incorrect-buffer-lock-description.patchDownload
From f2f2780d08c201d53fa7c3e7a6049a668a2ee588 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Wed, 24 Dec 2025 10:07:22 +0800
Subject: [PATCH] pg_visibility: Fix incorrect buffer lock description.
Although the comment in collect_corrupt_items() stated that the buffer
is locked in exclusive mode, it is actually locked in shared mode.
Author: Chao Li <lic@highgo.com>
Discussion: https://postgr.es/m/CAEoWx2kkhxgfp=kinPMetnwHaa0JjR6YBkO_0gg0oiy6mu7Zjw@mail.gmail.com
---
contrib/pg_visibility/pg_visibility.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c
index c019202f150..9bc3a784bf7 100644
--- a/contrib/pg_visibility/pg_visibility.c
+++ b/contrib/pg_visibility/pg_visibility.c
@@ -818,7 +818,7 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
*
* From a concurrency point of view, it sort of sucks to
* retake ProcArrayLock here while we're holding the buffer
- * exclusively locked, but it should be safe against
+ * locked in shared mode, but it should be safe against
* deadlocks, because surely
* GetStrictOldestNonRemovableTransactionId() should never
* take a buffer lock. And this shouldn't happen often, so
--
2.47.3
On Jan 6, 2026, at 02:41, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
I find that "shared locked" sounds unnatural to me. How about
rephrasing to "... we're holding the buffer locked in shared mode”?
Hi Masahiko-san,
Thanks for taking care of this patch. Yeah, I agree “shared locked” is not good, at least it should be “shared-ly locked”. So, Your version, “locked in shared mode” is better.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
On Mon, Jan 5, 2026 at 3:45 PM Chao Li <li.evan.chao@gmail.com> wrote:
On Jan 6, 2026, at 02:41, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
I find that "shared locked" sounds unnatural to me. How about
rephrasing to "... we're holding the buffer locked in shared mode”?Hi Masahiko-san,
Thanks for taking care of this patch. Yeah, I agree “shared locked” is not good, at least it should be “shared-ly locked”. So, Your version, “locked in shared mode” is better.
Thank you for reviewing the patch! Pushed.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com