database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-11-28 11:44:57 +03:00
Code Activity

Fix possibly uninitialized HeapScanDesc.rs_startblock

Browse Source 
The solution used in 0ca3b1697 to determine the Parallel TID Range
Scan's start location was to modify the signature of
table_block_parallelscan_startblock_init() to allow the startblock
to be passed in as a parameter.  This allows the scan limits to be
adjusted before that function is called so that the limits are picked up
when the parallel scan starts.  The commit made it so the call to
table_block_parallelscan_startblock_init uses the HeapScanDesc's
rs_startblock to pass the startblock to the parallel scan.  That all
works ok for Parallel TID Range scans as the HeapScanDesc rs_startblock
gets set by heap_setscanlimits(), but for Parallel Seq Scans, initscan()
does not initialize rs_startblock, and that results in passing an
uninitialized value to table_block_parallelscan_startblock_init() as
noted by the buildfarm member skink, running Valgrind.

To fix this issue, make it so initscan() sets the rs_startblock for
parallel scans unless we're doing a rescan.  This makes it so
table_block_parallelscan_startblock_init() will be called with the
startblock set to InvalidBlockNumber, and that'll allow the syncscan
code to find the correct start location (when enabled).  For Parallel
TID Range Scans, this InvalidBlockNumber value will be overwritten in
the call to heap_setscanlimits().

initscan() is a bit light on documentation on what's meant to get
initialized where for parallel scans.  From what I can tell, it looks like
it just didn't matter prior to 0ca3b1697 that rs_startblock was left
uninitialized for parallel scans.  To address the light documentation,
I've also added some comments to mention that the syncscan location for
parallel scans is figured out in table_block_parallelscan_startblock_init.
I've also taken the liberty to adjust the if/else if/else code in
initscan() to make it clearer which parts apply to parallel scans and
which parts are for the serial scans.

Author: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/CAApHDvqALm+k7FyfdQdCw1yF_8HojvR61YRrNhwRQPE=zSmnQA@mail.gmail.com
This commit is contained in:
David Rowley
2025-11-28 12:40:50 +13:00
parent c75bf57a90
commit d167c19295
1 changed files with 30 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
src/backend/access/heap/heapam.c
View File

@@ -415,13 +415,25 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock)
scan->rs_base.rs_flags |= SO_ALLOW_SYNC;
else
scan->rs_base.rs_flags &= ~SO_ALLOW_SYNC;
/*
* If not rescanning, initialize the startblock. Finding the actual
* start location is done in table_block_parallelscan_startblock_init,
* based on whether an alternative start location has been set with
* heap_setscanlimits, or using the syncscan location, when syncscan
* is enabled.
*/
if (!keep_startblock)
scan->rs_startblock = InvalidBlockNumber;
}
else if (keep_startblock)
else
{
if (keep_startblock)
{
/*
* When rescanning, we want to keep the previous startblock setting,
* so that rewinding a cursor doesn't generate surprising results.
* Reset the active syncscan setting, though.
* When rescanning, we want to keep the previous startblock
* setting, so that rewinding a cursor doesn't generate surprising
* results. Reset the active syncscan setting, though.
*/
if (allow_sync && synchronize_seqscans)
scan->rs_base.rs_flags |= SO_ALLOW_SYNC;
@@ -438,6 +450,7 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock)
scan->rs_base.rs_flags &= ~SO_ALLOW_SYNC;
scan->rs_startblock = 0;
}
}
scan->rs_numblocks = InvalidBlockNumber;
scan->rs_inited = false;