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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers