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)
forexample).
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
toXLOG records) fields when they are not?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com