BitmapHeapScan: Use correct recheck flag for skip_fetch

As of 7c70996ebf0949b142a9, BitmapPrefetch() used the recheck flag for
the current block to determine whether or not it should skip prefetching
the proposed prefetch block. As explained in the comment, this assumed
the index AM will report the same recheck value for the future page as
it did for the current page - but there's no guarantee.

This only affects prefetching - if the recheck flag changes, we may
prefetch blocks unecessarily and not prefetch blocks that will be
needed. But we don't need to rely on that assumption - we know the
recheck flag for the block we're considering prefetching, so we can
use that.

The impact is very limited in practice - the opclass would need to
assign different recheck flags to different blocks, but none of the
built-in opclasses seems to do that.

Author: Melanie Plageman
Reviewed-by: Tomas Vondra, Andres Freund, Tom Lane
Discussion: https://postgr.es/m/1939305.1712415547%40sss.pgh.pa.us

diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 5f768701a5ee981e52f3b9525b4ebe6e5b8ed8a7..6f843908032564c986c7e7de60c195ee627308a8 100644 (file)
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -475,14 +475,9 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
                 * skip this prefetch call, but continue to run the prefetch
                 * logic normally.  (Would it be better not to increment
                 * prefetch_pages?)
-                *
-                * This depends on the assumption that the index AM will
-                * report the same recheck flag for this future heap page as
-                * it did for the current heap page; which is not a certainty
-                * but is true in many cases.
                 */
                skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLES) &&
-                             (node->tbmres ? !node->tbmres->recheck : false) &&
+                             !tbmpre->recheck &&
                              VM_ALL_VISIBLE(node->ss.ss_currentRelation,
                                             tbmpre->blockno,
                                             &node->pvmbuffer));
@@ -533,7 +528,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
 
                /* As above, skip prefetch if we expect not to need page */
                skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLES) &&
-                             (node->tbmres ? !node->tbmres->recheck : false) &&
+                             !tbmpre->recheck &&
                              VM_ALL_VISIBLE(node->ss.ss_currentRelation,
                                             tbmpre->blockno,
                                             &node->pvmbuffer));