ср, 7 апр. 2021 г. в 17:55, Bharath Rupireddy < bharath.rupireddyforpostg...@gmail.com>:
> 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). > Its maxalign is ensured by its size of 24bytes (which is maxalign'ed). I think if we change this to not-maxalign'ed value bad things can happen. So I've added assert checking for this value. I think it is similar situation for both page header and page special, I wonder why they've been treated differently in PageInit. > > 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? > Alike for page header, it is ensured by the current size of page special in all access methods now (except the size of sequence_magic, which I've corrected in the call). If someone wants to break this in the future, there is an added assert checking in PageInit. I think we should not maxalign both SizeOfPageHeaderData and specialSize manually, just check they have the right (already maxalign'ed) length to be safe in the future. -- Best regards, Pavel Borisov Postgres Professional: http://postgrespro.com <http://www.postgrespro.com>
