v3-0001-Add-no-movement-scan-helper.patch
text/x-patch
Filename: v3-0001-Add-no-movement-scan-helper.patch
Type: text/x-patch
Part: 3
Message:
Re: heapgettup refactoring
Patch
Format: format-patch
Series: patch v3-0001
Subject: Add no movement scan helper
| File | + | − |
|---|---|---|
| src/backend/access/heap/heapam.c | 56 | 62 |
From 38d83796ff3f3060a8dcb244c85864279f080450 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Wed, 30 Nov 2022 10:20:30 -0500
Subject: [PATCH v3 1/7] Add no movement scan helper
No movement scan can be handled first in heapgettup() and
heapgettup_pagemode(). Refactor this case into its own helper function
to improve readability.
---
src/backend/access/heap/heapam.c | 118 +++++++++++++++----------------
1 file changed, 56 insertions(+), 62 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 747db50376..f15296a67a 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -484,6 +484,46 @@ heapgetpage(TableScanDesc sscan, BlockNumber block)
scan->rs_ntuples = ntup;
}
+/*
+ * ``no movement'' scan direction: refetch prior tuple
+ */
+static inline void
+heapgettup_no_movement(HeapScanDesc scan)
+{
+ ItemId lpp;
+ OffsetNumber lineoff;
+ BlockNumber page;
+ Page dp;
+ HeapTuple tuple = &(scan->rs_ctup);
+
+ /* The scan must be init'd for there to be a current tuple (rs_ctup) */
+ Assert(scan->rs_inited);
+
+ /* Since the tuple was previously fetched, needn't lock page here */
+ page = ItemPointerGetBlockNumber(&(tuple->t_self));
+ if (page != scan->rs_cblock)
+ heapgetpage((TableScanDesc) scan, page);
+
+ /* Since the tuple was previously fetched, needn't lock page here */
+ dp = BufferGetPage(scan->rs_cbuf);
+ TestForOldSnapshot(scan->rs_base.rs_snapshot, scan->rs_base.rs_rd, dp);
+ lineoff = scan->rs_cindex;
+ lpp = PageGetItemId(dp, lineoff);
+ Assert(ItemIdIsNormal(lpp));
+
+ tuple->t_data = (HeapTupleHeader) PageGetItem((Page) dp, lpp);
+ tuple->t_len = ItemIdGetLength(lpp);
+
+ /* check that rs_cindex is in sync if in pagemode */
+ Assert(!(scan->rs_base.rs_flags & SO_ALLOW_PAGEMODE) ||
+ (scan->rs_cindex < scan->rs_ntuples));
+
+ Assert(!(scan->rs_base.rs_flags & SO_ALLOW_PAGEMODE) ||
+ (lineoff == scan->rs_vistuples[scan->rs_cindex]));
+
+ return;
+}
+
/* ----------------
* heapgettup - fetch next heap tuple
*
@@ -524,6 +564,12 @@ heapgettup(HeapScanDesc scan,
int linesleft;
ItemId lpp;
+ if (unlikely(ScanDirectionIsNoMovement(dir)))
+ {
+ heapgettup_no_movement(scan);
+ return;
+ }
+
/*
* calculate next starting lineoff, given scan direction
*/
@@ -584,8 +630,9 @@ heapgettup(HeapScanDesc scan,
linesleft = lines - lineoff + 1;
}
- else if (backward)
+ else
{
+ Assert(backward);
/* backward parallel scan not supported */
Assert(scan->rs_base.rs_parallel == NULL);
@@ -654,34 +701,6 @@ heapgettup(HeapScanDesc scan,
linesleft = lineoff;
}
- else
- {
- /*
- * ``no movement'' scan direction: refetch prior tuple
- */
- if (!scan->rs_inited)
- {
- Assert(!BufferIsValid(scan->rs_cbuf));
- tuple->t_data = NULL;
- return;
- }
-
- block = ItemPointerGetBlockNumber(&(tuple->t_self));
- if (block != scan->rs_cblock)
- heapgetpage((TableScanDesc) scan, block);
-
- /* Since the tuple was previously fetched, needn't lock page here */
- page = BufferGetPage(scan->rs_cbuf);
- TestForOldSnapshot(snapshot, scan->rs_base.rs_rd, page);
- lineoff = ItemPointerGetOffsetNumber(&(tuple->t_self));
- lpp = PageGetItemId(page, lineoff);
- Assert(ItemIdIsNormal(lpp));
-
- tuple->t_data = (HeapTupleHeader) PageGetItem(page, lpp);
- tuple->t_len = ItemIdGetLength(lpp);
-
- return;
- }
/*
* advance the scan until we find a qualifying tuple or run out of stuff
@@ -862,6 +881,12 @@ heapgettup_pagemode(HeapScanDesc scan,
int linesleft;
ItemId lpp;
+ if (unlikely(ScanDirectionIsNoMovement(dir)))
+ {
+ heapgettup_no_movement(scan);
+ return;
+ }
+
/*
* calculate next starting lineindex, given scan direction
*/
@@ -919,8 +944,9 @@ heapgettup_pagemode(HeapScanDesc scan,
linesleft = lines - lineindex;
}
- else if (backward)
+ else
{
+ Assert(backward);
/* backward parallel scan not supported */
Assert(scan->rs_base.rs_parallel == NULL);
@@ -979,38 +1005,6 @@ heapgettup_pagemode(HeapScanDesc scan,
linesleft = lineindex + 1;
}
- else
- {
- /*
- * ``no movement'' scan direction: refetch prior tuple
- */
- if (!scan->rs_inited)
- {
- Assert(!BufferIsValid(scan->rs_cbuf));
- tuple->t_data = NULL;
- return;
- }
-
- block = ItemPointerGetBlockNumber(&(tuple->t_self));
- if (block != scan->rs_cblock)
- heapgetpage((TableScanDesc) scan, block);
-
- /* Since the tuple was previously fetched, needn't lock page here */
- page = BufferGetPage(scan->rs_cbuf);
- TestForOldSnapshot(scan->rs_base.rs_snapshot, scan->rs_base.rs_rd, page);
- lineoff = ItemPointerGetOffsetNumber(&(tuple->t_self));
- lpp = PageGetItemId(page, lineoff);
- Assert(ItemIdIsNormal(lpp));
-
- tuple->t_data = (HeapTupleHeader) PageGetItem(page, lpp);
- tuple->t_len = ItemIdGetLength(lpp);
-
- /* check that rs_cindex is in sync */
- Assert(scan->rs_cindex < scan->rs_ntuples);
- Assert(lineoff == scan->rs_vistuples[scan->rs_cindex]);
-
- return;
- }
/*
* advance the scan until we find a qualifying tuple or run out of stuff
--
2.34.1