Re: Change xl_hash_vacuum_one_page.ntuples from int to uint16

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Nathan Bossart <nathandbossart(at)gmail(dot)com>
Cc: "Drouvot, Bertrand" <bertranddrouvot(dot)pg(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Change xl_hash_vacuum_one_page.ntuples from int to uint16
Date: 2023-02-16 11:00:09
Message-ID: CAA4eK1Kea0DmA0BwFQtvQhLAUMPpoVtHWczBZ6ui8toHWEmHjA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Feb 15, 2023 at 3:35 AM Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
>
> On Sat, Jan 21, 2023 at 06:42:08AM +0100, Drouvot, Bertrand wrote:
> > On 1/20/23 9:01 PM, Nathan Bossart wrote:
> >> Should we also change the related
> >> variables (e.g., ndeletable in _hash_vacuum_one_page()) to uint16?
> >
> > Yeah, I thought about it too. What I saw is that there is other places that would be good candidates for the same
> > kind of changes (see the int ntodelete argument in gistXLogDelete() being assigned to gistxlogDelete.ntodelete (uint16) for example).
> >
> > So, what do you think about:
> >
> > 1) keep this patch as it is (to "only" address the struct field and avoid possible future "useless" padding size increase)
> > and
> > 2) create a new patch (once this one is committed) to align the types for variables/arguments with the structs (related to XLOG records) fields when they are not?
>
> Okay. I've marked this one as ready-for-committer, then.
>

LGTM. I think the padding space we are trying to save here can be used
for the patch [1], right? BTW, feel free to create the second patch
(to align the types for variables/arguments) as that would be really
helpful after we commit this one.

I think this would require XLOG_PAGE_MAGIC as it changes the WAL record.

BTW, how about a commit message like:
Change xl_hash_vacuum_one_page.ntuples from int to uint16.

This will create two bytes of padding space in xl_hash_vacuum_one_page
which can be used for future patches. This makes the datatype of
xl_hash_vacuum_one_page.ntuples same as gistxlogDelete.ntodelete which
is advisable as both are used for the same purpose.

[1] - https://www.postgresql.org/message-id/2d62f212-fce6-d639-b9eb-2a5bc4bec3b4%40gmail.com

--
With Regards,
Amit Kapila.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message John Naylor 2023-02-16 11:28:23 Re: Todo: Teach planner to evaluate multiple windows in the optimal order
Previous Message Laurenz Albe 2023-02-16 10:32:16 Re: Move defaults toward ICU in 16?