> > + 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.
I think we had added 'ptr' variable initially just to dump hash code in hexadecimal format but now since we have removed that logic from current code, I think it is no more required and I have therefore removed it from the current patch. Below is the code that used it initially. It got changed as per your comment - [1] > > + 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. okay, agreed. I have corrected this in the attached v10 patch. > > 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. > I think it's OK to check hash_bitmap_info() or any other functions with different page types at least once. [1]- https://www.postgresql.org/message-id/CA%2BTgmoZUjrVy52TUU3b_Q5L6jcrt7w5R4qFwMXdeCuKQBmYWqQ%40mail.gmail.com -- With Regards, Ashutosh Sharma EnterpriseDB:http://www.enterprisedb.com
0001-Add-support-for-hash-index-in-pageinspect-contrib-mo_v10.patch
Description: invalid/octet-stream
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
