Remove Item type

Started by Peter Eisentraut7 months ago4 messageshackers
Jump to latest
#1Peter Eisentraut
peter_e@gmx.net

Continuing the theme of cleaning up old and weird stuff in the code, I
propose to remove the Item type. It's defined as

typedef Pointer Item;

(note that Pointer is char *), and its almost only use is as part of
PageAddItem*():

extern OffsetNumber PageAddItemExtended(Page page, Item item, Size size,
OffsetNumber offsetNumber, int flags);

The actual thing passed to PageAddItem*() is of some AM-specific type,
and so that forces casts in all calls, like

offnum = PageAddItem(page, (Item) tuple, tuplen, offnum, true, false);

So this type doesn't promote any type safety, but in fact sort of forces
the opposite.

And then we have the usual problem that this is a type that hides
pointerness, and so we can't add any qualifiers, which leads to some
unconstify nonsense.

(Also, Item is unrelated to ItemPointer, which seems confusing.)

Initially I considered creating some underlying type to point to like
ItemData, but that didn't seem useful. Ultimately, PageAddItem*() is
just a fancy memcpy(), and using void * is an idiomatic way to refer to
a generic chunk of memory. So I'm proposing to remove the Item type,
replace it with void * in function prototypes, and remove all the casts.

Extension authors can make their code backward compatible by replacing

PageAddItem(page, (Item) tuple, ...)

with

PageAddItem(page, (void *) tuple, ...)

Attachments:

0001-Remove-Item-type.patchtext/plain; charset=UTF-8; name=0001-Remove-Item-type.patchDownload+102-178
0002-Some-const-qualifications.patchtext/plain; charset=UTF-8; name=0002-Some-const-qualifications.patchDownload+7-8
#2Nathan Bossart
nathandbossart@gmail.com
In reply to: Peter Eisentraut (#1)
Re: Remove Item type

On Mon, Sep 29, 2025 at 12:20:00PM +0200, Peter Eisentraut wrote:

So I'm proposing to remove the Item type, replace it with void * in
function prototypes, and remove all the casts.

The general idea seems reasonable to me, but I'm a little concerned that
using "void *" could break extensions written in C++ (see commit d5ca15e).
I haven't confirmed there's an actual issue here, though.

--
nathan

In reply to: Peter Eisentraut (#1)
Re: Remove Item type

On Mon, Sep 29, 2025 at 6:20 AM Peter Eisentraut <peter@eisentraut.org> wrote:

Continuing the theme of cleaning up old and weird stuff in the code, I
propose to remove the Item type.

+1

--
Peter Geoghegan

#4Peter Eisentraut
peter_e@gmx.net
In reply to: Nathan Bossart (#2)
Re: Remove Item type

On 24.10.25 17:32, Nathan Bossart wrote:

On Mon, Sep 29, 2025 at 12:20:00PM +0200, Peter Eisentraut wrote:

So I'm proposing to remove the Item type, replace it with void * in
function prototypes, and remove all the casts.

The general idea seems reasonable to me, but I'm a little concerned that
using "void *" could break extensions written in C++ (see commit d5ca15e).
I haven't confirmed there's an actual issue here, though.

Thanks for checking. This is not a problem, because what C++ disallows
is implicit conversion from void * to another pointer type, but it does
allow it the other way around, which is what would be happening here.

I have committed this.