Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitd167c19

Browse files
committed
Fix possibly uninitialized HeapScanDesc.rs_startblock
The solution used in0ca3b16 to determine the Parallel TID RangeScan's start location was to modify the signature oftable_block_parallelscan_startblock_init() to allow the startblockto be passed in as a parameter. This allows the scan limits to beadjusted before that function is called so that the limits are picked upwhen the parallel scan starts. The commit made it so the call totable_block_parallelscan_startblock_init uses the HeapScanDesc'srs_startblock to pass the startblock to the parallel scan. That allworks ok for Parallel TID Range scans as the HeapScanDesc rs_startblockgets set by heap_setscanlimits(), but for Parallel Seq Scans, initscan()does not initialize rs_startblock, and that results in passing anuninitialized value to table_block_parallelscan_startblock_init() asnoted by the buildfarm member skink, running Valgrind.To fix this issue, make it so initscan() sets the rs_startblock forparallel scans unless we're doing a rescan. This makes it sotable_block_parallelscan_startblock_init() will be called with thestartblock set to InvalidBlockNumber, and that'll allow the syncscancode to find the correct start location (when enabled). For ParallelTID Range Scans, this InvalidBlockNumber value will be overwritten inthe call to heap_setscanlimits().initscan() is a bit light on documentation on what's meant to getinitialized where for parallel scans. From what I can tell, it looks likeit just didn't matter prior to0ca3b16 that rs_startblock was leftuninitialized for parallel scans. To address the light documentation,I've also added some comments to mention that the syncscan location forparallel scans is figured out in table_block_parallelscan_startblock_init.I've also taken the liberty to adjust the if/else if/else code ininitscan() to make it clearer which parts apply to parallel scans andwhich parts are for the serial scans.Author: David Rowley <dgrowleyml@gmail.com>Discussion:https://postgr.es/m/CAApHDvqALm+k7FyfdQdCw1yF_8HojvR61YRrNhwRQPE=zSmnQA@mail.gmail.com
1 parentc75bf57 commitd167c19

File tree

1 file changed

+30
-17
lines changed

1 file changed

+30
-17
lines changed

‎src/backend/access/heap/heapam.c‎

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -415,28 +415,41 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock)
415415
scan->rs_base.rs_flags |=SO_ALLOW_SYNC;
416416
else
417417
scan->rs_base.rs_flags &= ~SO_ALLOW_SYNC;
418-
}
419-
elseif (keep_startblock)
420-
{
418+
421419
/*
422-
* When rescanning, we want to keep the previous startblock setting,
423-
* so that rewinding a cursor doesn't generate surprising results.
424-
* Reset the active syncscan setting, though.
420+
* If not rescanning, initialize the startblock. Finding the actual
421+
* start location is done in table_block_parallelscan_startblock_init,
422+
* based on whether an alternative start location has been set with
423+
* heap_setscanlimits, or using the syncscan location, when syncscan
424+
* is enabled.
425425
*/
426-
if (allow_sync&&synchronize_seqscans)
427-
scan->rs_base.rs_flags |=SO_ALLOW_SYNC;
428-
else
429-
scan->rs_base.rs_flags &= ~SO_ALLOW_SYNC;
430-
}
431-
elseif (allow_sync&&synchronize_seqscans)
432-
{
433-
scan->rs_base.rs_flags |=SO_ALLOW_SYNC;
434-
scan->rs_startblock=ss_get_location(scan->rs_base.rs_rd,scan->rs_nblocks);
426+
if (!keep_startblock)
427+
scan->rs_startblock=InvalidBlockNumber;
435428
}
436429
else
437430
{
438-
scan->rs_base.rs_flags &= ~SO_ALLOW_SYNC;
439-
scan->rs_startblock=0;
431+
if (keep_startblock)
432+
{
433+
/*
434+
* When rescanning, we want to keep the previous startblock
435+
* setting, so that rewinding a cursor doesn't generate surprising
436+
* results. Reset the active syncscan setting, though.
437+
*/
438+
if (allow_sync&&synchronize_seqscans)
439+
scan->rs_base.rs_flags |=SO_ALLOW_SYNC;
440+
else
441+
scan->rs_base.rs_flags &= ~SO_ALLOW_SYNC;
442+
}
443+
elseif (allow_sync&&synchronize_seqscans)
444+
{
445+
scan->rs_base.rs_flags |=SO_ALLOW_SYNC;
446+
scan->rs_startblock=ss_get_location(scan->rs_base.rs_rd,scan->rs_nblocks);
447+
}
448+
else
449+
{
450+
scan->rs_base.rs_flags &= ~SO_ALLOW_SYNC;
451+
scan->rs_startblock=0;
452+
}
440453
}
441454

442455
scan->rs_numblocks=InvalidBlockNumber;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp