From 25211563d5f3cf90fd9dfe9f06a0890d782ff981 Mon Sep 17 00:00:00 2001 From: Nazir Bilal Yavuz Date: Fri, 27 Mar 2026 14:54:40 +0300 Subject: [PATCH v3 2/2] Improve API for storing data in read streams. Read stream callbacks receive a void pointer into the per-buffer data queue so that can store data there for later retrieval by the buffer consumer. We can improve readability and safety a bit by changing cast-and-assign or raw memcpy() to: read_stream_put_value(stream, per_buffer_data, my_int); This form infers the size and asserts that the storage space matches, generally mirroring the read_stream_get_buffer_and_value() call used for retrieving the streamed data later. --- src/backend/access/heap/heapam_handler.c | 9 ++------- src/backend/access/heap/vacuumlazy.c | 9 ++++++--- src/include/storage/read_stream.h | 9 +++++++++ 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 7229dc3bd51..ead3ad3ecb3 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2490,7 +2490,6 @@ BitmapHeapScanNextBlock(TableScanDesc scan, BitmapHeapScanDesc bscan = (BitmapHeapScanDesc) scan; HeapScanDesc hscan = (HeapScanDesc) bscan; BlockNumber block; - void *per_buffer_data; Buffer buffer; Snapshot snapshot; int ntup; @@ -2511,8 +2510,7 @@ BitmapHeapScanNextBlock(TableScanDesc scan, hscan->rs_cbuf = InvalidBuffer; } - hscan->rs_cbuf = read_stream_next_buffer(hscan->rs_read_stream, - &per_buffer_data); + hscan->rs_cbuf = read_stream_get_buffer_and_pointer(hscan->rs_read_stream, &tbmres); if (BufferIsInvalid(hscan->rs_cbuf)) { @@ -2520,10 +2518,7 @@ BitmapHeapScanNextBlock(TableScanDesc scan, return false; } - Assert(per_buffer_data); - - tbmres = per_buffer_data; - + Assert(tbmres); Assert(BlockNumberIsValid(tbmres->blockno)); Assert(BufferGetBlockNumber(hscan->rs_cbuf) == tbmres->blockno); diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index df4ef84f6ff..cda3247cc69 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -1693,6 +1693,9 @@ heap_vac_scan_next_block(ReadStream *stream, /* Now we must be in one of the two remaining states: */ if (next_block < vacrel->next_unskippable_block) { + /* read_stream_put_value() requires an lvalue, not a literal */ + bool temp = false; + /* * 2. We are processing a range of blocks that we could have skipped * but chose not to. We know that they are all-visible in the VM, @@ -1700,7 +1703,7 @@ heap_vac_scan_next_block(ReadStream *stream, */ vacrel->current_block = next_block; /* Block was not eager scanned */ - *((bool *) per_buffer_data) = false; + read_stream_put_value(stream, per_buffer_data, temp); return vacrel->current_block; } else @@ -1712,7 +1715,7 @@ heap_vac_scan_next_block(ReadStream *stream, Assert(next_block == vacrel->next_unskippable_block); vacrel->current_block = next_block; - *((bool *) per_buffer_data) = vacrel->next_unskippable_eager_scanned; + read_stream_put_value(stream, per_buffer_data, vacrel->next_unskippable_eager_scanned); return vacrel->current_block; } } @@ -2600,7 +2603,7 @@ vacuum_reap_lp_read_stream_next(ReadStream *stream, * Save the TidStoreIterResult for later, so we can extract the offsets. * It is safe to copy the result, according to TidStoreIterateNext(). */ - memcpy(per_buffer_data, iter_result, sizeof(*iter_result)); + read_stream_put_value(stream, per_buffer_data, *iter_result); return iter_result->blkno; } diff --git a/src/include/storage/read_stream.h b/src/include/storage/read_stream.h index b86450d5b39..fb3a47e74cc 100644 --- a/src/include/storage/read_stream.h +++ b/src/include/storage/read_stream.h @@ -159,4 +159,13 @@ read_stream_get_buffer_and_value_with_size(ReadStream *stream, (AssertMacro(sizeof(**(pointer)) <= read_stream_per_buffer_data_size(stream)), \ read_stream_next_buffer((stream), ((void **) (pointer)))) +/* + * Set the per-buffer data by value. This can be called from inside a + * callback that is returning block numbers. It asserts that the value's size + * matches the available space. + */ +#define read_stream_put_value(stream, per_buffer_data, value) \ + (AssertMacro(sizeof(value) == read_stream_per_buffer_data_size(stream)), \ + memcpy((per_buffer_data), &(value), sizeof(value))) + #endif /* READ_STREAM_H */ -- 2.47.3