v14-0005-BitmapPrefetch-use-prefetch-block-recheck-for-sk.patch

text/x-patch

Filename: v14-0005-BitmapPrefetch-use-prefetch-block-recheck-for-sk.patch
Type: text/x-patch
Part: 4
Message: Re: BitmapHeapScan streaming read user and prelim refactoring

Patch

Format: format-patch
Series: patch v14-0005
Subject: BitmapPrefetch use prefetch block recheck for skip fetch
File+
src/backend/executor/nodeBitmapHeapscan.c 2 7
From e3fa2f25b5631bb9a5e27515be8161ef89418c40 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Mon, 12 Feb 2024 19:03:24 -0500
Subject: [PATCH v14 05/19] BitmapPrefetch use prefetch block recheck for skip
 fetch

As of 7c70996ebf0949b142a9, BitmapPrefetch() used the recheck flag for
the current block to determine whether or not it could skip prefetching
the proposed prefetch block. It makes more sense for it to use the
recheck flag from the TBMIterateResult for the prefetch block instead.
See this [1] thread on hackers reporting the issue.

XXX I think this commit message should "distill" the reasoning from [1]
to justify why this "makes more sense". The details can be left to the
thread, but the basics should be in this message I think.

[1] https://www.postgresql.org/message-id/CAAKRu_bxrXeZ2rCnY8LyeC2Ls88KpjWrQ%2BopUrXDRXdcfwFZGA%40mail.gmail.com

Author: Melanie Plageman
Reviewed-by: Andres Freund, Heikki Linnakangas, Mark Dilger
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
 src/backend/executor/nodeBitmapHeapscan.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index a8bc5dec53d..2e2cec8b3b5 100644
--- 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_TUPLE) &&
-							  (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_TUPLE) &&
-							  (node->tbmres ? !node->tbmres->recheck : false) &&
+							  !tbmpre->recheck &&
 							  VM_ALL_VISIBLE(node->ss.ss_currentRelation,
 											 tbmpre->blockno,
 											 &node->pvmbuffer));
-- 
2.44.0