database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-05-05 09:19:17 +03:00
Code

Fix some oversights in BRIN patch.

Browse Source 
Remove HeapScanDescData.rs_initblock, which wasn't being used for anything
in the final version of the patch.

Fix IndexBuildHeapScan so that it supports syncscan again; the patch
broke synchronous scanning for index builds by forcing rs_startblk
to zero even when the caller did not care about that and had asked
for syncscan.

Add some commentary and usage defenses to heap_setscanlimits().

Fix heapam so that asking for rs_numblocks == 0 does what you would
reasonably expect.  As coded it amounted to requesting a whole-table
scan, because those "--x <= 0" tests on an unsigned variable would
behave surprisingly.
This commit is contained in:
Tom Lane 2015-07-21 13:38:24 -04:00
parent 29f171c81a
commit 35ac618a7c
3 changed files with 32 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -277,7 +277,6 @@ initscan(HeapScanDesc scan, ScanKey key, bool is_rescan)
scan->rs_startblock = 0;
}
scan->rs_initblock = 0;
scan->rs_numblocks = InvalidBlockNumber;
scan->rs_inited = false;
scan->rs_ctup.t_data = NULL;
@ -302,11 +301,22 @@ initscan(HeapScanDesc scan, ScanKey key, bool is_rescan)
pgstat_count_heap_scan(scan->rs_rd);
}
/*
* heap_setscanlimits - restrict range of a heapscan
*
* startBlk is the page to start at
* numBlks is number of pages to scan (InvalidBlockNumber means "all")
*/
void
heap_setscanlimits(HeapScanDesc scan, BlockNumber startBlk, BlockNumber numBlks)
{
Assert(!scan->rs_inited); /* else too late to change */
Assert(!scan->rs_syncscan); /* else rs_startblock is significant */
/* Check startBlk is valid (but allow case of zero blocks...) */
Assert(startBlk == 0 || startBlk < scan->rs_nblocks);
scan->rs_startblock = startBlk;
scan->rs_initblock = startBlk;
scan->rs_numblocks = numBlks;
}
@ -477,7 +487,7 @@ heapgettup(HeapScanDesc scan,
/*
* return null immediately if relation is empty
*/
if (scan->rs_nblocks == 0)
if (scan->rs_nblocks == 0 || scan->rs_numblocks == 0)
{
Assert(!BufferIsValid(scan->rs_cbuf));
tuple->t_data = NULL;
@ -511,7 +521,7 @@ heapgettup(HeapScanDesc scan,
/*
* return null immediately if relation is empty
*/
if (scan->rs_nblocks == 0)
if (scan->rs_nblocks == 0 || scan->rs_numblocks == 0)
{
Assert(!BufferIsValid(scan->rs_cbuf));
tuple->t_data = NULL;
@ -651,7 +661,7 @@ heapgettup(HeapScanDesc scan,
if (backward)
{
finished = (page == scan->rs_startblock) ||
(scan->rs_numblocks != InvalidBlockNumber ? --scan->rs_numblocks <= 0 : false);
(scan->rs_numblocks != InvalidBlockNumber ? --scan->rs_numblocks == 0 : false);
if (page == 0)
page = scan->rs_nblocks;
page--;
@ -662,7 +672,7 @@ heapgettup(HeapScanDesc scan,
if (page >= scan->rs_nblocks)
page = 0;
finished = (page == scan->rs_startblock) ||
(scan->rs_numblocks != InvalidBlockNumber ? --scan->rs_numblocks <= 0 : false);
(scan->rs_numblocks != InvalidBlockNumber ? --scan->rs_numblocks == 0 : false);
/*
* Report our new scan position for synchronization purposes. We
@ -754,7 +764,7 @@ heapgettup_pagemode(HeapScanDesc scan,
/*
* return null immediately if relation is empty
*/
if (scan->rs_nblocks == 0)
if (scan->rs_nblocks == 0 || scan->rs_numblocks == 0)
{
Assert(!BufferIsValid(scan->rs_cbuf));
tuple->t_data = NULL;
@ -785,7 +795,7 @@ heapgettup_pagemode(HeapScanDesc scan,
/*
* return null immediately if relation is empty
*/
if (scan->rs_nblocks == 0)
if (scan->rs_nblocks == 0 || scan->rs_numblocks == 0)
{
Assert(!BufferIsValid(scan->rs_cbuf));
tuple->t_data = NULL;
@ -914,7 +924,7 @@ heapgettup_pagemode(HeapScanDesc scan,
if (backward)
{
finished = (page == scan->rs_startblock) ||
(scan->rs_numblocks != InvalidBlockNumber ? --scan->rs_numblocks <= 0 : false);
(scan->rs_numblocks != InvalidBlockNumber ? --scan->rs_numblocks == 0 : false);
if (page == 0)
page = scan->rs_nblocks;
page--;
@ -925,7 +935,7 @@ heapgettup_pagemode(HeapScanDesc scan,
if (page >= scan->rs_nblocks)
page = 0;
finished = (page == scan->rs_startblock) ||
(scan->rs_numblocks != InvalidBlockNumber ? --scan->rs_numblocks <= 0 : false);
(scan->rs_numblocks != InvalidBlockNumber ? --scan->rs_numblocks == 0 : false);
/*
* Report our new scan position for synchronization purposes. We

12
src/backend/catalog/index.c
View File

@ -2168,7 +2168,8 @@ IndexBuildHeapScan(Relation heapRelation,
/*
* As above, except that instead of scanning the complete heap, only the given
* number of blocks are scanned. Scan to end-of-rel can be signalled by
* passing InvalidBlockNumber as numblocks.
* passing InvalidBlockNumber as numblocks. Note that restricting the range
* to scan cannot be done when requesting syncscan.
*/
double
IndexBuildHeapRangeScan(Relation heapRelation,
@ -2251,7 +2252,14 @@ IndexBuildHeapRangeScan(Relation heapRelation,
allow_sync); /* syncscan OK? */
/* set our scan endpoints */
heap_setscanlimits(scan, start_blockno, numblocks);
if (!allow_sync)
heap_setscanlimits(scan, start_blockno, numblocks);
else
{
/* syncscan can only be requested on whole relation */
Assert(start_blockno == 0);
Assert(numblocks == InvalidBlockNumber);
}
reltuples = 0;

4
src/include/access/relscan.h
View File

@ -38,8 +38,8 @@ typedef struct HeapScanDescData
/* state set up at initscan time */
BlockNumber rs_nblocks; /* total number of blocks in rel */
BlockNumber rs_startblock; /* block # to start at */
BlockNumber rs_initblock; /* block # to consider initial of rel */
BlockNumber rs_numblocks; /* number of blocks to scan */
BlockNumber rs_numblocks; /* max number of blocks to scan */
/* rs_numblocks is usually InvalidBlockNumber, meaning "scan whole rel" */
BufferAccessStrategy rs_strategy; /* access strategy for reads */
bool rs_syncscan; /* report location to syncscan logic? */