Re: pageinspect: Hash index support

On Wed, Sep 21, 2016 at 3:25 AM, Jesper Pedersen
<jesper.pedersen@redhat.com> wrote:
> On 09/20/2016 12:45 PM, Jeff Janes wrote:
>> 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.
>>
>
> Adjusted in v4.

Thanks for the new version.

> Code/doc will need an update once the CHI patch goes in.

If you are referring to the WAL support, I guess that this patch or
the other patch could just rebase and adjust as needed.

hash_page_items and hash_page_stats share a lot of common points with
their btree equivalents, still doing a refactoring would just impact
the visibility of the code, and it is wanted as educational in this
module, so let's go with things the way you suggest.

+     <para>
+      The type information will be '<literal>m</literal>' for a metadata page,
+      '<literal>v</literal>' for an overflow page,
'<literal>b</literal>' for a bucket page,
+      '<literal>i</literal>' for a bitmap page, and
'<literal>u</literal>' for an unused page.
+     </para>
Other functions don't go into this level of details, so I would just
rip out this paragraph.

The patch looks in pretty good shape to me, so I am switching it as
ready for committer.
-- 
Michael