OK, here's an updated version...
1. Use IncrBufferRefCount() so that we can do unconditional
ReleaseBuffers elsewhere. I'm not sure this is really any simpler,
and although IncrBufferRefCount() is pretty cheap, it's certainly not
as cheap as a NULL pointer test.
2. Consolidate a bunch of logic into a new function
RelationReadBuffer. This simpifies the logic in
RelationGetBufferForTuple() considerably.
3. Make RelationGetBufferForTuple ignore relation->rd_block in favor
of bistate->last_pin whenever possible. Changing this to also not
bother setting relation->rd_block didn't seem worthwhile, so I didn't.
...Robert
On Tue, Nov 4, 2008 at 4:18 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> "Robert Haas" <robertmhaas@gmail.com> writes:
>>> 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.
>
>> Hmm, would that be safe in the presence of concurrent or recursive
>> bulk inserts into the same relation?
>
> As safe as it is now --- you're relying on the bistate to carry the
> query-local state. Probably the best design is to just ignore
> rd_targblock when a bistate is provided, and use the bistate's buffer
> instead.
>
> regards, tom lane
>