Re: BUG #19006: Assert(BufferIsPinned) in BufferGetBlockNumber() is triggered for forwarded buffer

Thomas Munro <thomas(dot)munro(at)gmail(dot)com> writes:
> Unfortunately this fell through the cracks (sorry) and I didn't push
> it before the freeze. Any objections to pushing it now? I can live
> with deferring it until master reopens if that's the call (CC RMT),
> but it would be nice to tidy up this design wart if we can.

This doesn't seem to me to be a "new feature", so I'm not sure that
feature freeze applies.

> This patch shifts some of that responsibility to a more natural place:

> * it now seems obvious that StartReadBuffers() should just allow an
> in/out npinned counter to travel along with the in/out buffers array
> * read_stream.c still needs to know how many there are for pin limit purposes
> * it also needs to know in the unusual case that the stream ends
> earlier and it has to unpin them
> * other than that, it's StartReadBuffers()'s private business to manage them
> * StartReadBuffers() can do that with trivial arithmetic, no need to
> distinguish and count the buffers
> * the end result is much simpler and more robust

IIUC, this is basically fixing StartReadBuffers' API, and if we don't
do it now then the v19 code will differ from both earlier and later
branches. That doesn't seem like a great place to be when you think
about having to back-patch bug fixes in this area.

So yeah, squeezing this in now seems like a good bet to me.

regards, tom lane