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 <[email protected]> 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
