Re: [HACKERS] pageinspect: Hash index support

On Fri, Jan 13, 2017 at 12:49 AM, Jesper Pedersen
<jesper.pedersen@redhat.com> wrote:
> Rebased, and removed the compile warn in hashfuncs.c

I did some testing and review for the patch. I did not see any major
issue, but there are few minor cases for which I have few suggestions.

1. Source File :  /doc/src/sgml/pageinspect.sgml
example given.
SELECT * FROM hash_page_type(get_raw_page('con_hash_index', 0));
can be changed to
SELECT hash_page_type(get_raw_page('con_hash_index', 0));

2. @verify_hash_page
I can easily produce this error right after the split, so there will
be pages which are not inited before it is used. So an error saying it
is unexpected might be slightly misleading. I think error message
should be improved.
postgres=# SELECT hash_page_type(get_raw_page('i1', 12));
ERROR:  index table contains unexpected zero page

3. @verify_hash_page

(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("page is not a HASH page"),
+ errdetail("Expected %08x, got %08x.",

In error message "HASH" can downcase to "hash". That makes error
messages consistent with other error messages like "page is not a hash
page of expected type"


4. The condition is raw_page_size != BLCKSZ; But error msg "input page
too small"; I think error message should be changed to "invalid page
size".

if (raw_page_size != BLCKSZ)
+ ereport(ERROR,i+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("input page too small"),
+ errdetail("Expected size %d, got %d",
+   BLCKSZ, raw_page_size)));


5. @hash_bitmap_info
Metabuf can be released once after bitmapblkno is set it is off no use.
_hash_relbuf(indexRel, metabuf);

is Write lock required here for bitmap page?
mapbuf = _hash_getbuf(indexRel, bitmapblkno, HASH_WRITE, LH_BITMAP_PAGE);


6. You have renamed convert_ovflblkno_to_bitno to
_hash_ovflblkno_to_bitno. But unfortunately, you also moved the
function down. The diff appears as if you have completely replaced it.
I think we should put it back to at old place.

7. I think it is not your bug, but probably a bug in Hash index
itself; page flag is set to 66 (for below test); So the page is both
LH_BUCKET_PAGE and LH_BITMAP_PAGE. Is not this a bug in hash index?

I have inserted 3000 records. Hash index is on integer column.
select hasho_flag FROM hash_page_stats(get_raw_page('i1', 1));hasho_flag
------------        66

-- 
Thanks and Regards
Mithun C Y
EnterpriseDB: http://www.enterprisedb.com