Re: pageinspect: Hash index support

On Sat, Jan 28, 2017 at 9:09 PM, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)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