Re: BufferAccessStrategy for bulk insert

"Robert Haas" <robertmhaas(at)gmail(dot)com> writes:
> Patch resnapped to HEAD, with straightforward adjustments to
> compensate for Heikki's changes to the ReadBuffer interface. See
> attached.

I looked this over a bit. A couple of suggestions:

1. You could probably simplify life a bit by treating the
BulkInsertState as having an *extra* pin on the buffer, ie, do
IncrBufferRefCount when saving a buffer reference in BulkInsertState and
ReleaseBuffer when removing one. Changing a buffer's local pin count
from 1 to 2 or back again is quite cheap, so you wouldn't need to
special-case things to avoid the existing pin and release operations.
For instance this diff hunk goes away:

***************
*** 1963,1969 ****

END_CRIT_SECTION();

! UnlockReleaseBuffer(buffer);

/*
* If tuple is cachable, mark it for invalidation from the caches in case
--- 1987,1996 ----

END_CRIT_SECTION();

! /* Release the lock, but keep the buffer pinned if doing bulk insert. */
! LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
! if (!bistate)
! ReleaseBuffer(buffer);

/*
* If tuple is cachable, mark it for invalidation from the caches in case

2. The logic changes in RelationGetBufferForTuple seem bizarre and
overcomplicated. ISTM that the buffer saved by the bistate ought to
be about equivalent to relation->rd_targblock, ie, it's your first
trial location and also a place to save the located buffer on the way
out. I'd suggest tossing that part of the patch and starting over.

regards, tom lane