Re: pageinspect: Hash index support

On Tue, Sep 20, 2016 at 5:40 AM, Jesper Pedersen <jesper(dot)pedersen(at)redhat(dot)com
> wrote:

> On 09/20/2016 03:19 AM, Michael Paquier wrote:
>
>> You did not get right the comments from Alvaro upthread. The following
>> functions are added with this patch:
>> function hash_metap(text)
>> function hash_metap_bytea(bytea)
>> function hash_page_items(text,integer)
>> function hash_page_items_bytea(bytea)
>> function hash_page_stats(text,integer)
>> function hash_page_stats_bytea(bytea,integer)
>>
>> Now the following set of functions would be sufficient:
>> function hash_metapage_info(bytea)
>> function hash_page_items(bytea)
>> function hash_page_stats(bytea)
>> The last time pageinspect has been updated, when BRIN functions have
>> been added, it has been discussed to just use (bytea) as an argument
>> interface and just rely on get_raw_page() to get the pages wanted, so
>> I think that we had better stick with that and keep things simple.
>>
>>
> Yes, I know, Alvaro and you voted for the bytea methods, and Jeff asked
> for both.
>
> Attached is v3 with only the bytea based methods.
>
> Alvaro, Michael and Jeff - Thanks for the review !
>

Is the 2nd "1" in this call needed?

SELECT * FROM hash_page_stats(get_raw_page('mytab_index', 1), 1)

As far as I can tell, that argument is only used to stuff into the output
field "blkno", it is not used to instruct the interpretation of the raw
page itself. It doesn't seem worthwhile to have the parameter that only
echos back to the user what the user already put in (twice). The only
existing funtions which take the blkno argument are those that don't use
the get_raw_page style.

Also, should we document what the single letter values mean in the
hash_page_stats.type column? It is not obvious that 'i' means bitmap, for
example.

Cheers,

Jeff