Re: pageinspect: Hash index support

On Fri, Feb 3, 2017 at 11:49 AM, Jesper Pedersen
<jesper.pedersen@redhat.com> wrote:
> Disregard, as Tom has committed a fix.

So we're six commits into this mess now and I'm hopeful that we've got
most of the problems with type selection and crashing fixed now.
However, I discovered another thing that doesn't make sense to me --
and I admit this is another thing I should have noticed before
committing, but better late than never.

As far as I can tell, the hash_bitmap_info() function is doing
something completely ridiculous.  One would expect that the purpose of
this function was to tell you about the status of pages in the bitmap.
The documentation claims that this is what the function does: it
claims that this function "shows the status of a bit in the bitmap
page for a particular overflow page".  So you would think that what
the function would do is:

1. Work out which bitmap page contains the bit for the page number in question.
2. Read that bitmap page.
3. Indicate the status of that bit within that page.

However, that's not what the function actually does.  Instead, it does this:

1. Go examine the overflow page and see whether hasho_prevblkno.  If
so, claim that the bit is set in the bitmap; if not, claim that it
isn't.
2. Work out which bitmap page contains the bit for the page number in question.
3. But don't look at it.  Instead, tell the user which bitmap page and
bit you would have looked at, but instead of returning the status of
that bit, return the value you computed in step 1.

I do not think this can be the right approach.

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