Re: pageinspect: Hash index support

On Sat, Jan 28, 2017 at 9:09 PM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
> okay. Thanks. I have done changes on top of this patch.

+               ptr = (char *) itup + IndexInfoFindDataOffset(itup->t_info);
+               Assert(ptr <= uargs->page + BLCKSZ);

I think this should be promoted to an ereport(); these functions can
accept an arbitrary bytea.

+       if (opaque->hasho_flag & LH_BUCKET_PAGE)
+               stat->hasho_prevblkno = InvalidBlockNumber;
+       else
+               stat->hasho_prevblkno = opaque->hasho_prevblkno;

I think we should return the raw value here.  Mithun's patch to cache
the metapage hasn't gone in yet, but even if it does, let's assume
anyone using contrib/pageinspect wants to see the data that's
physically present, not our gloss on it.

Other than that, I don't think I have any other comments on this.  The
tests that got added look a little verbose to me (do we really need to
check pages 1-4 separately in each case when they're all hash pages?
if hash_bitmap_info rejects one, surely it will reject the others) but
I'm not going to fight too hard if Peter wants it that way.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company