get rid of Pointer type, mostly

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 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]: /messages/by-id/CA+hUKG+NFKnr=K4oybwDvT35dW=VAjAAfiuLxp+5JeZSOV3nBg@mail.gmail.com&lt;0001-Remove-useless-casts-to-Pointer.patch&gt;&lt;0002-Use-better-DatumGet-function.patch&gt;&lt;0003-Don-t-rely-on-pointer-arithmetic-with-Pointer-type.patch&gt;&lt;0004-Change-Pointer-to-void.patch&gt;&lt;0005-Remove-no-longer-needed-casts-to-Pointer.patch&gt;&lt;0006-Remove-some-uses-of-the-Pointer-type.patch&gt;

0001 - Removed type cast from memcpy(), which should be safe, as memcpy doesn’t care about types of source and dest pointers referring 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 of Pointer.

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, so these 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 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.

+1.

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.

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

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

Hi Peter,

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.

It seems to me that we reached agreement. Are you planning to still
apply these patches?

--
David Geier

On Dec 8, 2025, at 18:25, David Geier <geidav.pg@gmail.com> wrote:

Hi Peter,

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.

It seems to me that we reached agreement. Are you planning to still
apply these patches?

Basically I am not against this patch, as 756a43689324b473ee07549a6eb7a53a203df5ad has done similar changes.

What I want to understand is that why do we delete Pointer and add GinExtraPointer?

```
-/*
- * Pointer
- * Variable holding address of any memory resident object.
- * (obsolescent; use void * or char *)
- */
-typedef void *Pointer;
```

And
```
+typedef void *GinExtraPointer;
```

They both are underlying “void *”. Are we expecting to improve code readability? More specific maybe?

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