Обсуждение: get rid of Pointer type, mostly
In a previous thread[0], the question was asked, 'Why do we bother with
a "Pointer" type?'. So I looked into get rid of it.
There are two stages to this. One is changing all code that wants to do
pointer arithmetic to use char * instead of relying on Pointer being
char *. Then we can change Pointer to be void * and remove a bunch of
casts.
The second is getting rid of uses of Pointer for variables where you
might as well use void * directly. These are actually not that many.
This gets rid of all uses, except in the GIN code, which is full of
Pointer use, and it's part of the documented API. I'm not touching
that, not least because this kind of code
Pointer **extra_data = (Pointer **) PG_GETARG_POINTER(4);
needs more brain-bending to understand that I'm prepared to spend. So
as far as I'm concerned, the pointer type can continue to exist as a
curiosity of the GIN API, but in all other places, it wasn't really
doing much of anything anyway.
[0]:
https://www.postgresql.org/message-id/CA%2BhUKG%2BNFKnr%3DK4oybwDvT35dW%3DVAjAAfiuLxp%2B5JeZSOV3nBg%40mail.gmail.com
Вложения
> On Nov 24, 2025, at 18:20, Peter Eisentraut <peter@eisentraut.org> wrote: > > In a previous thread[0], the question was asked, 'Why do we bother with a "Pointer" type?'. So I looked into get rid ofit. > > There are two stages to this. One is changing all code that wants to do pointer arithmetic to use char * instead of relyingon Pointer being char *. Then we can change Pointer to be void * and remove a bunch of casts. > > The second is getting rid of uses of Pointer for variables where you might as well use void * directly. These are actuallynot that many. > > This gets rid of all uses, except in the GIN code, which is full of Pointer use, and it's part of the documented API. I'm not touching that, not least because this kind of code > > Pointer **extra_data = (Pointer **) PG_GETARG_POINTER(4); > > needs more brain-bending to understand that I'm prepared to spend. So as far as I'm concerned, the pointer type can continueto exist as a curiosity of the GIN API, but in all other places, it wasn't really doing much of anything anyway. > > > [0]: https://www.postgresql.org/message-id/CA%2BhUKG%2BNFKnr%3DK4oybwDvT35dW%3DVAjAAfiuLxp%2B5JeZSOV3nBg%40mail.gmail.com<0001-Remove-useless-casts-to-Pointer.patch><0002-Use-better-DatumGet-function.patch><0003-Don-t-rely-on-pointer-arithmetic-with-Pointer-type.patch><0004-Change-Pointer-to-void.patch><0005-Remove-no-longer-needed-casts-to-Pointer.patch><0006-Remove-some-uses-of-the-Pointer-type.patch> 0001 - Removed type cast from memcpy(), which should be safe, as memcpy doesn’t care about types of source and dest pointersreferring to, type casting is redundant. 0002 - Changed DatumGetPointer() to DatumGetCString() for %s. Basically, DatumGetPointer() returns a void * pointer and DatumGetCString()returns a char * pointers, but they return the same addresses, thus DatumGetCString() better fits %s. 0003 - Changed type casting from Pointer to char *. Now, Pointer is a typedef of char *, so the replacement is safe. In generic_desc(),Pointer is replaced with const char *, which should be safe also. 0004 - Changed typedef of Pointer from char * to void *. I guess the purpose is to let compiler alter for missed usages ofPointer. 0005/0006 - Removed all usages of Pointer expect in Gin code. All look good. Only things is that, as Pointer is changed from char * to void *, and Gin code are still using Pointer, sothese is a change for Gin code. But I don’t think that would impact runtime, as long as build passes, that should be fine.Build passed on my MacBook M4. If there is a breakage, build farm should be able to catch the error. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
Hi,
On Mon, Nov 24, 2025 at 11:20:56AM +0100, Peter Eisentraut wrote:
> In a previous thread[0], the question was asked, 'Why do we bother with a
> "Pointer" type?'. So I looked into get rid of it.
>
> There are two stages to this. One is changing all code that wants to do
> pointer arithmetic to use char * instead of relying on Pointer being char *.
> Then we can change Pointer to be void * and remove a bunch of casts.
>
> The second is getting rid of uses of Pointer for variables where you might
> as well use void * directly. These are actually not that many.
>
> This gets rid of all uses, except in the GIN code, which is full of Pointer
> use, and it's part of the documented API. I'm not touching that, not least
> because this kind of code
>
> Pointer **extra_data = (Pointer **) PG_GETARG_POINTER(4);
>
> needs more brain-bending to understand that I'm prepared to spend. So as
> far as I'm concerned, the pointer type can continue to exist as a curiosity
> of the GIN API, but in all other places, it wasn't really doing much of
> anything anyway.
The patch series is very easy to follow, thanks! I agree with the idea: we now
use (char *) for byte(s) manipulation and (void *) for generic pointer usage.
I checked the changes and if any have been missed and that looks ok to me.
Just a nit, while at it, maybe we could get rid of those extra parentheses:
@@ -140,20 +140,20 @@ GinDataLeafPageGetItems(Page page, int *nitems, ItemPointerData advancePast)
{
GinPostingList *seg = GinDataLeafPageGetPostingList(page);
Size len = GinDataLeafPageGetPostingListSize(page);
- Pointer endptr = ((Pointer) seg) + len;
+ char *endptr = ((char *) seg) + len;
The other existing ones in some macros are good to keep.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Peter Eisentraut <peter@eisentraut.org> writes:
> In a previous thread[0], the question was asked, 'Why do we bother with
> a "Pointer" type?'. So I looked into get rid of it.
> There are two stages to this. One is changing all code that wants to do
> pointer arithmetic to use char * instead of relying on Pointer being
> char *. Then we can change Pointer to be void * and remove a bunch of
> casts.
I'm in favor of that ...
> The second is getting rid of uses of Pointer for variables where you
> might as well use void * directly. These are actually not that many.
... but not of that. In particular, I think it's just fine if
DatumGetPointer and PointerGetDatum take and return Pointer.
regards, tom lane
On Mon, Nov 24, 2025 at 11:09 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > The second is getting rid of uses of Pointer for variables where you > > might as well use void * directly. These are actually not that many. > > ... but not of that. In particular, I think it's just fine if > DatumGetPointer and PointerGetDatum take and return Pointer. What's your objection? (I don't have a considered opinion on this particular point, but in general I've found that using Pointer seems to make life worse rather than better.) -- Robert Haas EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Nov 24, 2025 at 11:09 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> The second is getting rid of uses of Pointer for variables where you
>>> might as well use void * directly. These are actually not that many.
>> ... but not of that. In particular, I think it's just fine if
>> DatumGetPointer and PointerGetDatum take and return Pointer.
> What's your objection?
We have lots of places where we use trivial typedefs to annotate
what something is. For instance "text *" is not really different
from "struct varlena *", but I don't think anyone would be in favor
of removing the "text" typedef. In this case we have decades of
practice using Pointer to annotate something as being a generic
pointer. I'm in favor of switching it to be "void *" to conform
more closely to modern C semantics, but not of just trying to get
rid of it. Especially so if the removal is incomplete. What have
you really accomplished then?
> (I don't have a considered opinion on this particular point, but in
> general I've found that using Pointer seems to make life worse rather
> than better.)
How much of that comes from "char *" versus "void *"?
regards, tom lane
On Mon, Nov 24, 2025, 09:34 Tom Lane <tgl@sss.pgh.pa.us> wrote:
Especially so if the removal is incomplete. What have
you really accomplished then?
In this case, what we would accomplish is that no new developer to the project has to understand what some unclear typedef means, *unless* they touch GIN related code. Just from its name it's definitely not clear to me that Pointer means char * instead of void *. And this typedef is ven shorter than the thing it represents.
Side annoyance: I think this is a falacy that hackers discussions end up in a lot. Someone suggesting that the partial improvements have (almost) no benefit and all cases need to be fixed in one go to before it should be committed. Then the patch author thinks that's too much work and then nothing ends up being improved at all.
On Mon, Nov 24, 2025 at 11:33 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > We have lots of places where we use trivial typedefs to annotate > what something is. For instance "text *" is not really different > from "struct varlena *", but I don't think anyone would be in favor > of removing the "text" typedef. In this case we have decades of > practice using Pointer to annotate something as being a generic > pointer. In my mind, a text * points to a varlena whose payload data is valid in the relevant encoding, i.e. something that will be legal if you pass it to functions that expect to work on text Datums. But I'm not very clear on what Pointer is supposed to mean. Sometimes we use it to indicate that a char * is a generic pointer rather than a pointer to a C string, but sometimes we just write char * anyway, so you can't use the fact that something is declared as char * to mean that it isn't intended as a generic pointer. If we change Pointer to be void *, then even that clarifying value is lost, since void * has no other meaning. But if it were up to me, I'd rip out Pointer completely, because reading code that uses the native C type names is easier for me than reading code that substitutes other notation. In my experience, there's rarely any practical confusion about whether char * is a C string, a character array, or a generic pointer. It's not impossible for such confusion to exist, of course, but typically pointer arithmetic is confined to relatively brief stretches of code to avoid breaking the author's brain, or the reader's. If you see a pointer to a struct get cast to a char * or the other way around, you know what's happening. It would be confusing if the char * intended as a generic pointer were passed through multiple layers of function calls, but for those cases it's typically convenient to use void *, so the problem doesn't really arise, and in the rare cases where it might, one can always write a comment to clear things up. We have lots of data types that seem to me to have enough documentation value to justify their existence, but IMHO, this isn't one of them. -- Robert Haas EDB: http://www.enterprisedb.com
On Mon, Nov 24, 2025 at 12:30 PM Jelte Fennema-Nio <postgres@jeltef.nl> wrote: > In this case, what we would accomplish is that no new developer to the project has to understand what some unclear typedefmeans, *unless* they touch GIN related code. Just from its name it's definitely not clear to me that Pointer meanschar * instead of void *. And this typedef is ven shorter than the thing it represents. +1. > Side annoyance: I think this is a falacy that hackers discussions end up in a lot. Someone suggesting that the partialimprovements have (almost) no benefit and all cases need to be fixed in one go to before it should be committed. Thenthe patch author thinks that's too much work and then nothing ends up being improved at all. This is definitely a thing that happens, but what also happens pretty often is that people claim that we'll follow up on a partial improvement with lots more work and then we never do, and then it creates a big mess for somebody else to untangle later. I understand the frustration with getting a partial solution blocked, because half a loaf is better than none, but I've also done my share of cleaning up changes that weren't so much half a loaf as half-baked. -- Robert Haas EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
> But if it were up to me, I'd rip out Pointer completely, because
> reading code that uses the native C type names is easier for me than
> reading code that substitutes other notation.
I can follow the argument that using the native type "void *" is
better, since every C programmer must know that already. But you
cannot argue for this patch on that ground unless Pointer goes away
entirely. I don't understand leaving it in place for GIN. It's
not like GIN indexes are some hoary backwater that nobody pays
attention to.
regards, tom lane
I'm in big favor of this change. Such types just cover up what's really going on and make reading the code more difficult than needed, especially for people new to the code base. On 24.11.2025 19:26, Tom Lane wrote: > Robert Haas <robertmhaas@gmail.com> writes: > I can follow the argument that using the native type "void *" is > better, since every C programmer must know that already. But you > cannot argue for this patch on that ground unless Pointer goes away > entirely. I don't understand leaving it in place for GIN. It's > not like GIN indexes are some hoary backwater that nobody pays > attention to. +1 The GIN code makes use of pointer but src/backend/access/gin only has 29 occurrences. If you like I can help out fixing up the GIN code and share a page here. Let me know. -- David Geier
On Mon, Nov 24, 2025 at 1:46 PM David Geier <geidav.pg@gmail.com> wrote: > The GIN code makes use of pointer but src/backend/access/gin only has 29 > occurrences. If you like I can help out fixing up the GIN code and share > a page here. Let me know. I'd go for it! I mean, who knows whether your patch will be accepted? But another pair of eyes couldn't hurt. It seems like we all agree that a full removal of Pointer would be better than a partial removal; it's just a question of whether we can get there without too much other awkwardness. -- Robert Haas EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Nov 24, 2025 at 1:46 PM David Geier <geidav.pg@gmail.com> wrote:
>> The GIN code makes use of pointer but src/backend/access/gin only has 29
>> occurrences. If you like I can help out fixing up the GIN code and share
>> a page here. Let me know.
> I'd go for it! I mean, who knows whether your patch will be accepted?
> But another pair of eyes couldn't hurt. It seems like we all agree
> that a full removal of Pointer would be better than a partial removal;
> it's just a question of whether we can get there without too much
> other awkwardness.
If there are actually places in GIN where using void* would be less
readable than using Pointer, that would certainly be interesting
information. Perhaps the patch would need to spend some effort
on adding comments, not just mechanically replacing the typedef?
regards, tom lane
On Mon, Nov 24, 2025 at 1:26 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > I don't understand leaving it in place for GIN. I haven't tried removing it for GIN so I don't know how awkward that would be or for what reasons, but... > It's > not like GIN indexes are some hoary backwater that nobody pays > attention to. ...I almost feel like you're trolling with this comment. It is true that we maintain that code, and I see in the commit log that there are even some GIN-specific improvements in the recent past. But the average PostgreSQL hacker can probably go years and years without ever having to touch the GIN code. Hoary backwater might be overselling it, but it's far enough in that direction that confining the need to be aware of one specific PostgreSQL-ism just to GIN is not without value. -- Robert Haas EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Nov 24, 2025 at 1:26 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I don't understand leaving it in place for GIN.
> I haven't tried removing it for GIN so I don't know how awkward that
> would be or for what reasons, but...
Well, either Peter just ran out of energy or there is actually some
notational value in Pointer. If it's the latter, I'd like to know.
regards, tom lane
On Mon, Nov 24, 2025 at 2:38 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Well, either Peter just ran out of energy or there is actually some
> notational value in Pointer. If it's the latter, I'd like to know.
I agree that would be nice to know.
Peter's original email seemed to indicate that he was deterred by this
sort of thing:
Pointer **extra_data = (Pointer **) PG_GETARG_POINTER(4);
If Pointer is merely char *, then this is equivalent to:
char ***extra_data = (char ***) PG_GETARG_POINTER(4);
I believe this is the same extra_data that is documented thus:
extra_data is an output argument that allows extractQuery to pass
additional data to the consistent and comparePartial methods. To use
it, extractQuery must allocate an array of *nkeys pointers and store
its address at *extra_data, then store whatever it wants to into the
individual pointers. The variable is initialized to NULL before call,
so this argument can simply be ignored by operator classes that do not
require extra data. If *extra_data is set, the whole array is passed
to the consistent method, and the appropriate element to the
comparePartial method.
So in other words, it's a pointer to an array of generic pointers. In
a vacuum, I'd suggest that having three levels of indirection that are
all semantically different but all denoted by an asterisk in C is
confusing enough to be a bad idea regardless of the specifics. But
since we've already crossed that bridge, we'll just need to make the
best of it. Maybe we could use a more specific typedef here, like
GinExtraPointer. That would be a lot more greppable than just writing
Pointer, and every GinExtraPointer would be the same flavor of generic
pointer, whereas any given Pointer is not necessarily related in any
semantic way to any other.
--
Robert Haas
EDB: http://www.enterprisedb.com
Tom Lane <tgl@sss.pgh.pa.us> writes:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Mon, Nov 24, 2025 at 1:46 PM David Geier <geidav.pg@gmail.com> wrote:
>>> The GIN code makes use of pointer but src/backend/access/gin only has 29
>>> occurrences. If you like I can help out fixing up the GIN code and share
>>> a page here. Let me know.
>
>> I'd go for it! I mean, who knows whether your patch will be accepted?
>> But another pair of eyes couldn't hurt. It seems like we all agree
>> that a full removal of Pointer would be better than a partial removal;
>> it's just a question of whether we can get there without too much
>> other awkwardness.
>
> If there are actually places in GIN where using void* would be less
> readable than using Pointer, that would certainly be interesting
> information. Perhaps the patch would need to spend some effort
> on adding comments, not just mechanically replacing the typedef?
I got curious and did the replacement, and IMO there's no need for any
further commentary than what's already there. I did however take the
opportunity to get rid of some pointless casts (except the return value
of PG_GETARG_POINTER(), which seems to be de rigueur to redundantly
cast), and to convert a nearby char * that's only used for memcpy() to
void *.
- ilmari
From 9c5221b1de91523db775867158ea3da9b00cb650 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Mon, 24 Nov 2025 20:05:38 +0000
Subject: [PATCH] Convert remaning uses of Pointer to void *
Also remove redundant casts of the modified variables (except
PG_GETARG_POINTER()), and change a nearby char * to void *.
---
contrib/amcheck/verify_gin.c | 4 ++--
contrib/btree_gin/btree_gin.c | 6 +++---
contrib/hstore/hstore_gin.c | 2 +-
contrib/intarray/_int_gin.c | 2 +-
contrib/pg_trgm/trgm_gin.c | 18 ++++++++---------
src/backend/access/gin/ginarrayproc.c | 6 +++---
src/backend/access/gin/ginentrypage.c | 8 ++++----
src/backend/access/gin/ginscan.c | 8 ++++----
src/backend/utils/adt/jsonb_gin.c | 28 ++++++++++++---------------
src/backend/utils/adt/selfuncs.c | 2 +-
src/backend/utils/adt/tsginidx.c | 16 +++++++--------
src/include/access/gin_private.h | 6 +++---
src/include/access/ginblock.h | 2 +-
13 files changed, 51 insertions(+), 57 deletions(-)
diff --git a/contrib/amcheck/verify_gin.c b/contrib/amcheck/verify_gin.c
index 5c3eb4d0fd4..8333dcdcba3 100644
--- a/contrib/amcheck/verify_gin.c
+++ b/contrib/amcheck/verify_gin.c
@@ -98,7 +98,7 @@ gin_index_check(PG_FUNCTION_ARGS)
static ItemPointer
ginReadTupleWithoutState(IndexTuple itup, int *nitems)
{
- Pointer ptr = GinGetPosting(itup);
+ void *ptr = GinGetPosting(itup);
int nipd = GinGetNPosting(itup);
ItemPointer ipd;
int ndecoded;
@@ -107,7 +107,7 @@ ginReadTupleWithoutState(IndexTuple itup, int *nitems)
{
if (nipd > 0)
{
- ipd = ginPostingListDecode((GinPostingList *) ptr, &ndecoded);
+ ipd = ginPostingListDecode(ptr, &ndecoded);
if (nipd != ndecoded)
elog(ERROR, "number of items mismatch in GIN entry tuple, %d in tuple header, %d decoded",
nipd, ndecoded);
diff --git a/contrib/btree_gin/btree_gin.c b/contrib/btree_gin/btree_gin.c
index 8c477d17e22..7162efd68ad 100644
--- a/contrib/btree_gin/btree_gin.c
+++ b/contrib/btree_gin/btree_gin.c
@@ -73,7 +73,7 @@ gin_btree_extract_query(FunctionCallInfo fcinfo,
int32 *nentries = (int32 *) PG_GETARG_POINTER(1);
StrategyNumber strategy = PG_GETARG_UINT16(2);
bool **partialmatch = (bool **) PG_GETARG_POINTER(3);
- Pointer **extra_data = (Pointer **) PG_GETARG_POINTER(4);
+ void ***extra_data = (void ***) PG_GETARG_POINTER(4);
Datum *entries = (Datum *) palloc(sizeof(Datum));
QueryInfo *data = (QueryInfo *) palloc(sizeof(QueryInfo));
bool *ptr_partialmatch = (bool *) palloc(sizeof(bool));
@@ -139,8 +139,8 @@ gin_btree_extract_query(FunctionCallInfo fcinfo,
data->orig_datum = datum;
data->entry_datum = entries[0];
data->typecmp = cmp_fns[rhs_code];
- *extra_data = (Pointer *) palloc(sizeof(Pointer));
- **extra_data = (Pointer) data;
+ *extra_data = palloc(sizeof(void *));
+ **extra_data = data;
PG_RETURN_POINTER(entries);
}
diff --git a/contrib/hstore/hstore_gin.c b/contrib/hstore/hstore_gin.c
index 2e5fa115924..4b446f4d9e3 100644
--- a/contrib/hstore/hstore_gin.c
+++ b/contrib/hstore/hstore_gin.c
@@ -156,7 +156,7 @@ gin_consistent_hstore(PG_FUNCTION_ARGS)
/* HStore *query = PG_GETARG_HSTORE_P(2); */
int32 nkeys = PG_GETARG_INT32(3);
- /* Pointer *extra_data = (Pointer *) PG_GETARG_POINTER(4); */
+ /* void **extra_data = (void **) PG_GETARG_POINTER(4); */
bool *recheck = (bool *) PG_GETARG_POINTER(5);
bool res = true;
int32 i;
diff --git a/contrib/intarray/_int_gin.c b/contrib/intarray/_int_gin.c
index b7958d8eca5..5deb3d437e8 100644
--- a/contrib/intarray/_int_gin.c
+++ b/contrib/intarray/_int_gin.c
@@ -113,7 +113,7 @@ ginint4_consistent(PG_FUNCTION_ARGS)
StrategyNumber strategy = PG_GETARG_UINT16(1);
int32 nkeys = PG_GETARG_INT32(3);
- /* Pointer *extra_data = (Pointer *) PG_GETARG_POINTER(4); */
+ /* void **extra_data = (void **) PG_GETARG_POINTER(4); */
bool *recheck = (bool *) PG_GETARG_POINTER(5);
bool res = false;
int32 i;
diff --git a/contrib/pg_trgm/trgm_gin.c b/contrib/pg_trgm/trgm_gin.c
index 29a52eac7af..e9c59636f28 100644
--- a/contrib/pg_trgm/trgm_gin.c
+++ b/contrib/pg_trgm/trgm_gin.c
@@ -74,7 +74,7 @@ gin_extract_query_trgm(PG_FUNCTION_ARGS)
StrategyNumber strategy = PG_GETARG_UINT16(2);
/* bool **pmatch = (bool **) PG_GETARG_POINTER(3); */
- Pointer **extra_data = (Pointer **) PG_GETARG_POINTER(4);
+ void ***extra_data = (void ***) PG_GETARG_POINTER(4);
/* bool **nullFlags = (bool **) PG_GETARG_POINTER(5); */
int32 *searchMode = (int32 *) PG_GETARG_POINTER(6);
@@ -120,12 +120,12 @@ gin_extract_query_trgm(PG_FUNCTION_ARGS)
/*
* Successful regex processing: store NFA-like graph as
* extra_data. GIN API requires an array of nentries
- * Pointers, but we just put the same value in each element.
+ * pointers, but we just put the same value in each element.
*/
trglen = ARRNELEM(trg);
- *extra_data = (Pointer *) palloc(sizeof(Pointer) * trglen);
+ *extra_data = palloc(sizeof(void *) * trglen);
for (i = 0; i < trglen; i++)
- (*extra_data)[i] = (Pointer) graph;
+ (*extra_data)[i] = graph;
}
else
{
@@ -174,7 +174,7 @@ gin_trgm_consistent(PG_FUNCTION_ARGS)
/* text *query = PG_GETARG_TEXT_PP(2); */
int32 nkeys = PG_GETARG_INT32(3);
- Pointer *extra_data = (Pointer *) PG_GETARG_POINTER(4);
+ void **extra_data = (void **) PG_GETARG_POINTER(4);
bool *recheck = (bool *) PG_GETARG_POINTER(5);
bool res;
int32 i,
@@ -247,8 +247,7 @@ gin_trgm_consistent(PG_FUNCTION_ARGS)
res = true;
}
else
- res = trigramsMatchGraph((TrgmPackedGraph *) extra_data[0],
- check);
+ res = trigramsMatchGraph(extra_data[0], check);
break;
default:
elog(ERROR, "unrecognized strategy number: %d", strategy);
@@ -273,7 +272,7 @@ gin_trgm_triconsistent(PG_FUNCTION_ARGS)
/* text *query = PG_GETARG_TEXT_PP(2); */
int32 nkeys = PG_GETARG_INT32(3);
- Pointer *extra_data = (Pointer *) PG_GETARG_POINTER(4);
+ void **extra_data = (void **) PG_GETARG_POINTER(4);
GinTernaryValue res = GIN_MAYBE;
int32 i,
ntrue;
@@ -342,8 +341,7 @@ gin_trgm_triconsistent(PG_FUNCTION_ARGS)
boolcheck = (bool *) palloc(sizeof(bool) * nkeys);
for (i = 0; i < nkeys; i++)
boolcheck[i] = (check[i] != GIN_FALSE);
- if (!trigramsMatchGraph((TrgmPackedGraph *) extra_data[0],
- boolcheck))
+ if (!trigramsMatchGraph(extra_data[0], boolcheck))
res = GIN_FALSE;
pfree(boolcheck);
}
diff --git a/src/backend/access/gin/ginarrayproc.c b/src/backend/access/gin/ginarrayproc.c
index 1f821323eb0..eaeb8feb3a9 100644
--- a/src/backend/access/gin/ginarrayproc.c
+++ b/src/backend/access/gin/ginarrayproc.c
@@ -84,7 +84,7 @@ ginqueryarrayextract(PG_FUNCTION_ARGS)
StrategyNumber strategy = PG_GETARG_UINT16(2);
/* bool **pmatch = (bool **) PG_GETARG_POINTER(3); */
- /* Pointer *extra_data = (Pointer *) PG_GETARG_POINTER(4); */
+ /* void **extra_data = (void **) PG_GETARG_POINTER(4); */
bool **nullFlags = (bool **) PG_GETARG_POINTER(5);
int32 *searchMode = (int32 *) PG_GETARG_POINTER(6);
int16 elmlen;
@@ -147,7 +147,7 @@ ginarrayconsistent(PG_FUNCTION_ARGS)
/* ArrayType *query = PG_GETARG_ARRAYTYPE_P(2); */
int32 nkeys = PG_GETARG_INT32(3);
- /* Pointer *extra_data = (Pointer *) PG_GETARG_POINTER(4); */
+ /* void **extra_data = (void **) PG_GETARG_POINTER(4); */
bool *recheck = (bool *) PG_GETARG_POINTER(5);
/* Datum *queryKeys = (Datum *) PG_GETARG_POINTER(6); */
@@ -231,7 +231,7 @@ ginarraytriconsistent(PG_FUNCTION_ARGS)
/* ArrayType *query = PG_GETARG_ARRAYTYPE_P(2); */
int32 nkeys = PG_GETARG_INT32(3);
- /* Pointer *extra_data = (Pointer *) PG_GETARG_POINTER(4); */
+ /* void **extra_data = (void **) PG_GETARG_POINTER(4); */
/* Datum *queryKeys = (Datum *) PG_GETARG_POINTER(5); */
bool *nullFlags = (bool *) PG_GETARG_POINTER(6);
GinTernaryValue res;
diff --git a/src/backend/access/gin/ginentrypage.c b/src/backend/access/gin/ginentrypage.c
index c0592367700..6be7b3ffcd1 100644
--- a/src/backend/access/gin/ginentrypage.c
+++ b/src/backend/access/gin/ginentrypage.c
@@ -43,7 +43,7 @@ static void entrySplitPage(GinBtree btree, Buffer origbuf,
IndexTuple
GinFormTuple(GinState *ginstate,
OffsetNumber attnum, Datum key, GinNullCategory category,
- Pointer data, Size dataSize, int nipd,
+ void *data, Size dataSize, int nipd,
bool errorTooBig)
{
Datum datums[2];
@@ -136,7 +136,7 @@ GinFormTuple(GinState *ginstate,
*/
if (data)
{
- char *ptr = GinGetPosting(itup);
+ void *ptr = GinGetPosting(itup);
memcpy(ptr, data, dataSize);
}
@@ -162,7 +162,7 @@ ItemPointer
ginReadTuple(GinState *ginstate, OffsetNumber attnum, IndexTuple itup,
int *nitems)
{
- Pointer ptr = GinGetPosting(itup);
+ void *ptr = GinGetPosting(itup);
int nipd = GinGetNPosting(itup);
ItemPointer ipd;
int ndecoded;
@@ -171,7 +171,7 @@ ginReadTuple(GinState *ginstate, OffsetNumber attnum, IndexTuple itup,
{
if (nipd > 0)
{
- ipd = ginPostingListDecode((GinPostingList *) ptr, &ndecoded);
+ ipd = ginPostingListDecode(ptr, &ndecoded);
if (nipd != ndecoded)
elog(ERROR, "number of items mismatch in GIN entry tuple, %d in tuple header, %d decoded",
nipd, ndecoded);
diff --git a/src/backend/access/gin/ginscan.c b/src/backend/access/gin/ginscan.c
index 26081693383..5aec1943ece 100644
--- a/src/backend/access/gin/ginscan.c
+++ b/src/backend/access/gin/ginscan.c
@@ -57,7 +57,7 @@ static GinScanEntry
ginFillScanEntry(GinScanOpaque so, OffsetNumber attnum,
StrategyNumber strategy, int32 searchMode,
Datum queryKey, GinNullCategory queryCategory,
- bool isPartialMatch, Pointer extra_data)
+ bool isPartialMatch, void *extra_data)
{
GinState *ginstate = &so->ginstate;
GinScanEntry scanEntry;
@@ -160,7 +160,7 @@ ginFillScanKey(GinScanOpaque so, OffsetNumber attnum,
StrategyNumber strategy, int32 searchMode,
Datum query, uint32 nQueryValues,
Datum *queryValues, GinNullCategory *queryCategories,
- bool *partial_matches, Pointer *extra_data)
+ bool *partial_matches, void **extra_data)
{
GinScanKey key = &(so->keys[so->nkeys++]);
GinState *ginstate = &so->ginstate;
@@ -206,7 +206,7 @@ ginFillScanKey(GinScanOpaque so, OffsetNumber attnum,
Datum queryKey;
GinNullCategory queryCategory;
bool isPartialMatch;
- Pointer this_extra;
+ void *this_extra;
queryKey = queryValues[i];
queryCategory = queryCategories[i];
@@ -302,7 +302,7 @@ ginNewScanKey(IndexScanDesc scan)
Datum *queryValues;
int32 nQueryValues = 0;
bool *partial_matches = NULL;
- Pointer *extra_data = NULL;
+ void **extra_data = NULL;
bool *nullFlags = NULL;
GinNullCategory *categories;
int32 searchMode = GIN_SEARCH_MODE_DEFAULT;
diff --git a/src/backend/utils/adt/jsonb_gin.c b/src/backend/utils/adt/jsonb_gin.c
index 9b56248cf0b..670e1520e8e 100644
--- a/src/backend/utils/adt/jsonb_gin.c
+++ b/src/backend/utils/adt/jsonb_gin.c
@@ -746,7 +746,7 @@ emit_jsp_gin_entries(JsonPathGinNode *node, GinEntries *entries)
*/
static Datum *
extract_jsp_query(JsonPath *jp, StrategyNumber strat, bool pathOps,
- int32 *nentries, Pointer **extra_data)
+ int32 *nentries, void ***extra_data)
{
JsonPathGinContext cxt;
JsonPathItem root;
@@ -786,7 +786,7 @@ extract_jsp_query(JsonPath *jp, StrategyNumber strat, bool pathOps,
return NULL;
*extra_data = palloc0(sizeof(**extra_data) * entries.count);
- **extra_data = (Pointer) node;
+ **extra_data = node;
return entries.buf;
}
@@ -909,7 +909,7 @@ gin_extract_jsonb_query(PG_FUNCTION_ARGS)
strategy == JsonbJsonpathExistsStrategyNumber)
{
JsonPath *jp = PG_GETARG_JSONPATH_P(0);
- Pointer **extra_data = (Pointer **) PG_GETARG_POINTER(4);
+ void ***extra_data = (void ***) PG_GETARG_POINTER(4);
entries = extract_jsp_query(jp, strategy, false, nentries, extra_data);
@@ -934,7 +934,7 @@ gin_consistent_jsonb(PG_FUNCTION_ARGS)
/* Jsonb *query = PG_GETARG_JSONB_P(2); */
int32 nkeys = PG_GETARG_INT32(3);
- Pointer *extra_data = (Pointer *) PG_GETARG_POINTER(4);
+ void **extra_data = (void **) PG_GETARG_POINTER(4);
bool *recheck = (bool *) PG_GETARG_POINTER(5);
bool res = true;
int32 i;
@@ -999,8 +999,7 @@ gin_consistent_jsonb(PG_FUNCTION_ARGS)
if (nkeys > 0)
{
Assert(extra_data && extra_data[0]);
- res = execute_jsp_gin_node((JsonPathGinNode *) extra_data[0], check,
- false) != GIN_FALSE;
+ res = execute_jsp_gin_node(extra_data[0], check, false) != GIN_FALSE;
}
}
else
@@ -1017,7 +1016,7 @@ gin_triconsistent_jsonb(PG_FUNCTION_ARGS)
/* Jsonb *query = PG_GETARG_JSONB_P(2); */
int32 nkeys = PG_GETARG_INT32(3);
- Pointer *extra_data = (Pointer *) PG_GETARG_POINTER(4);
+ void **extra_data = (void **) PG_GETARG_POINTER(4);
GinTernaryValue res = GIN_MAYBE;
int32 i;
@@ -1060,8 +1059,7 @@ gin_triconsistent_jsonb(PG_FUNCTION_ARGS)
if (nkeys > 0)
{
Assert(extra_data && extra_data[0]);
- res = execute_jsp_gin_node((JsonPathGinNode *) extra_data[0], check,
- true);
+ res = execute_jsp_gin_node(extra_data[0], check, true);
/* Should always recheck the result */
if (res == GIN_TRUE)
@@ -1200,7 +1198,7 @@ gin_extract_jsonb_query_path(PG_FUNCTION_ARGS)
strategy == JsonbJsonpathExistsStrategyNumber)
{
JsonPath *jp = PG_GETARG_JSONPATH_P(0);
- Pointer **extra_data = (Pointer **) PG_GETARG_POINTER(4);
+ void ***extra_data = (void ***) PG_GETARG_POINTER(4);
entries = extract_jsp_query(jp, strategy, true, nentries, extra_data);
@@ -1224,7 +1222,7 @@ gin_consistent_jsonb_path(PG_FUNCTION_ARGS)
/* Jsonb *query = PG_GETARG_JSONB_P(2); */
int32 nkeys = PG_GETARG_INT32(3);
- Pointer *extra_data = (Pointer *) PG_GETARG_POINTER(4);
+ void **extra_data = (void **) PG_GETARG_POINTER(4);
bool *recheck = (bool *) PG_GETARG_POINTER(5);
bool res = true;
int32 i;
@@ -1258,8 +1256,7 @@ gin_consistent_jsonb_path(PG_FUNCTION_ARGS)
if (nkeys > 0)
{
Assert(extra_data && extra_data[0]);
- res = execute_jsp_gin_node((JsonPathGinNode *) extra_data[0], check,
- false) != GIN_FALSE;
+ res = execute_jsp_gin_node(extra_data[0], check, false) != GIN_FALSE;
}
}
else
@@ -1276,7 +1273,7 @@ gin_triconsistent_jsonb_path(PG_FUNCTION_ARGS)
/* Jsonb *query = PG_GETARG_JSONB_P(2); */
int32 nkeys = PG_GETARG_INT32(3);
- Pointer *extra_data = (Pointer *) PG_GETARG_POINTER(4);
+ void **extra_data = (void **) PG_GETARG_POINTER(4);
GinTernaryValue res = GIN_MAYBE;
int32 i;
@@ -1302,8 +1299,7 @@ gin_triconsistent_jsonb_path(PG_FUNCTION_ARGS)
if (nkeys > 0)
{
Assert(extra_data && extra_data[0]);
- res = execute_jsp_gin_node((JsonPathGinNode *) extra_data[0], check,
- true);
+ res = execute_jsp_gin_node(extra_data[0], check, true);
/* Should always recheck the result */
if (res == GIN_TRUE)
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 540aa9628d7..f97d50641cf 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -8249,7 +8249,7 @@ gincost_pattern(IndexOptInfo *index, int indexcol,
righttype;
int32 nentries = 0;
bool *partial_matches = NULL;
- Pointer *extra_data = NULL;
+ void **extra_data = NULL;
bool *nullFlags = NULL;
int32 searchMode = GIN_SEARCH_MODE_DEFAULT;
int32 i;
diff --git a/src/backend/utils/adt/tsginidx.c b/src/backend/utils/adt/tsginidx.c
index 2712fd89df0..4fe01d5a005 100644
--- a/src/backend/utils/adt/tsginidx.c
+++ b/src/backend/utils/adt/tsginidx.c
@@ -44,7 +44,7 @@ gin_cmp_prefix(PG_FUNCTION_ARGS)
#ifdef NOT_USED
StrategyNumber strategy = PG_GETARG_UINT16(2);
- Pointer extra_data = PG_GETARG_POINTER(3);
+ void *extra_data = (void *) PG_GETARG_POINTER(3);
#endif
int cmp;
@@ -98,7 +98,7 @@ gin_extract_tsquery(PG_FUNCTION_ARGS)
/* StrategyNumber strategy = PG_GETARG_UINT16(2); */
bool **ptr_partialmatch = (bool **) PG_GETARG_POINTER(3);
- Pointer **extra_data = (Pointer **) PG_GETARG_POINTER(4);
+ void ***extra_data = (void ***) PG_GETARG_POINTER(4);
/* bool **nullFlags = (bool **) PG_GETARG_POINTER(5); */
int32 *searchMode = (int32 *) PG_GETARG_POINTER(6);
@@ -141,7 +141,7 @@ gin_extract_tsquery(PG_FUNCTION_ARGS)
* same, entry's) number. Entry's number is used in check array in
* consistent method. We use the same map for each entry.
*/
- *extra_data = (Pointer *) palloc(sizeof(Pointer) * j);
+ *extra_data = palloc(sizeof(void *) * j);
map_item_operand = (int *) palloc0(sizeof(int) * query->size);
/* Now rescan the VAL items and fill in the arrays */
@@ -157,7 +157,7 @@ gin_extract_tsquery(PG_FUNCTION_ARGS)
val->length);
entries[j] = PointerGetDatum(txt);
partialmatch[j] = val->prefix;
- (*extra_data)[j] = (Pointer) map_item_operand;
+ (*extra_data)[j] = map_item_operand;
map_item_operand[i] = j;
j++;
}
@@ -219,7 +219,7 @@ gin_tsquery_consistent(PG_FUNCTION_ARGS)
TSQuery query = PG_GETARG_TSQUERY(2);
/* int32 nkeys = PG_GETARG_INT32(3); */
- Pointer *extra_data = (Pointer *) PG_GETARG_POINTER(4);
+ void **extra_data = (void **) PG_GETARG_POINTER(4);
bool *recheck = (bool *) PG_GETARG_POINTER(5);
bool res = false;
@@ -236,7 +236,7 @@ gin_tsquery_consistent(PG_FUNCTION_ARGS)
*/
gcv.first_item = GETQUERY(query);
gcv.check = (GinTernaryValue *) check;
- gcv.map_item_operand = (int *) (extra_data[0]);
+ gcv.map_item_operand = extra_data[0];
switch (TS_execute_ternary(GETQUERY(query),
&gcv,
@@ -268,7 +268,7 @@ gin_tsquery_triconsistent(PG_FUNCTION_ARGS)
TSQuery query = PG_GETARG_TSQUERY(2);
/* int32 nkeys = PG_GETARG_INT32(3); */
- Pointer *extra_data = (Pointer *) PG_GETARG_POINTER(4);
+ void **extra_data = (void **) PG_GETARG_POINTER(4);
GinTernaryValue res = GIN_FALSE;
if (query->size > 0)
@@ -281,7 +281,7 @@ gin_tsquery_triconsistent(PG_FUNCTION_ARGS)
*/
gcv.first_item = GETQUERY(query);
gcv.check = check;
- gcv.map_item_operand = (int *) (extra_data[0]);
+ gcv.map_item_operand = extra_data[0];
res = TS_execute_ternary(GETQUERY(query),
&gcv,
diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h
index db19ffd9897..ff42c41847c 100644
--- a/src/include/access/gin_private.h
+++ b/src/include/access/gin_private.h
@@ -214,7 +214,7 @@ extern void ginInsertValue(GinBtree btree, GinBtreeStack *stack,
/* ginentrypage.c */
extern IndexTuple GinFormTuple(GinState *ginstate,
OffsetNumber attnum, Datum key, GinNullCategory category,
- Pointer data, Size dataSize, int nipd, bool errorTooBig);
+ void *data, Size dataSize, int nipd, bool errorTooBig);
extern void ginPrepareEntryScan(GinBtree btree, OffsetNumber attnum,
Datum key, GinNullCategory category,
GinState *ginstate);
@@ -303,7 +303,7 @@ typedef struct GinScanKeyData
/* NB: these three arrays have only nuserentries elements! */
Datum *queryValues;
GinNullCategory *queryCategories;
- Pointer *extra_data;
+ void **extra_data;
StrategyNumber strategy;
int32 searchMode;
OffsetNumber attnum;
@@ -341,7 +341,7 @@ typedef struct GinScanEntryData
Datum queryKey;
GinNullCategory queryCategory;
bool isPartialMatch;
- Pointer extra_data;
+ void *extra_data;
StrategyNumber strategy;
int32 searchMode;
OffsetNumber attnum;
diff --git a/src/include/access/ginblock.h b/src/include/access/ginblock.h
index 4c1681068db..e7365c220cf 100644
--- a/src/include/access/ginblock.h
+++ b/src/include/access/ginblock.h
@@ -236,7 +236,7 @@ typedef signed char GinNullCategory;
#define GIN_ITUP_COMPRESSED (1U << 31)
#define GinGetPostingOffset(itup) (GinItemPointerGetBlockNumber(&(itup)->t_tid) & (~GIN_ITUP_COMPRESSED))
#define GinSetPostingOffset(itup,n) ItemPointerSetBlockNumber(&(itup)->t_tid,(n)|GIN_ITUP_COMPRESSED)
-#define GinGetPosting(itup) ((Pointer) ((char*)(itup) + GinGetPostingOffset(itup)))
+#define GinGetPosting(itup) ((void *) ((char*)(itup) + GinGetPostingOffset(itup)))
#define GinItupIsCompressed(itup) ((GinItemPointerGetBlockNumber(&(itup)->t_tid) & GIN_ITUP_COMPRESSED) != 0)
/*
--
2.52.0
On 24.11.2025 20:52, Robert Haas wrote: > On Mon, Nov 24, 2025 at 2:38 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Well, either Peter just ran out of energy or there is actually some >> notational value in Pointer. If it's the latter, I'd like to know. > > I agree that would be nice to know. > > Peter's original email seemed to indicate that he was deterred by this > sort of thing: > > Pointer **extra_data = (Pointer **) PG_GETARG_POINTER(4); > > If Pointer is merely char *, then this is equivalent to: > > char ***extra_data = (char ***) PG_GETARG_POINTER(4); > > I believe this is the same extra_data that is documented thus: I figured the same while doing the change. > So in other words, it's a pointer to an array of generic pointers. In > a vacuum, I'd suggest that having three levels of indirection that are > all semantically different but all denoted by an asterisk in C is > confusing enough to be a bad idea regardless of the specifics. But > since we've already crossed that bridge, we'll just need to make the > best of it. Maybe we could use a more specific typedef here, like > GinExtraPointer. That would be a lot more greppable than just writing > Pointer, and every GinExtraPointer would be the same flavor of generic > pointer, whereas any given Pointer is not necessarily related in any > semantic way to any other. I went with your proposal of GinExtraPointer. See attached patch. It's based on the series of patches from Peter's initial mail. I've included the removal of the Pointer typedef in the same patch. -- David Geier