v25-0006-Add-and-use-BitmapHeapScanDesc-struct.patch

application/octet-stream

Filename: v25-0006-Add-and-use-BitmapHeapScanDesc-struct.patch
Type: application/octet-stream
Part: 5
Message: Re: BitmapHeapScan streaming read user and prelim refactoring

Patch

Format: format-patch
Series: patch v25-0006
Subject: Add and use BitmapHeapScanDesc struct
File+
src/backend/access/heap/heapam.c 34 16
src/backend/access/heap/heapam_handler.c 11 7
src/include/access/heapam.h 16 8
src/tools/pgindent/typedefs.list 1 0
From 18fbbdeb912be86c17dd994c9d7c2812f28820d6 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Tue, 8 Oct 2024 18:56:40 -0400
Subject: [PATCH v25 06/11] Add and use BitmapHeapScanDesc struct

Move the several members of HeapScanDesc which are specific to Bitmap
Heap Scans into a new struct BitmapHeapScanDesc which inherits from
HeapScanDesc.

This reduces the size of the HeapScanDesc for other types of scans and
will allow us to add additional bitmap heap scan-specific members in
the future without fear of bloating the HeapScanDesc.

Author: Melanie Plageman
Reviewed-by: Tomas Vondra
Discussion: https://postgr.es/m/c736f6aa-8b35-4e20-9621-62c7c82e2168%40vondra.me
---
 src/backend/access/heap/heapam.c         | 50 ++++++++++++++++--------
 src/backend/access/heap/heapam_handler.c | 18 +++++----
 src/include/access/heapam.h              | 24 ++++++++----
 src/tools/pgindent/typedefs.list         |  1 +
 4 files changed, 62 insertions(+), 31 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index f9e3be1e846..90f6816922c 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -1046,7 +1046,16 @@ heap_beginscan(Relation relation, Snapshot snapshot,
 	/*
 	 * allocate and initialize scan descriptor
 	 */
-	scan = (HeapScanDesc) palloc(sizeof(HeapScanDescData));
+	if (flags & SO_TYPE_BITMAPSCAN)
+	{
+		BitmapHeapScanDesc bscan = palloc(sizeof(BitmapHeapScanDescData));
+
+		bscan->rs_vmbuffer = InvalidBuffer;
+		bscan->rs_empty_tuples_pending = 0;
+		scan = (HeapScanDesc) bscan;
+	}
+	else
+		scan = (HeapScanDesc) palloc(sizeof(HeapScanDescData));
 
 	scan->rs_base.rs_rd = relation;
 	scan->rs_base.rs_snapshot = snapshot;
@@ -1054,8 +1063,6 @@ heap_beginscan(Relation relation, Snapshot snapshot,
 	scan->rs_base.rs_flags = flags;
 	scan->rs_base.rs_parallel = parallel_scan;
 	scan->rs_strategy = NULL;	/* set in initscan */
-	scan->rs_vmbuffer = InvalidBuffer;
-	scan->rs_empty_tuples_pending = 0;
 
 	/*
 	 * Disable page-at-a-time mode if it's not a MVCC-safe snapshot.
@@ -1171,18 +1178,23 @@ heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params,
 	if (BufferIsValid(scan->rs_cbuf))
 		ReleaseBuffer(scan->rs_cbuf);
 
-	if (BufferIsValid(scan->rs_vmbuffer))
+	if (scan->rs_base.rs_flags & SO_TYPE_BITMAPSCAN)
 	{
-		ReleaseBuffer(scan->rs_vmbuffer);
-		scan->rs_vmbuffer = InvalidBuffer;
-	}
+		BitmapHeapScanDesc bscan = (BitmapHeapScanDesc) scan;
 
-	/*
-	 * Reset rs_empty_tuples_pending, a field only used by bitmap heap scan,
-	 * to avoid incorrectly emitting NULL-filled tuples from a previous scan
-	 * on rescan.
-	 */
-	scan->rs_empty_tuples_pending = 0;
+		/*
+		 * Reset empty_tuples_pending, a field only used by bitmap heap scan,
+		 * to avoid incorrectly emitting NULL-filled tuples from a previous
+		 * scan on rescan.
+		 */
+		bscan->rs_empty_tuples_pending = 0;
+
+		if (BufferIsValid(bscan->rs_vmbuffer))
+		{
+			ReleaseBuffer(bscan->rs_vmbuffer);
+			bscan->rs_vmbuffer = InvalidBuffer;
+		}
+	}
 
 	/*
 	 * The read stream is reset on rescan. This must be done before
@@ -1211,9 +1223,6 @@ heap_endscan(TableScanDesc sscan)
 	if (BufferIsValid(scan->rs_cbuf))
 		ReleaseBuffer(scan->rs_cbuf);
 
-	if (BufferIsValid(scan->rs_vmbuffer))
-		ReleaseBuffer(scan->rs_vmbuffer);
-
 	/*
 	 * Must free the read stream before freeing the BufferAccessStrategy.
 	 */
@@ -1237,6 +1246,15 @@ heap_endscan(TableScanDesc sscan)
 	if (scan->rs_base.rs_flags & SO_TEMP_SNAPSHOT)
 		UnregisterSnapshot(scan->rs_base.rs_snapshot);
 
+	if (scan->rs_base.rs_flags & SO_TYPE_BITMAPSCAN)
+	{
+		BitmapHeapScanDesc bscan = (BitmapHeapScanDesc) sscan;
+
+		bscan->rs_empty_tuples_pending = 0;
+		if (BufferIsValid(bscan->rs_vmbuffer))
+			ReleaseBuffer(bscan->rs_vmbuffer);
+	}
+
 	pfree(scan);
 }
 
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index c51f5e921f1..192a326a019 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2118,13 +2118,16 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
 							  BlockNumber *blockno, bool *recheck,
 							  uint64 *lossy_pages, uint64 *exact_pages)
 {
-	HeapScanDesc hscan = (HeapScanDesc) scan;
+	BitmapHeapScanDesc bscan = (BitmapHeapScanDesc) scan;
+	HeapScanDesc hscan = (HeapScanDesc) bscan;
 	BlockNumber block;
 	Buffer		buffer;
 	Snapshot	snapshot;
 	int			ntup;
 	TBMIterateResult *tbmres;
 
+	Assert(scan->rs_flags & SO_TYPE_BITMAPSCAN);
+
 	hscan->rs_cindex = 0;
 	hscan->rs_ntuples = 0;
 
@@ -2162,13 +2165,13 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
 	 */
 	if (!(scan->rs_flags & SO_NEED_TUPLES) &&
 		!tbmres->recheck &&
-		VM_ALL_VISIBLE(scan->rs_rd, tbmres->blockno, &hscan->rs_vmbuffer))
+		VM_ALL_VISIBLE(scan->rs_rd, tbmres->blockno, &bscan->rs_vmbuffer))
 	{
 		/* can't be lossy in the skip_fetch case */
 		Assert(tbmres->ntuples >= 0);
-		Assert(hscan->rs_empty_tuples_pending >= 0);
+		Assert(bscan->rs_empty_tuples_pending >= 0);
 
-		hscan->rs_empty_tuples_pending += tbmres->ntuples;
+		bscan->rs_empty_tuples_pending += tbmres->ntuples;
 
 		return true;
 	}
@@ -2282,18 +2285,19 @@ static bool
 heapam_scan_bitmap_next_tuple(TableScanDesc scan,
 							  TupleTableSlot *slot)
 {
-	HeapScanDesc hscan = (HeapScanDesc) scan;
+	BitmapHeapScanDesc bscan = (BitmapHeapScanDesc) scan;
+	HeapScanDesc hscan = (HeapScanDesc) bscan;
 	OffsetNumber targoffset;
 	Page		page;
 	ItemId		lp;
 
-	if (hscan->rs_empty_tuples_pending > 0)
+	if (bscan->rs_empty_tuples_pending > 0)
 	{
 		/*
 		 * If we don't have to fetch the tuple, just return nulls.
 		 */
 		ExecStoreAllNullTuple(slot);
-		hscan->rs_empty_tuples_pending--;
+		bscan->rs_empty_tuples_pending--;
 		return true;
 	}
 
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index b951466ced2..a789c608f1d 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -92,22 +92,30 @@ typedef struct HeapScanDescData
 	 */
 	ParallelBlockTableScanWorkerData *rs_parallelworkerdata;
 
+	/* these fields only used in page-at-a-time mode and for bitmap scans */
+	int			rs_cindex;		/* current tuple's index in vistuples */
+	int			rs_ntuples;		/* number of visible tuples on page */
+	OffsetNumber rs_vistuples[MaxHeapTuplesPerPage];	/* their offsets */
+}			HeapScanDescData;
+typedef struct HeapScanDescData *HeapScanDesc;
+
+typedef struct BitmapHeapScanDescData
+{
+	HeapScanDescData rs_heap_base;
+
 	/*
 	 * These fields are only used for bitmap scans for the "skip fetch"
 	 * optimization. Bitmap scans needing no fields from the heap may skip
 	 * fetching an all visible block, instead using the number of tuples per
 	 * block reported by the bitmap to determine how many NULL-filled tuples
-	 * to return.
+	 * to return. They are common to parallel and serial BitmapHeapScans
 	 */
+
+	/* page of VM containing info for current block */
 	Buffer		rs_vmbuffer;
 	int			rs_empty_tuples_pending;
-
-	/* these fields only used in page-at-a-time mode and for bitmap scans */
-	int			rs_cindex;		/* current tuple's index in vistuples */
-	int			rs_ntuples;		/* number of visible tuples on page */
-	OffsetNumber rs_vistuples[MaxHeapTuplesPerPage];	/* their offsets */
-}			HeapScanDescData;
-typedef struct HeapScanDescData *HeapScanDesc;
+}			BitmapHeapScanDescData;
+typedef struct BitmapHeapScanDescData *BitmapHeapScanDesc;
 
 /*
  * Descriptor for fetches from heap via an index.
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index f219e806c8b..88045252768 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -263,6 +263,7 @@ BitmapAndState
 BitmapHeapPath
 BitmapHeapScan
 BitmapHeapScanInstrumentation
+BitmapHeapScanDesc
 BitmapHeapScanState
 BitmapIndexScan
 BitmapIndexScanState
-- 
2.45.2