On Wed, Feb 8, 2017 at 9:25 AM, Ashutosh Sharma <ashu.coe...@gmail.com> wrote: >>> 1) Check if an overflow page is a new page. If so, read a bitmap page >>> to confirm if a bit corresponding to this overflow page is clear or >>> not. For this, I would first add Assert statement to ensure that the >>> bit is clear and if it is, then set the statusbit as false indicating >>> that the page is free. >>> >>> 2) In case if an overflow page is not new, first verify if it is >>> really an overflow page and if so, check if the bit corresponding to >>> it in the bitmap page is SET. If so, set the statusbit as true; If >>> not, we would see an assertion failure happening. >> >> I think this is complicated and not what anybody wants. I think you >> should do exactly what I said above: return true if the bit is set in >> the bitmap, and false if it isn't. Full stop. Don't read or do >> anything with the underlying page. Only read the bitmap page. > > Okay, As per the inputs from you, I have modified hash_bitmap_info() > and have tried to keep the things simple. Attached is the patch that > has this changes. Please have a look and let me know if you feel it is > not yet in the right shape. Thanks.
Well, this changes the function signature and I don't see any advantage in doing that. Also, it still doesn't read the code that reads the overflow page. Any correct patch for this problem needs to include the following hunk: - buf = ReadBufferExtended(indexRel, MAIN_FORKNUM, (BlockNumber) ovflblkno, - RBM_NORMAL, NULL); - LockBuffer(buf, BUFFER_LOCK_SHARE); - _hash_checkpage(indexRel, buf, LH_PAGE_TYPE); - page = BufferGetPage(buf); - opaque = (HashPageOpaque) PageGetSpecialPointer(page); - - if (opaque->hasho_flag != LH_OVERFLOW_PAGE) - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("page is not an overflow page"), - errdetail("Expected %08x, got %08x.", - LH_OVERFLOW_PAGE, opaque->hasho_flag))); - - if (BlockNumberIsValid(opaque->hasho_prevblkno)) - bit = true; - - UnlockReleaseBuffer(buf); And then, instead, you need to add some code to set bit based on the bitmap page, like what you have: + mapbuf = _hash_getbuf(indexRel, bitmapblkno, HASH_READ, LH_BITMAP_PAGE); + mappage = BufferGetPage(mapbuf); + freep = HashPageGetBitmap(mappage); + + if (ISSET(freep, bitmapbit)) + bit = 1; Except I would write that last line as... bit = ISSET(freep, bitmapbit) != 0 ...instead of using an if statement. I'm sort of confused as to why the idea of not reading the underlying page is so hard to understand. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers