On Thu, Mar 23, 2017 at 2:35 PM, Ashutosh Sharma <ashu.coe...@gmail.com> wrote: > I take your suggestion. Please find the attachments.
I think you should consider refactoring this so that it doesn't need to use goto. Maybe move the while (offnum <= maxoff) logic into a helper function and have it return itemIndex. If itemIndex == 0, you can call it again. Notice that the code in the "else" branch of the if (itemIndex == 0) stuff could actually be removed from the else block without changing anything, because the "if" block always either returns or does a goto. That's worthy of a little more work to try to make things simple and clear. + * We find the first item(or, if backward scan, the last item) in Missing space. In _hash_dropscanbuf(), the handling of hashso_bucket_buf is now inconsistent with the handling of hashso_split_bucket_buf, which seems suspicious. Suppose we enter this function with so->hashso_bucket_buf and so->currPos.buf both being valid buffers, but not the same one. It looks to me as if we'll release the first pin but not the second one. My guess (which could be wrong) is that so->hashso_bucket_buf = InvalidBuffer should be moved back up higher in the function where it was before, just after the first if statement, and that the new condition so->hashso_bucket_buf == so->currPos.buf on the last _hash_dropbuf() should be removed. If that's not right, then I think I need somebody to explain why not. I am suspicious that _hash_kill_items() is going to have problems if the overflow page is freed before it reacquires the lock. _btkillitems() contains safeguards against similar cases. This is not a full review, but I'm out of time for the moment. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers