Re: hash index improving v3

I performed code review and see my comments.

pgsql/src/backend/access/hash/hashpage.c
<http://reviewdemo.postgresql.org/r/26/#comment31>

use sizeof() or something better the 4.

pgsql/src/backend/access/hash/hashpage.c
<http://reviewdemo.postgresql.org/r/26/#comment32>

New empty line.

pgsql/src/backend/access/hash/hashutil.c
<http://reviewdemo.postgresql.org/r/26/#comment27>

It would be better remove #define from hash.h and setup it there
directly.

pgsql/src/backend/access/hash/hashutil.c
<http://reviewdemo.postgresql.org/r/26/#comment28>

Why not return directly uint32?

pgsql/src/backend/access/hash/hashutil.c
<http://reviewdemo.postgresql.org/r/26/#comment22>

Retype to correct return type.

pgsql/src/backend/access/hash/hashutil.c
<http://reviewdemo.postgresql.org/r/26/#comment29>

Whats about null values?

pgsql/src/backend/access/hash/hashutil.c
<http://reviewdemo.postgresql.org/r/26/#comment30>

I'm not sure if values modification is safe. Please, recheck.

pgsql/src/backend/access/hash/hashutil.c
<http://reviewdemo.postgresql.org/r/26/#comment23>

Return value is not much clear. I prefer to return InvalidOffset
when no record is found. However it seems that you use result also for
PageAddItem to put item on correct ordered position. I think better
explanation should help to understand how it works.

pgsql/src/backend/access/hash/hashutil.c
<http://reviewdemo.postgresql.org/r/26/#comment26>

It could return FirstOffset number in case when nothing interesting
is on the page.

pgsql/src/include/access/hash.h
<http://reviewdemo.postgresql.org/r/26/#comment34>

Why not define new datatype for example HashKey instead of uint32?

pgsql/src/include/access/hash.h
<http://reviewdemo.postgresql.org/r/26/#comment33>

It is not good place. See my other comment.

--------------
You also forgot to bump hash index version in meta page.

Zdenek