On Tue, Sep 20, 2016 at 8:34 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Pavan Deolasee <pavan.deolasee@gmail.com> writes: > I happened to notice this comment in src/include/storage/itemptr.h
> * Note: because there is an item pointer in each tuple header and index > * tuple header on disk, it's very important not to waste space with > * structure padding bytes. The struct is designed to be six bytes long > * (it contains three int16 fields) but a few compilers will pad it to > * eight bytes unless coerced. We apply appropriate persuasion where > * possible, and to cope with unpersuadable compilers, we try to use > * "SizeOfIptrData" rather than "sizeof(ItemPointerData)" when computing > * on-disk sizes. > */
> Is that now obsolete?
Realistically, because struct HeapTupleHeaderData contains a field of type ItemPointerData, it's probably silly to imagine that we can save anything if the compiler can't be persuaded to believe that sizeof(ItemPointerData) is 6. It may well be that the structure pragmas work on everything that wouldn't natively believe that anyway.
Yeah, that's what I thought and rest of the code seems to make that assumption as well. Attached patch removes the last remaining reference to SizeOfIptrData and also removes the macro and the associated comment. TBH I couldn't fully trace how the TID array gets generated in nodeTidscan.c, but I'm fairly confident that this should not break anything because this was the only remaining reference.
While htup.h refactoring happened in 9.5, I don't see any point in back patching this.
