On Sat, Aug 5, 2017 at 7:50 AM, Robert Haas <robertmh...@gmail.com> wrote: > On Fri, Aug 4, 2017 at 2:45 PM, Amit Kapila <amit.kapil...@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
remove_unnecessary_comment_hash_v1.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
