Re: Beautify read stream "per buffer data" APIs

Thanks for rebasing, putting this together, and extracting from that
other length thread!

On Fri, Mar 27, 2026 at 8:02 AM Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com> wrote:
>
> The read_stream_put_value() macro doesn't accept literal constant
> values, we need to pass a variable to it. Otherwise, the compilation
> fails with:
>
> ```
> ../../postgres/src/include/storage/read_stream.h:169:36: error: lvalue
> required as unary ‘&’ operand
> 169 | memcpy((per_buffer_data), &(value), sizeof(value)))
> | ^
> ../../postgres/src/backend/access/heap/vacuumlazy.c:1703:17: note: in
> expansion of macro ‘read_stream_put_value’
> 1703 | read_stream_put_value(stream, per_buffer_data, false);
> ```
>
> If that is not intentional, I think it would be better if we can
> convert read_stream_put_value() to a way that it accepts rvalues.

I agree this would be much better from an API usability standpoint.
I'm not sure we can do this in a way people would find acceptable in
Postgres code, though. Apparently this works:

read_stream_put_value(stream, per_buffer_data, (bool){false});

but it is a little weird.

As we discussed off-list, I think the BitmapHeapScanNextBlock()
read_stream_get_buffer_and_pointer() transition should be moved from
0002 to 0001 for clarity.

I think this is unequivocally an improvement in the API. But it
probably needs more time to bake now that you've started a new thread.
It only helps in development, so we can commit it early in 20 and
still get much of the same benefit. External users would have to wait.
But it is probably better to wait than to have an API in 19 that
changes again in 20.

I don't have strong objections if Thomas wants to commit them, though.

- Melanie