From 3a0469df93690889793823fdec235f72c8fb81d7 Mon Sep 17 00:00:00 2001 From: "dgrowley@gmail.com" Date: Thu, 21 Jan 2021 16:27:25 +1300 Subject: [PATCH v11 1/2] Fix hypothetical bug in heap backward scans Both heapgettup() and heapgettup_pagemode() incorrectly set the first page to scan in a backward scan in which the pages to scan was specified by heap_setscanlimits(). In theory, this could result in the incorrect pages being scanned. In practice, nowhere in core code performs backward scans after having used heap_setscanlimits(). However, it's possible an extension uses the heap functions in this way. For the bug to manifest, the scan must be limited to fewer than the number of pages in the relation and start at page 0. The scan will start on the final page in the table rather than the final page in the range of pages to scan. The correct number of pages is always scanned, it's just the pages which are scanned which can be incorrect. This is a precursor fix to a future patch which allows TID Range scans to scan a subset of a heap table. Proper adjustment of the heap scan code seems to have been missed when heap_setscanlimits() was added in 7516f5259. Author: David Rowley Discussion: https://postgr.es/m/CAApHDvpGc9h0_oVD2CtgBcxCS1N-qDYZSeBRnUh+0CWJA9cMaA@mail.gmail.com --- src/backend/access/heap/heapam.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index faffbb1865..ddd214b7af 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -603,11 +603,15 @@ heapgettup(HeapScanDesc scan, * forward scanners. */ scan->rs_base.rs_flags &= ~SO_ALLOW_SYNC; - /* start from last page of the scan */ - if (scan->rs_startblock > 0) - page = scan->rs_startblock - 1; + + /* + * Start from last page of the scan. Ensure we take into account + * rs_numblocks if it's been adjusted by heap_setscanlimits(). + */ + if (scan->rs_numblocks != InvalidBlockNumber) + page = (scan->rs_startblock + scan->rs_numblocks - 1) % scan->rs_nblocks; else - page = scan->rs_nblocks - 1; + page = (scan->rs_startblock + scan->rs_nblocks - 1) % scan->rs_nblocks; heapgetpage((TableScanDesc) scan, page); } else @@ -918,11 +922,15 @@ heapgettup_pagemode(HeapScanDesc scan, * forward scanners. */ scan->rs_base.rs_flags &= ~SO_ALLOW_SYNC; - /* start from last page of the scan */ - if (scan->rs_startblock > 0) - page = scan->rs_startblock - 1; + + /* + * Start from last page of the scan. Ensure we take into account + * rs_numblocks if it's been adjusted by heap_setscanlimits(). + */ + if (scan->rs_numblocks != InvalidBlockNumber) + page = (scan->rs_startblock + scan->rs_numblocks - 1) % scan->rs_nblocks; else - page = scan->rs_nblocks - 1; + page = (scan->rs_startblock + scan->rs_nblocks - 1) % scan->rs_nblocks; heapgetpage((TableScanDesc) scan, page); } else -- 2.27.0