Re: pageinspect: Hash index support

On 9/21/16 9:30 AM, Jesper Pedersen wrote:
> Attached is v5, which add basic page verification.

There are still some things that I found that appear to follow the old
style (btree) conventions instead the new style (brin, gin) conventions.

- Rename hash_metap to hash_metapage_info.

- Don't use BuildTupleFromCStrings(). (There is nothing inherently
wrong with it, but why convert numbers to strings and back again when it
can be done more directly.)

- Documentation for hash_page_stats still has blkno argument.

- In hash_metap, the magic field could be type bytea, so that it
displays in hex. Or text like the brin functions.

Some other comments:

- hash_metap result fields spares and mapp should be arrays of integer.

(Incidentally, the comment in hash.h talks about bitmaps[] but I think
it means hashm_mapp[].)

- As of very recently, we don't need to move pageinspect--1.5.sql to
pageinspect--1.6.sql anymore. Just add pageinspect--1.5--1.6.sql.

- In the documentation for hash_page_stats(), the "+" sign is misaligned.

- In hash_page_items, the fields itemlen, nulls, vars are always 16,
false, false. So perhaps there is no need for them. Similarly, the
hash_page_stats in hash_page_stats is always 16.

- The data field could be of type bytea.

- Add a pointer in the documentation to the relevant header file.

Bonus:

- Add subsections in the documentation (general functions, B-tree
functions, etc.)

- Add tests.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services