Re: pageinspect and hash indexes

On Fri, Mar 24, 2017 at 3:54 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Fri, Mar 24, 2017 at 9:50 AM, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
>> On Fri, Mar 24, 2017 at 9:46 AM, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
>>>
>>> Thanks for reviewing my patch. I have removed the extra white space.
>>> Attached are both the patches.
>>
>> Sorry, I have mistakenly attached wrong patch. Here are the correct
>> set of patches.
>
> The latest version of patches looks fine to me.

I don't really like 0002. What about this, instead?

--- a/contrib/pageinspect/hashfuncs.c
+++ b/contrib/pageinspect/hashfuncs.c
@@ -80,7 +80,8 @@ verify_hash_page(bytea *raw_page, int flags)
/* Check that page type is sane. */
pagetype = pageopaque->hasho_flag & LH_PAGE_TYPE;
if (pagetype != LH_OVERFLOW_PAGE && pagetype != LH_BUCKET_PAGE &&
- pagetype != LH_BITMAP_PAGE && pagetype != LH_META_PAGE)
+ pagetype != LH_BITMAP_PAGE && pagetype != LH_META_PAGE &&
+ pagetype != LH_UNUSED_PAGE)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("invalid hash page type %08x", pagetype)));

The advantage of that is (1) it won't get confused if in the future we
have an unused page that has some flag bit not in LH_PAGE_TYPE set,
and (2) if in the future we want to add any other checks to this
function which should apply to unused pages also, they won't get
bypassed by an early return statement.

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