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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to