remove_HeapScanDescData_rs_inited_field.patch

text/plain

Filename: remove_HeapScanDescData_rs_inited_field.patch
Type: text/plain
Part: 0
Message: Re: heapgettup refactoring

Patch

Format: unified
File+
src/backend/access/heap/heapam.c 14 15
src/backend/access/heap/heapam_handler.c 1 7
src/include/access/heapam.h 12 4
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 7eb79cee58..e171d6e38b 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -321,13 +321,15 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock)
 	}
 
 	scan->rs_numblocks = InvalidBlockNumber;
-	scan->rs_inited = false;
 	scan->rs_ctup.t_data = NULL;
 	ItemPointerSetInvalid(&scan->rs_ctup.t_self);
 	scan->rs_cbuf = InvalidBuffer;
 	scan->rs_cblock = InvalidBlockNumber;
 
-	/* page-at-a-time fields are always invalid when not rs_inited */
+	/*
+	 * page-at-a-time fields are always invalid when
+	 * rs_cblock == InvalidBlockNumber
+	 */
 
 	/*
 	 * copy the scan key, if appropriate
@@ -355,7 +357,8 @@ heap_setscanlimits(TableScanDesc sscan, BlockNumber startBlk, BlockNumber numBlk
 {
 	HeapScanDesc scan = (HeapScanDesc) sscan;
 
-	Assert(!scan->rs_inited);	/* else too late to change */
+	/* we can't set any limits when there's a scan already running */
+	Assert(scan->rs_cblock == InvalidBlockNumber);
 	/* else rs_startblock is significant */
 	Assert(!(scan->rs_base.rs_flags & SO_ALLOW_SYNC));
 
@@ -493,7 +496,7 @@ heapgetpage(TableScanDesc sscan, BlockNumber block)
 static BlockNumber
 heapgettup_initial_block(HeapScanDesc scan, ScanDirection dir)
 {
-	Assert(!scan->rs_inited);
+	Assert(scan->rs_cblock == InvalidBlockNumber);
 
 	/* When there are no pages to scan, return InvalidBlockNumber */
 	if (scan->rs_nblocks == 0 || scan->rs_numblocks == 0)
@@ -559,7 +562,7 @@ heapgettup_start_page(HeapScanDesc scan, ScanDirection dir, int *linesleft,
 {
 	Page		page;
 
-	Assert(scan->rs_inited);
+	Assert(scan->rs_cblock != InvalidBlockNumber);
 	Assert(BufferIsValid(scan->rs_cbuf));
 
 	/* Caller is responsible for ensuring buffer is locked if needed */
@@ -592,7 +595,7 @@ heapgettup_continue_page(HeapScanDesc scan, ScanDirection dir, int *linesleft,
 {
 	Page		page;
 
-	Assert(scan->rs_inited);
+	Assert(scan->rs_cblock != InvalidBlockNumber);
 	Assert(BufferIsValid(scan->rs_cbuf));
 
 	/* Caller is responsible for ensuring buffer is locked if needed */
@@ -717,9 +720,9 @@ heapgettup_advance_block(HeapScanDesc scan, BlockNumber block, ScanDirection dir
  * the scankeys.
  *
  * Note: when we fall off the end of the scan in either direction, we
- * reset rs_inited.  This means that a further request with the same
- * scan direction will restart the scan, which is a bit odd, but a
- * request with the opposite scan direction will start a fresh scan
+ * reset rs_cblock to InvalidBlockNumber.  This means that a further request
+ * with the same scan direction will restart the scan, which is a bit odd, but
+ * a request with the opposite scan direction will start a fresh scan
  * in the proper direction.  The latter is required behavior for cursors,
  * while the former case is generally undefined behavior in Postgres
  * so we don't care too much.
@@ -737,12 +740,11 @@ heapgettup(HeapScanDesc scan,
 	OffsetNumber lineoff;
 	int			linesleft;
 
-	if (unlikely(!scan->rs_inited))
+	if (unlikely(scan->rs_cblock == InvalidBlockNumber))
 	{
 		block = heapgettup_initial_block(scan, dir);
 		/* ensure rs_cbuf is invalid when we get InvalidBlockNumber */
 		Assert(block != InvalidBlockNumber || !BufferIsValid(scan->rs_cbuf));
-		scan->rs_inited = true;
 	}
 	else
 	{
@@ -824,7 +826,6 @@ continue_page:
 	scan->rs_cbuf = InvalidBuffer;
 	scan->rs_cblock = InvalidBlockNumber;
 	tuple->t_data = NULL;
-	scan->rs_inited = false;
 }
 
 /* ----------------
@@ -852,12 +853,11 @@ heapgettup_pagemode(HeapScanDesc scan,
 	int			lineindex;
 	int			linesleft;
 
-	if (unlikely(!scan->rs_inited))
+	if (unlikely(scan->rs_cblock == InvalidBlockNumber))
 	{
 		block = heapgettup_initial_block(scan, dir);
 		/* ensure rs_cbuf is invalid when we get InvalidBlockNumber */
 		Assert(block != InvalidBlockNumber || !BufferIsValid(scan->rs_cbuf));
-		scan->rs_inited = true;
 	}
 	else
 	{
@@ -924,7 +924,6 @@ continue_page:
 	scan->rs_cbuf = InvalidBuffer;
 	scan->rs_cblock = InvalidBlockNumber;
 	tuple->t_data = NULL;
-	scan->rs_inited = false;
 }
 
 
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index c4b1916d36..e763ac61a6 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2279,14 +2279,9 @@ heapam_scan_sample_next_block(TableScanDesc scan, SampleScanState *scanstate)
 		/* scanning table sequentially */
 
 		if (hscan->rs_cblock == InvalidBlockNumber)
-		{
-			Assert(!hscan->rs_inited);
 			blockno = hscan->rs_startblock;
-		}
 		else
 		{
-			Assert(hscan->rs_inited);
-
 			blockno = hscan->rs_cblock + 1;
 
 			if (blockno >= hscan->rs_nblocks)
@@ -2321,13 +2316,12 @@ heapam_scan_sample_next_block(TableScanDesc scan, SampleScanState *scanstate)
 			ReleaseBuffer(hscan->rs_cbuf);
 		hscan->rs_cbuf = InvalidBuffer;
 		hscan->rs_cblock = InvalidBlockNumber;
-		hscan->rs_inited = false;
 
 		return false;
 	}
 
 	heapgetpage(scan, blockno);
-	hscan->rs_inited = true;
+	Assert(hscan->rs_cblock != InvalidBlockNumber);
 
 	return true;
 }
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 8d28bc93ef..c6efd59eb5 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -56,9 +56,18 @@ typedef struct HeapScanDescData
 	/* rs_numblocks is usually InvalidBlockNumber, meaning "scan whole rel" */
 
 	/* scan current state */
-	bool		rs_inited;		/* false = scan not init'd yet */
-	OffsetNumber rs_coffset;	/* current offset # in non-page-at-a-time mode */
-	BlockNumber rs_cblock;		/* current block # in scan, if any */
+	union
+	{
+		/* current offset in non-page-at-a-time mode */
+		OffsetNumber rs_coffset;
+
+		/* current tuple's index in vistuples for page-at-a-time mode */
+		int			rs_cindex;
+	};
+
+	BlockNumber rs_cblock;		/* current block # in scan, or
+								 * InvalidBlockNumber when the scan is not yet
+								 * initialized */
 	Buffer		rs_cbuf;		/* current buffer in scan, if any */
 	/* NB: if rs_cbuf is not InvalidBuffer, we hold a pin on that buffer */
 
@@ -74,7 +83,6 @@ 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;