Re: Change xl_hash_vacuum_one_page.ntuples from int to uint16

From: "Drouvot, Bertrand" <bertranddrouvot(dot)pg(at)gmail(dot)com>
To: Nathan Bossart <nathandbossart(at)gmail(dot)com>
Cc: 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-01-21 05:42:08
Message-ID: 332d055a-f167-a39a-8cb7-931fcf8cbe20@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 1/20/23 9:01 PM, Nathan Bossart wrote:
> On Tue, Jan 10, 2023 at 11:08:33AM +0100, Drouvot, Bertrand wrote:
>> While working on [1], I noticed that xl_hash_vacuum_one_page.ntuples is an int.
>>
>> Unless I'm missing something, It seems to me that it would make more sense to use an uint16 (like this is done for
>> gistxlogDelete.ntodelete for example).
>
> I think that is correct. This value is determined by looping through
> offsets, which are uint16 as well.

Thanks for the review!

> 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?

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Drouvot, Bertrand 2023-01-21 05:42:51 Re: Split index and table statistics into different types of stats
Previous Message Andres Freund 2023-01-21 05:31:57 Re: Reduce timing overhead of EXPLAIN ANALYZE using rdtsc?