Move BitmapTableScan per-scan setup into a helper

Add BitmapTableScanSetup(), a helper which contains all of the code that
must be done on every scan of the table in a bitmap table scan. This
includes scanning the index, building the bitmap, and setting up the
scan descriptors.

Pushing this setup into a helper function makes BitmapHeapNext() more
readable.

Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/CAN55FZ1vXu%2BZdT0_MM-i1vbTdfHHf0KR3cK6R5gs6dNNNpyrJw%40mail.gmail.com

src/backend/executor/nodeBitmapHeapscan.c

@@ -48,6 +48,7 @@
 #include "utils/rel.h"
 #include "utils/spccache.h"
 
+static void BitmapTableScanSetup(BitmapHeapScanState *node);
 static TupleTableSlot *BitmapHeapNext(BitmapHeapScanState *node);
 static inline void BitmapDoneInitializingSharedState(ParallelBitmapHeapState *pstate);
 static inline void BitmapAdjustPrefetchIterator(BitmapHeapScanState *node);
@@ -57,80 +58,55 @@ static inline void BitmapPrefetch(BitmapHeapScanState *node,
 static bool BitmapShouldInitializeSharedState(ParallelBitmapHeapState *pstate);
 
 
-/* ----------------------------------------------------------------
+/*
- *		BitmapHeapNext
+ * Do the underlying index scan, build the bitmap, set up the parallel state
+ * needed for parallel workers to iterate through the bitmap, and set up the
+ * underlying table scan descriptor.
  *
- *		Retrieve next tuple from the BitmapHeapScan node's currentRelation
+ * For prefetching, we use *two* iterators, one for the pages we are actually
- * ----------------------------------------------------------------
+ * scanning and another that runs ahead of the first for prefetching.
- */
+ * node->prefetch_pages tracks exactly how many pages ahead the prefetch
-static TupleTableSlot *
+ * iterator is.  Also, node->prefetch_target tracks the desired prefetch
-BitmapHeapNext(BitmapHeapScanState *node)
+ * distance, which starts small and increases up to the
-{
+ * node->prefetch_maximum.  This is to avoid doing a lot of prefetching in a
-	ExprContext *econtext;
+ * scan that stops after a few tuples because of a LIMIT.
-	TableScanDesc scan;
+ */
-	TIDBitmap  *tbm;
+static void
-	TupleTableSlot *slot;
+BitmapTableScanSetup(BitmapHeapScanState *node)
+{
+	TBMIterator tbmiterator = {0};
 	ParallelBitmapHeapState *pstate = node->pstate;
 	dsa_area   *dsa = node->ss.ps.state->es_query_dsa;
 
-	/*
-	 * extract necessary information from index scan node
-	 */
-	econtext = node->ss.ps.ps_ExprContext;
-	slot = node->ss.ss_ScanTupleSlot;
-	scan = node->ss.ss_currentScanDesc;
-	tbm = node->tbm;
-
-	/*
-	 * If we haven't yet performed the underlying index scan, do it, and begin
-	 * the iteration over the bitmap.
-	 *
-	 * For prefetching, we use *two* iterators, one for the pages we are
-	 * actually scanning and another that runs ahead of the first for
-	 * prefetching.  node->prefetch_pages tracks exactly how many pages ahead
-	 * the prefetch iterator is.  Also, node->prefetch_target tracks the
-	 * desired prefetch distance, which starts small and increases up to the
-	 * node->prefetch_maximum.  This is to avoid doing a lot of prefetching in
-	 * a scan that stops after a few tuples because of a LIMIT.
-	 */
-	if (!node->initialized)
-	{
-		TBMIterator tbmiterator;
-
 	if (!pstate)
 	{
-			tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
+		node->tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
 
-			if (!tbm || !IsA(tbm, TIDBitmap))
+		if (!node->tbm || !IsA(node->tbm, TIDBitmap))
 			elog(ERROR, "unrecognized result from subplan");
-
-			node->tbm = tbm;
 	}
 	else if (BitmapShouldInitializeSharedState(pstate))
 	{
 		/*
-			 * The leader will immediately come out of the function, but
+		 * The leader will immediately come out of the function, but others
-			 * others will be blocked until leader populates the TBM and wakes
+		 * will be blocked until leader populates the TBM and wakes them up.
-			 * them up.
 		 */
-			tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
+		node->tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
-			if (!tbm || !IsA(tbm, TIDBitmap))
+		if (!node->tbm || !IsA(node->tbm, TIDBitmap))
 			elog(ERROR, "unrecognized result from subplan");
 
-			node->tbm = tbm;
-
 		/*
-			 * Prepare to iterate over the TBM. This will return the
+		 * Prepare to iterate over the TBM. This will return the dsa_pointer
-			 * dsa_pointer of the iterator state which will be used by
+		 * of the iterator state which will be used by multiple processes to
-			 * multiple processes to iterate jointly.
+		 * iterate jointly.
 		 */
-			pstate->tbmiterator = tbm_prepare_shared_iterate(tbm);
+		pstate->tbmiterator = tbm_prepare_shared_iterate(node->tbm);
 
 #ifdef USE_PREFETCH
 		if (node->prefetch_maximum > 0)
 		{
 			pstate->prefetch_iterator =
-					tbm_prepare_shared_iterate(tbm);
+				tbm_prepare_shared_iterate(node->tbm);
 		}
 #endif							/* USE_PREFETCH */
 
@@ -138,7 +114,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
 		BitmapDoneInitializingSharedState(pstate);
 	}
 
-		tbmiterator = tbm_begin_iterate(tbm, dsa,
+	tbmiterator = tbm_begin_iterate(node->tbm, dsa,
 									pstate ?
 									pstate->tbmiterator :
 									InvalidDsaPointer);
@@ -146,7 +122,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
 #ifdef USE_PREFETCH
 	if (node->prefetch_maximum > 0)
 		node->prefetch_iterator =
-				tbm_begin_iterate(tbm, dsa,
+			tbm_begin_iterate(node->tbm, dsa,
 							  pstate ?
 							  pstate->prefetch_iterator :
 							  InvalidDsaPointer);
@@ -156,33 +132,65 @@ BitmapHeapNext(BitmapHeapScanState *node)
 	 * If this is the first scan of the underlying table, create the table
 	 * scan descriptor and begin the scan.
 	 */
-		if (!scan)
+	if (!node->ss.ss_currentScanDesc)
 	{
 		bool		need_tuples = false;
 
 		/*
-			 * We can potentially skip fetching heap pages if we do not need
+		 * We can potentially skip fetching heap pages if we do not need any
-			 * any columns of the table, either for checking non-indexable
+		 * columns of the table, either for checking non-indexable quals or
-			 * quals or for returning data.  This test is a bit simplistic, as
+		 * for returning data.  This test is a bit simplistic, as it checks
-			 * it checks the stronger condition that there's no qual or return
+		 * the stronger condition that there's no qual or return tlist at all.
-			 * tlist at all. But in most cases it's probably not worth working
+		 * But in most cases it's probably not worth working harder than that.
-			 * harder than that.
 		 */
 		need_tuples = (node->ss.ps.plan->qual != NIL ||
 					   node->ss.ps.plan->targetlist != NIL);
 
-			scan = table_beginscan_bm(node->ss.ss_currentRelation,
+		node->ss.ss_currentScanDesc =
+			table_beginscan_bm(node->ss.ss_currentRelation,
 							   node->ss.ps.state->es_snapshot,
 							   0,
 							   NULL,
 							   need_tuples);
-
-			node->ss.ss_currentScanDesc = scan;
 	}
 
-		scan->st.rs_tbmiterator = tbmiterator;
+	node->ss.ss_currentScanDesc->st.rs_tbmiterator = tbmiterator;
 	node->initialized = true;
+}
 
+
+/* ----------------------------------------------------------------
+ *		BitmapHeapNext
+ *
+ *		Retrieve next tuple from the BitmapHeapScan node's currentRelation
+ * ----------------------------------------------------------------
+ */
+static TupleTableSlot *
+BitmapHeapNext(BitmapHeapScanState *node)
+{
+	ExprContext *econtext;
+	TableScanDesc scan;
+	TupleTableSlot *slot;
+
+#ifdef USE_PREFETCH
+	ParallelBitmapHeapState *pstate = node->pstate;
+#endif
+
+	/*
+	 * extract necessary information from index scan node
+	 */
+	econtext = node->ss.ps.ps_ExprContext;
+	slot = node->ss.ss_ScanTupleSlot;
+	scan = node->ss.ss_currentScanDesc;
+
+	/*
+	 * If we haven't yet performed the underlying index scan, do it, and begin
+	 * the iteration over the bitmap.
+	 */
+	if (!node->initialized)
+	{
+		BitmapTableScanSetup(node);
+		scan = node->ss.ss_currentScanDesc;
 		goto new_page;
 	}