On Mon, Jun 20, 2016 at 7:28 AM, Asif Naeem <anaeem...@gmail.com> wrote: > Thank you for useful suggestions. PFA patch, I have tried to cover all the > points mentioned.
Thanks for the new patch. I think that you have failed to address this point from my previous review: # I see why you changed the calling convention for visibilitymap_pin() # and RecordPageWithFreeSpace(), but that's awfully invasive. I wonder # if there's a better way to do that, like maybe having vacuumlazy.c ask # the VM and FSM for their length in pages and then not trying to use # those functions for block numbers that are too large. The patch has gotten a lot smaller, and that's clearly good, but introducing extended versions of those functions still seems like a thing we should try to avoid. In particular, there's no way this hunk is going to be acceptable: @@ -286,6 +299,10 @@ visibilitymap_set(Relation rel, BlockNumber heapBlk, Buffer heapBuf, if (BufferIsValid(heapBuf) && BufferGetBlockNumber(heapBuf) != heapBlk) elog(ERROR, "wrong heap buffer passed to visibilitymap_set"); + /* In case of invalid buffer just return */ + if(vmBuf == InvalidBuffer) + return; + /* Check that we have the right VM page pinned */ if (!BufferIsValid(vmBuf) || BufferGetBlockNumber(vmBuf) != mapBlock) elog(ERROR, "wrong VM buffer passed to visibilitymap_set"); You're going to have to find a different approach there. -- 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