Subject: [PATCH v5 01/14] BitmapHeapScan begin scan after bitmap creation

From: Melanie Plageman <[email-masked]>
Date: Mon, 12 Feb 2024 18:50:29 -0500

There is no reason for a BitmapHeapScan to begin the scan of the
underlying table in ExecInitBitmapHeapScan(). Instead, do so after
completing the index scan and building the bitmap.

ExecBitmapHeapInitializeWorker() overwrote the snapshot in the scan
descriptor with the correct one provided by the parallel leader. Since
ExecBitmapHeapInitializeWorker() is now called before the scan
descriptor has been created, save the worker's snapshot in the
BitmapHeapScanState and pass it to table_beginscan_bm().
---
 src/backend/access/table/tableam.c        | 11 ------
 src/backend/executor/nodeBitmapHeapscan.c | 47 ++++++++++++++++++-----
 src/include/access/tableam.h              | 10 ++---
 src/include/nodes/execnodes.h             |  2 +
 4 files changed, 42 insertions(+), 28 deletions(-)

diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
index 6ed8cca05a1..e78d793f69c 100644
--- a/src/backend/access/table/tableam.c
+++ b/src/backend/access/table/tableam.c
@@ -120,17 +120,6 @@ table_beginscan_catalog(Relation relation, int nkeys, struct ScanKeyData *key)
 											NULL, flags);
 }
 
-void
-table_scan_update_snapshot(TableScanDesc scan, Snapshot snapshot)
-{
-	Assert(IsMVCCSnapshot(snapshot));
-
-	RegisterSnapshot(snapshot);
-	scan->rs_snapshot = snapshot;
-	scan->rs_flags |= SO_TEMP_SNAPSHOT;
-}
-
-
 /* ----------------------------------------------------------------------------
  * Parallel table scan related functions.
  * ----------------------------------------------------------------------------
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c1e81ebed63..44bf38be3c9 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -181,6 +181,34 @@ BitmapHeapNext(BitmapHeapScanState *node)
 			}
 #endif							/* USE_PREFETCH */
 		}
+
+		/*
+		 * If this is the first scan of the underlying table, create the table
+		 * scan descriptor and begin the scan.
+		 */
+		if (!scan)
+		{
+			Snapshot	snapshot = node->ss.ps.state->es_snapshot;
+			uint32		extra_flags = 0;
+
+			/*
+			 * Parallel workers must use the snapshot initialized by the
+			 * parallel leader.
+			 */
+			if (node->worker_snapshot)
+			{
+				snapshot = node->worker_snapshot;
+				extra_flags |= SO_TEMP_SNAPSHOT;
+			}
+
+			scan = node->ss.ss_currentScanDesc = table_beginscan_bm(
+																	node->ss.ss_currentRelation,
+																	snapshot,
+																	0,
+																	NULL,
+																	extra_flags);
+		}
+
 		node->initialized = true;
 	}
 
@@ -604,7 +632,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
 	PlanState  *outerPlan = outerPlanState(node);
 
 	/* rescan to release any page pin */
-	table_rescan(node->ss.ss_currentScanDesc, NULL);
+	if (node->ss.ss_currentScanDesc)
+		table_rescan(node->ss.ss_currentScanDesc, NULL);
 
 	/* release bitmaps and buffers if any */
 	if (node->tbmiterator)
@@ -681,7 +710,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 	/*
 	 * close heap scan
 	 */
-	table_endscan(scanDesc);
+	if (scanDesc)
+		table_endscan(scanDesc);
+
 }
 
 /* ----------------------------------------------------------------
@@ -739,6 +770,7 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 	 */
 	scanstate->can_skip_fetch = (node->scan.plan.qual == NIL &&
 								 node->scan.plan.targetlist == NIL);
+	scanstate->worker_snapshot = NULL;
 
 	/*
 	 * Miscellaneous initialization
@@ -787,11 +819,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 
 	scanstate->ss.ss_currentRelation = currentRelation;
 
-	scanstate->ss.ss_currentScanDesc = table_beginscan_bm(currentRelation,
-														  estate->es_snapshot,
-														  0,
-														  NULL);
-
 	/*
 	 * all done.
 	 */
@@ -930,13 +957,13 @@ ExecBitmapHeapInitializeWorker(BitmapHeapScanState *node,
 							   ParallelWorkerContext *pwcxt)
 {
 	ParallelBitmapHeapState *pstate;
-	Snapshot	snapshot;
 
 	Assert(node->ss.ps.state->es_query_dsa != NULL);
 
 	pstate = shm_toc_lookup(pwcxt->toc, node->ss.ps.plan->plan_node_id, false);
 	node->pstate = pstate;
 
-	snapshot = RestoreSnapshot(pstate->phs_snapshot_data);
-	table_scan_update_snapshot(node->ss.ss_currentScanDesc, snapshot);
+	node->worker_snapshot = RestoreSnapshot(pstate->phs_snapshot_data);
+	Assert(IsMVCCSnapshot(node->worker_snapshot));
+	RegisterSnapshot(node->worker_snapshot);
 }
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 5f8474871d2..5375dd7150f 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -944,9 +944,10 @@ table_beginscan_strat(Relation rel, Snapshot snapshot,
  */
 static inline TableScanDesc
 table_beginscan_bm(Relation rel, Snapshot snapshot,
-				   int nkeys, struct ScanKeyData *key)
+				   int nkeys, struct ScanKeyData *key,
+				   uint32 extra_flags)
 {
-	uint32		flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE;
+	uint32		flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE | extra_flags;
 
 	return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags);
 }
@@ -1038,11 +1039,6 @@ table_rescan_set_params(TableScanDesc scan, struct ScanKeyData *key,
 										 allow_pagemode);
 }
 
-/*
- * Update snapshot used by the scan.
- */
-extern void table_scan_update_snapshot(TableScanDesc scan, Snapshot snapshot);
-
 /*
  * Return next tuple from `scan`, store in slot.
  */
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 444a5f0fd57..00c75fb10e2 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1726,6 +1726,7 @@ typedef struct ParallelBitmapHeapState
  *		shared_tbmiterator	   shared iterator
  *		shared_prefetch_iterator shared iterator for prefetching
  *		pstate			   shared state for parallel bitmap scan
+ *		worker_snapshot	   snapshot for parallel worker
  * ----------------
  */
 typedef struct BitmapHeapScanState
@@ -1750,6 +1751,7 @@ typedef struct BitmapHeapScanState
 	TBMSharedIterator *shared_tbmiterator;
 	TBMSharedIterator *shared_prefetch_iterator;
 	ParallelBitmapHeapState *pstate;
+	Snapshot	worker_snapshot;
 } BitmapHeapScanState;
 
 /* ----------------
-- 
2.37.2

--6kjpcnqyi64ibp5i
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v5-0002-BitmapHeapScan-set-can_skip_fetch-later.patch"