Re: pgsql 10: hash indexes testing

On Sat, Aug 5, 2017 at 7:50 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Fri, Aug 4, 2017 at 2:45 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> I have not done anything for this comment as it doesn't sound wrong to
>> me.  I think it is not making much sense in the current code and we
>> can remove it or change it as part of the separate patch if you or
>> others think so.
>
> I don't get it.  The comment claims we have to _hash_getnewbuf before
> releasing the metapage write lock.  But the function neither calls
> _hash_getnewbuf nor releases the metapage write lock.

Both the actions _hash_getnewbuf and release of the metapage lock are
done in the caller (_hash_expandtable).

>  It then goes on
> to say that for this reason we pass the new buffer rather than just
> the block number.  However, even if it were true that we have to call
> _hash_getnewbuf before releasing the metapage write lock, what does
> that have to do with the choice of passing a buffer vs. a block
> number?
>

For this, you have to look at PG9.6 code.  In PG9.6, we were passing
old bucket's block number and new bucket's buffer and the reason is
that in the caller (_hash_expandtable) we only have access to a buffer
of new bucket block.

> Explain to me what I'm missing, please, because I'm completely befuddled!
>

I think you need to compare the code of PG10 with PG9.6 for functions
_hash_splitbucket and _hash_expandtable.  I don't find this comment
much useful starting PG10.  Patch attached to remove it.


> (On another note, I committed these patches.)
>

Thanks.



-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com