v22b-0010-review.patch

From b1ec31d645fea52f7c71bc9717ef182bf8f8cd18 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tv@fuzzy.cz>
Date: Tue, 18 Jun 2024 22:03:15 +0200
Subject: [PATCH v22b 10/29] review

---
 src/backend/executor/nodeBitmapHeapscan.c | 5 +++++
 src/include/nodes/execnodes.h             | 1 +
 2 files changed, 6 insertions(+)

diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 43297783ab3..7ca986ae5b2 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -263,6 +263,8 @@ new_page:
 		 * ahead of the current block.
 		 */
 		if (node->pstate == NULL &&
+			/* XXX shouldn't we have a function instead of checking the
+			 * field directly? */
 			!node->prefetch_iterator.exhausted &&
 			node->pfblockno < node->blockno)
 			elog(ERROR, "prefetch and main iterators are out of sync");
@@ -557,6 +559,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
 		table_rescan(node->ss.ss_currentScanDesc, NULL);
 
 	/* release bitmaps and buffers if any */
+	/* XXX seems it should not be right after the comment, also shouldn't
+	 * we still reset the prefetch_iterator field to NULL? */
 	tbm_end_iterate(&node->prefetch_iterator);
 	if (node->tbm)
 		tbm_free(node->tbm);
@@ -611,6 +615,7 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 	/*
 	 * release bitmaps and buffers if any
 	 */
+	/* XXX misleading place, not a bitmap or a buffer */
 	tbm_end_iterate(&node->prefetch_iterator);
 	if (node->tbm)
 		tbm_free(node->tbm);
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 1f262db776c..f47e6c75b7c 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1821,6 +1821,7 @@ typedef struct BitmapHeapScanState
 	int			prefetch_target;
 	int			prefetch_maximum;
 	bool		initialized;
+	/* XXX probably should be kept in the order per comment */
 	TBMIterator prefetch_iterator;
 	ParallelBitmapHeapState *pstate;
 	bool		recheck;
-- 
2.45.2