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.