Remove meaningless const qualifier from ginCompressPostingList()

Started by Chao Li3 months ago2 messages
#1Chao Li
li.evan.chao@gmail.com
1 attachment(s)

Hi Hacker,

While working on the other patch that fixed wrong "const" usage [1], I
found the function:
```
GinPostingList *
ginCompressPostingList(const ItemPointer ipd, int nipd, int maxsize,
int *nwritten)
```
uses "const" unnecessarily. Because it needs to assign an element of "ipd"
to the returned structure "GinPostingList->first" and "first" is a mutable
"ItemPointerData *", so that "ipd" cannot be of const pointer.

With the current usage, "const" only protects "ipd" itself from updating,
which is meaningless. Thus the "const" only adds confusion to code readers,
so I am deleting it.

/messages/by-id/CAEoWx2m2E0xE8Kvbkv31ULh_E+5zph-WA_bEdv3UR9CLhw+3vg@mail.gmail.com

Chao Li (Evan)
---------------------
HighGo Software Co., Ltd.
https://www.highgo.com/

Attachments:

v1-0001-Remove-meaningless-const-qualifier-from-ginCompre.patchapplication/octet-stream; name=v1-0001-Remove-meaningless-const-qualifier-from-ginCompre.patchDownload
From 0d0243dbbf59c745577f9de874f00281943b829a Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Wed, 29 Oct 2025 11:36:43 +0800
Subject: [PATCH v1] Remove meaningless const qualifier from
 ginCompressPostingList()

The parameter "ipd" was previously declared as "const ItemPointer",
but this const qualifier was ineffective. The function copies elements
of "ipd" into a returned structure that expects a mutable
ItemPointerData *, so the pointer itself is not treated as read-only.
Remove the const to better reflect the actual usage and avoid
confusing semantics.

Author: Chao Li <lic@highgo.com>
---
 src/backend/access/gin/ginpostinglist.c | 2 +-
 src/include/access/gin_private.h        | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/gin/ginpostinglist.c b/src/backend/access/gin/ginpostinglist.c
index 48eadec87b0..739d0c99d5f 100644
--- a/src/backend/access/gin/ginpostinglist.c
+++ b/src/backend/access/gin/ginpostinglist.c
@@ -194,7 +194,7 @@ decode_varbyte(unsigned char **ptr)
  * byte at the end, if any, is zero.
  */
 GinPostingList *
-ginCompressPostingList(const ItemPointer ipd, int nipd, int maxsize,
+ginCompressPostingList(ItemPointer ipd, int nipd, int maxsize,
 					   int *nwritten)
 {
 	uint64		prev;
diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h
index 9ea303a7c1b..3abccf278e0 100644
--- a/src/include/access/gin_private.h
+++ b/src/include/access/gin_private.h
@@ -478,7 +478,7 @@ extern void ginInsertCleanup(GinState *ginstate, bool full_clean,
 
 /* ginpostinglist.c */
 
-extern GinPostingList *ginCompressPostingList(const ItemPointer ipd, int nipd,
+extern GinPostingList *ginCompressPostingList(ItemPointer ipd, int nipd,
 											  int maxsize, int *nwritten);
 extern int	ginPostingListDecodeAllSegmentsToTbm(GinPostingList *ptr, int len, TIDBitmap *tbm);
 
-- 
2.39.5 (Apple Git-154)

#2Peter Eisentraut
peter@eisentraut.org
In reply to: Chao Li (#1)
Re: Remove meaningless const qualifier from ginCompressPostingList()

On 29.10.25 04:42, Chao Li wrote:

While working on the other patch that fixed wrong "const" usage [1], I
found the function:
```
GinPostingList *
ginCompressPostingList(const ItemPointer ipd, int nipd, int maxsize,
  int *nwritten)
```
uses "const" unnecessarily. Because it needs to assign an element of
"ipd" to the returned structure "GinPostingList->first" and "first" is a
mutable "ItemPointerData *", so that "ipd" cannot be of const pointer.

I have committed a fix for this together with the other one.

The code you are referring to here is:

result->first = ipd[0];

This is a value copy, so this does not violate the immutability of ipd.
So the const in the function prototype was the right idea, but in the
wrong place of course.