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

On Fri, Apr 10, 2026 at 10:22:56PM -0400, Tom Lane wrote:
> 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.

I have read the patch and I would agree this stance.

>> * 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.

+1 for doing it now. It's also worth noting that this shaves code
line-wise.
--
Michael