Re: Setting pd_lower in GIN metapage

On Mon, Sep 18, 2017 at 4:03 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Mon, Sep 18, 2017 at 11:16 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> On Sat, Sep 16, 2017 at 7:15 PM, Michael Paquier
>> <michael(dot)paquier(at)gmail(dot)com> wrote:
>>>
>>
>> You have already noticed above that it will help when
>> wal_checking_consistency is used and that was the main motivation to
>> pass REGBUF_STANDARD apart from maintaining consistency. It is not
>> clear to me what is bothering you. If your only worry about these
>> patches is that you want this sentence to be removed from the comment
>> because you think it is obvious or doesn't make much sense, then I
>> think we can leave this decision to committer. I have added it based
>> on Tom's suggestion above thread about explaining why it is
>> inessential or essential to set pd_lower. I think Amit Langote just
>> tried to mimic what I have done in hash and btree patches to maintain
>> consistency. I am also not very sure if we should write some detailed
>> comment or leave the existing comment as it is. I think it is just a
>> matter of different perspective.
>
> What is disturbing me a bit is that the existing comments mention
> something that could be supported (the compression of pages), but
> that's actually not done and this is unlikely to happen because the
> number of bytes associated to a meta page is going to be always
> cheaper than a FPW, which would cost in CPU to store it for
> compression is enabled. So I think that we should switch comments to
> mention that pd_lower is set so as this helps with page masking, but
> we don't take advantage of XLOG compression in the code.
>

I think that is not true because we do need FPW for certain usages of
metapage. Consider a case in _hash_doinsert where register metabuf
with just
REGBUF_STANDARD, it can take advantage of removing the hole if
pd_lower is set to its correct position. There are other similar
usages in hash index. For other indexes like btree, there is no such
usage currently, but it can also take advantage for
wal_consistency_checking. Now, probably there is an argument that we
use different comments for different indexes as the usage varies, but
I think someone looking at code after reading the comments can
differentiate such cases.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com