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


Reply via email to