On Wed, Apr 7, 2021 at 5:32 PM Pavel Borisov <pashkin.e...@gmail.com> wrote: > > I was looking at changes in Sp-Gist by commit > 4c0239cb7a7775e3183cb575e62703d71bf3302d > (discussion > https://postgr.es/m/CALj2ACViOo2qyaPT7krWm4LRyRTw9kOXt+g6PfNmYuGA=yh...@mail.gmail.com > ) and realized that during PageInit, both page header and page special are > expected to be maxaligned but in reality, their treatment is quite different:
How can we say that in PageInit the SizeOfPageHeaderData is expected to be max aligned? Am I missing something? There are lots of other places where SizeOfPageHeaderData is used, not MAXALIGN(SizeOfPageHeaderData). > 1. page special size is silently enforced to be maxaligned by PageInit() even > if caller-specified specialSize is not of a maxalign'ed size. > 2. page header size alignment is not checked at all (but we expect it > maxalign'ed, yes). > > I'd propose do both things in the same way: just Assert both sizes are > maxalign'ed during page init. > > I dived further and it appears that the only caller, who provides not > properly aligned page special is fill_seq_with_data() and corrected it. > > I am really convinced, that _callers_ should care about proper special size. > So now PageInit() just checks the right lengths of page special and page > header with assert, not enforcing size change silently. PFA my small patch on > this. I'd propose it to commit if in the HEAD only likewise the commit > 4c0239cb7a7775e3183cb575e62703d71bf3302d. > > What do you think? I still feel that for special size let callers call PageInit with sizeof(special_structure) and PageInit do the alignment. Others may have different opinion. On the patch itself, how can we say that other special sizes are max aligned except sequence_magic structure? With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com
