On Wed, Feb 15, 2023 at 3:35 AM Nathan Bossart <nathandboss...@gmail.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.
