Re: Setting pd_lower in GIN metapage

On 2017/09/10 15:22, Michael Paquier wrote:
> On Sun, Sep 10, 2017 at 3:15 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> On Sat, Sep 9, 2017 at 9:00 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
>>>> On Fri, Sep 8, 2017 at 5:24 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>>> In short, this patch needs a significant rewrite, and more analysis than
>>>>> you've done so far on whether there's actually any benefit to be gained.
>>>>> It might not be worth messing with.
>>>
>>>> I did some measurements of the compressibility of the GIN meta page,
>>>> looking at its FPWs with and without wal_compression and you are
>>>> right: there is no direct compressibility effect when setting pd_lower
>>>> on the meta page. However, it seems to me that there is an argument
>>>> still pleading on favor of this patch for wal_consistency_checking.
>>>
>>> I think that would be true if we did both my point 1 and 2, so that
>>> the wal replay functions could trust pd_lower to be sane in all cases.
>>> But really, if you have to touch all the places that write these
>>> metapages, you might as well mark them REGBUF_STANDARD while at it.
>>>
>>>> The same comment ought to be mentioned for btree.
>>>
>>> Yeah, I was wondering if we ought not clean up btree/hash while at it.
>>> At the very least, their existing comments saying that it's inessential
>>> to set pd_lower could use some more detail about why or why not.
>>>
>>
>> +1. I think we can even use REGBUF_STANDARD in the hash for metapage
>> where currently it is not used. I can give a try to write a patch for
>> hash/btree part if you want.
>
> Coordinating efforts here would be nice. If you, Amit K, are taking
> care of a patch for btree and hash, would you, Amit L, write the part
> for GIN, BRIN and SpGist? This needs a careful lookup as many code
> paths need a lookup so it may take time. Please note that I don't mind
> double-checking this part if you don't have enough room to do so.

Sorry, I didn't have time today to carefully go through the recent
discussion on this thread (starting with Tom's email wherein he said he
set the status of the patch to Waiting on Author). I will try tomorrow.

Thanks,
Amit