Horton

Thread

Commits

  1. Add some const qualifications

  2. Remove Item type

  3. Add type cast to foreach_internal's loop variable.

  1. Remove Item type

    Peter Eisentraut <peter@eisentraut.org> — 2025-09-29T10:20:00Z

    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, ...)

  2. Re: Remove Item type

    Nathan Bossart <nathandbossart@gmail.com> — 2025-10-24T15:32:15Z

    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

  3. Re: Remove Item type

    Peter Geoghegan <pg@bowt.ie> — 2025-10-25T00:48:11Z

    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

  4. Re: Remove Item type

    Peter Eisentraut <peter@eisentraut.org> — 2025-10-27T09:21:48Z

    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.