Mark ItemPointer parameters as const in tuple/table lock functions
Hi Hackers,
This is a pure refactor patch.
The functions LockTuple, ConditionalLockTuple, UnlockTuple, and
XactLockTableWait take an ItemPointer parameter named 'tid'. Since these
functions do not modify the tuple identifier, the parameter should be
declared as const to better convey intent and allow the compiler to enforce
immutability.
With this patch, build still passes, and "make check" also passes.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
v1-0001-Mark-ItemPointer-parameters-as-const-in-tuple-tab.patchapplication/octet-stream; name=v1-0001-Mark-ItemPointer-parameters-as-const-in-tuple-tab.patchDownload
From 9079a4ec75a29ec9b2e725a4b18b6fb3a0a29d94 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Wed, 27 Aug 2025 16:48:54 +0800
Subject: [PATCH v1] Mark ItemPointer parameters as const in tuple/table lock
functions
The functions LockTuple, ConditionalLockTuple, UnlockTuple, and
XactLockTableWait take an ItemPointer parameter named 'tid'. Since
these functions do not modify the tuple identifier, the parameter
should be declared as const to better convey intent and allow the
compiler to enforce immutability.
Changes:
- Add 'const' qualifier to ItemPointer tid parameter in the above functions.
- Update function declarations and definitions accordingly.
This improves code correctness and readability without affecting behavior.
Author: Chao Li <lic@highgo.com>
---
src/backend/storage/lmgr/lmgr.c | 8 ++++----
src/include/storage/lmgr.h | 8 ++++----
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c
index 3f6bf70bd3c..297472aed08 100644
--- a/src/backend/storage/lmgr/lmgr.c
+++ b/src/backend/storage/lmgr/lmgr.c
@@ -559,7 +559,7 @@ UnlockPage(Relation relation, BlockNumber blkno, LOCKMODE lockmode)
* tuple. See heap_lock_tuple before using this!
*/
void
-LockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode)
+LockTuple(Relation relation, const ItemPointer tid, LOCKMODE lockmode)
{
LOCKTAG tag;
@@ -579,7 +579,7 @@ LockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode)
* Returns true iff the lock was acquired.
*/
bool
-ConditionalLockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode,
+ConditionalLockTuple(Relation relation, const ItemPointer tid, LOCKMODE lockmode,
bool logLockFailure)
{
LOCKTAG tag;
@@ -598,7 +598,7 @@ ConditionalLockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode,
* UnlockTuple
*/
void
-UnlockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode)
+UnlockTuple(Relation relation, const ItemPointer tid, LOCKMODE lockmode)
{
LOCKTAG tag;
@@ -660,7 +660,7 @@ XactLockTableDelete(TransactionId xid)
* and if so wait for its parent.
*/
void
-XactLockTableWait(TransactionId xid, Relation rel, ItemPointer ctid,
+XactLockTableWait(TransactionId xid, Relation rel, const ItemPointer ctid,
XLTW_Oper oper)
{
LOCKTAG tag;
diff --git a/src/include/storage/lmgr.h b/src/include/storage/lmgr.h
index 58eee4e0d54..ed9a9f32488 100644
--- a/src/include/storage/lmgr.h
+++ b/src/include/storage/lmgr.h
@@ -71,16 +71,16 @@ extern bool ConditionalLockPage(Relation relation, BlockNumber blkno, LOCKMODE l
extern void UnlockPage(Relation relation, BlockNumber blkno, LOCKMODE lockmode);
/* Lock a tuple (see heap_lock_tuple before assuming you understand this) */
-extern void LockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode);
-extern bool ConditionalLockTuple(Relation relation, ItemPointer tid,
+extern void LockTuple(Relation relation, const ItemPointer tid, LOCKMODE lockmode);
+extern bool ConditionalLockTuple(Relation relation, const ItemPointer tid,
LOCKMODE lockmode, bool logLockFailure);
-extern void UnlockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode);
+extern void UnlockTuple(Relation relation, const ItemPointer tid, LOCKMODE lockmode);
/* Lock an XID (used to wait for a transaction to finish) */
extern void XactLockTableInsert(TransactionId xid);
extern void XactLockTableDelete(TransactionId xid);
extern void XactLockTableWait(TransactionId xid, Relation rel,
- ItemPointer ctid, XLTW_Oper oper);
+ const ItemPointer ctid, XLTW_Oper oper);
extern bool ConditionalXactLockTableWait(TransactionId xid,
bool logLockFailure);
--
2.39.5 (Apple Git-154)
On 27.08.25 10:57, Chao Li wrote:
This is a pure refactor patch.
The functions LockTuple, ConditionalLockTuple, UnlockTuple, and
XactLockTableWait take an ItemPointer parameter named 'tid'. Since these
functions do not modify the tuple identifier, the parameter should be
declared as const to better convey intent and allow the compiler to
enforce immutability.With this patch, build still passes, and "make check" also passes.
I think this is a misunderstanding:
-LockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode)
+LockTuple(Relation relation, const ItemPointer tid, LOCKMODE lockmode)This means that the pointer variable "tid" itself is constant, which is
probably not what you wanted. This would prevent changes to tid itself,
like if the function did
tid = NULL;
What you actually want would be something like
-LockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode)
+LockTuple(Relation relation, const ItemPointerData *tid, LOCKMODE lockmode)so that the qualifier applies to what is pointed to.
See for example the function ItemPointerGetBlockNumber() that
LockTuple() calls for this style.
This style of having Foo be a type alias for pointer-to-FooData is an
ancient Postgres coding convention that does not map well to modern C
that has an emphasis on judicious use of qualifiers and attributes, and
so if this abstraction gets in the way, we sometimes crack it open, like
in the case of ItemPointerGetBlockNumber() etc.
On Aug 27, 2025, at 17:24, Peter Eisentraut <peter@eisentraut.org> wrote:
This style of having Foo be a type alias for pointer-to-FooData is an ancient Postgres coding convention that does not map well to modern C that has an emphasis on judicious use of qualifiers and attributes, and so if this abstraction gets in the way, we sometimes crack it open, like in the case of ItemPointerGetBlockNumber() etc.
You are right, we want to protect the stuff that “tid” points to instead of “tid” itself:
*tid = something; // should hit compile error
tid = something; // ok
Also, thanks for telling the history. I have updated the patch to use “const ItemPointerData *” in the same way as ItemPointerGetBlockNumber().
Attached is the v2 patch.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Attachments:
v2-0001-Mark-ItemPointer-parameters-as-const-in-tuple-tab.patchapplication/octet-stream; name=v2-0001-Mark-ItemPointer-parameters-as-const-in-tuple-tab.patch; x-unix-mode=0644Download
From a26481cb0bcd0dbe223e0fc8ec1ea0c4d7a77619 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Wed, 27 Aug 2025 16:48:54 +0800
Subject: [PATCH v2] Mark ItemPointer parameters as const in tuple/table lock
functions
The functions LockTuple, ConditionalLockTuple, UnlockTuple, and
XactLockTableWait take an ItemPointer parameter named 'tid'. Since
these functions do not modify the tuple identifier, the parameter
should be declared as const to better convey intent and allow the
compiler to enforce immutability.
Changes:
- Add 'const' qualifier to ItemPointer tid parameter in the above functions.
- Update function declarations and definitions accordingly.
This improves code correctness and readability without affecting behavior.
Author: Chao Li <lic@highgo.com>
Discussion: https://postgr.es/m/CAEoWx2m9e4rECHBwpRE4%2BGCH%2BpbYZXLh2f4rB1Du5hDfKug%2BOg%40mail.gmail.com
---
src/backend/storage/lmgr/lmgr.c | 8 ++++----
src/include/storage/lmgr.h | 8 ++++----
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c
index 3f6bf70bd3c..81f9cdb0fc0 100644
--- a/src/backend/storage/lmgr/lmgr.c
+++ b/src/backend/storage/lmgr/lmgr.c
@@ -559,7 +559,7 @@ UnlockPage(Relation relation, BlockNumber blkno, LOCKMODE lockmode)
* tuple. See heap_lock_tuple before using this!
*/
void
-LockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode)
+LockTuple(Relation relation, const ItemPointerData *tid, LOCKMODE lockmode)
{
LOCKTAG tag;
@@ -579,7 +579,7 @@ LockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode)
* Returns true iff the lock was acquired.
*/
bool
-ConditionalLockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode,
+ConditionalLockTuple(Relation relation, const ItemPointerData *tid, LOCKMODE lockmode,
bool logLockFailure)
{
LOCKTAG tag;
@@ -598,7 +598,7 @@ ConditionalLockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode,
* UnlockTuple
*/
void
-UnlockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode)
+UnlockTuple(Relation relation, const ItemPointerData *tid, LOCKMODE lockmode)
{
LOCKTAG tag;
@@ -660,7 +660,7 @@ XactLockTableDelete(TransactionId xid)
* and if so wait for its parent.
*/
void
-XactLockTableWait(TransactionId xid, Relation rel, ItemPointer ctid,
+XactLockTableWait(TransactionId xid, Relation rel, const ItemPointerData *ctid,
XLTW_Oper oper)
{
LOCKTAG tag;
diff --git a/src/include/storage/lmgr.h b/src/include/storage/lmgr.h
index 58eee4e0d54..b7abd18397d 100644
--- a/src/include/storage/lmgr.h
+++ b/src/include/storage/lmgr.h
@@ -71,16 +71,16 @@ extern bool ConditionalLockPage(Relation relation, BlockNumber blkno, LOCKMODE l
extern void UnlockPage(Relation relation, BlockNumber blkno, LOCKMODE lockmode);
/* Lock a tuple (see heap_lock_tuple before assuming you understand this) */
-extern void LockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode);
-extern bool ConditionalLockTuple(Relation relation, ItemPointer tid,
+extern void LockTuple(Relation relation, const ItemPointerData *tid, LOCKMODE lockmode);
+extern bool ConditionalLockTuple(Relation relation, const ItemPointerData *tid,
LOCKMODE lockmode, bool logLockFailure);
-extern void UnlockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode);
+extern void UnlockTuple(Relation relation, const ItemPointerData *tid, LOCKMODE lockmode);
/* Lock an XID (used to wait for a transaction to finish) */
extern void XactLockTableInsert(TransactionId xid);
extern void XactLockTableDelete(TransactionId xid);
extern void XactLockTableWait(TransactionId xid, Relation rel,
- ItemPointer ctid, XLTW_Oper oper);
+ const ItemPointerData *ctid, XLTW_Oper oper);
extern bool ConditionalXactLockTableWait(TransactionId xid,
bool logLockFailure);
--
2.39.5 (Apple Git-154)
Mail Archive always misses attachments sent by Apple Mail. Resending from
the Gmail web client.
Chao Li (Evan)
---------------------
Highgo Software Co., Ltd.
https://www.highgo.com/
Chao Li <li.evan.chao@gmail.com> 于2025年8月28日周四 10:28写道：
Show quoted text
On Aug 27, 2025, at 17:24, Peter Eisentraut <peter@eisentraut.org> wrote:
This style of having Foo be a type alias for pointer-to-FooData is an
ancient Postgres coding convention that does not map well to modern C that
has an emphasis on judicious use of qualifiers and attributes, and so if
this abstraction gets in the way, we sometimes crack it open, like in the
case of ItemPointerGetBlockNumber() etc.You are right, we want to protect the stuff that “tid” points to instead
of “tid” itself:*tid = something; // should hit compile error
tid = something; // okAlso, thanks for telling the history. I have updated the patch to use
“const ItemPointerData *” in the same way as ItemPointerGetBlockNumber().Attached is the v2 patch.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
v2-0001-Mark-ItemPointer-parameters-as-const-in-tuple-tab.patchapplication/octet-stream; name=v2-0001-Mark-ItemPointer-parameters-as-const-in-tuple-tab.patchDownload
From a26481cb0bcd0dbe223e0fc8ec1ea0c4d7a77619 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Wed, 27 Aug 2025 16:48:54 +0800
Subject: [PATCH v2] Mark ItemPointer parameters as const in tuple/table lock
functions
The functions LockTuple, ConditionalLockTuple, UnlockTuple, and
XactLockTableWait take an ItemPointer parameter named 'tid'. Since
these functions do not modify the tuple identifier, the parameter
should be declared as const to better convey intent and allow the
compiler to enforce immutability.
Changes:
- Add 'const' qualifier to ItemPointer tid parameter in the above functions.
- Update function declarations and definitions accordingly.
This improves code correctness and readability without affecting behavior.
Author: Chao Li <lic@highgo.com>
Discussion: https://postgr.es/m/CAEoWx2m9e4rECHBwpRE4%2BGCH%2BpbYZXLh2f4rB1Du5hDfKug%2BOg%40mail.gmail.com
---
src/backend/storage/lmgr/lmgr.c | 8 ++++----
src/include/storage/lmgr.h | 8 ++++----
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c
index 3f6bf70bd3c..81f9cdb0fc0 100644
--- a/src/backend/storage/lmgr/lmgr.c
+++ b/src/backend/storage/lmgr/lmgr.c
@@ -559,7 +559,7 @@ UnlockPage(Relation relation, BlockNumber blkno, LOCKMODE lockmode)
* tuple. See heap_lock_tuple before using this!
*/
void
-LockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode)
+LockTuple(Relation relation, const ItemPointerData *tid, LOCKMODE lockmode)
{
LOCKTAG tag;
@@ -579,7 +579,7 @@ LockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode)
* Returns true iff the lock was acquired.
*/
bool
-ConditionalLockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode,
+ConditionalLockTuple(Relation relation, const ItemPointerData *tid, LOCKMODE lockmode,
bool logLockFailure)
{
LOCKTAG tag;
@@ -598,7 +598,7 @@ ConditionalLockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode,
* UnlockTuple
*/
void
-UnlockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode)
+UnlockTuple(Relation relation, const ItemPointerData *tid, LOCKMODE lockmode)
{
LOCKTAG tag;
@@ -660,7 +660,7 @@ XactLockTableDelete(TransactionId xid)
* and if so wait for its parent.
*/
void
-XactLockTableWait(TransactionId xid, Relation rel, ItemPointer ctid,
+XactLockTableWait(TransactionId xid, Relation rel, const ItemPointerData *ctid,
XLTW_Oper oper)
{
LOCKTAG tag;
diff --git a/src/include/storage/lmgr.h b/src/include/storage/lmgr.h
index 58eee4e0d54..b7abd18397d 100644
--- a/src/include/storage/lmgr.h
+++ b/src/include/storage/lmgr.h
@@ -71,16 +71,16 @@ extern bool ConditionalLockPage(Relation relation, BlockNumber blkno, LOCKMODE l
extern void UnlockPage(Relation relation, BlockNumber blkno, LOCKMODE lockmode);
/* Lock a tuple (see heap_lock_tuple before assuming you understand this) */
-extern void LockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode);
-extern bool ConditionalLockTuple(Relation relation, ItemPointer tid,
+extern void LockTuple(Relation relation, const ItemPointerData *tid, LOCKMODE lockmode);
+extern bool ConditionalLockTuple(Relation relation, const ItemPointerData *tid,
LOCKMODE lockmode, bool logLockFailure);
-extern void UnlockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode);
+extern void UnlockTuple(Relation relation, const ItemPointerData *tid, LOCKMODE lockmode);
/* Lock an XID (used to wait for a transaction to finish) */
extern void XactLockTableInsert(TransactionId xid);
extern void XactLockTableDelete(TransactionId xid);
extern void XactLockTableWait(TransactionId xid, Relation rel,
- ItemPointer ctid, XLTW_Oper oper);
+ const ItemPointerData *ctid, XLTW_Oper oper);
extern bool ConditionalXactLockTableWait(TransactionId xid,
bool logLockFailure);
--
2.39.5 (Apple Git-154)
On 28.08.25 04:27, Chao Li wrote:
On Aug 27, 2025, at 17:24, Peter Eisentraut <peter@eisentraut.org> wrote:
This style of having Foo be a type alias for pointer-to-FooData is an
ancient Postgres coding convention that does not map well to modern C
that has an emphasis on judicious use of qualifiers and attributes,
and so if this abstraction gets in the way, we sometimes crack it
open, like in the case of ItemPointerGetBlockNumber() etc.You are right, we want to protect the stuff that “tid” points to instead
of “tid” itself:*tid = something; // should hit compile error
tid = something; // okAlso, thanks for telling the history. I have updated the patch to use
“const ItemPointerData *” in the same way as ItemPointerGetBlockNumber().Attached is the v2 patch.
This patch still causes a compiler warning:
../src/backend/storage/lmgr/lmgr.c: In function 'XactLockTableWait':
../src/backend/storage/lmgr/lmgr.c:681:27: error: assignment discards
'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
I have fixed that and committed your patch.
Hi Peter,
Thank you so much for your help.
On Aug 29, 2025, at 13:42, Peter Eisentraut <peter@eisentraut.org> wrote:
This patch still causes a compiler warning:
../src/backend/storage/lmgr/lmgr.c: In function 'XactLockTableWait':
../src/backend/storage/lmgr/lmgr.c:681:27: error: assignment discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]I have fixed that and committed your patch.
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/