From 2b4c36f69548c1538463fee5e60612706ceaece8 Mon Sep 17 00:00:00 2001
From: jcoleman <jtc331@gmail.com>
Date: Fri, 2 Jun 2023 10:02:07 -0400
Subject: [PATCH v3 2/2] Opportunistically prune to avoid building a new page
 for an update

---
 src/backend/access/heap/heapam.c    | 34 +++++++++++++++++++++++++++++
 src/backend/access/heap/hio.c       | 20 +++++++++++++++++
 src/backend/access/heap/pruneheap.c |  2 +-
 src/include/access/heapam.h         |  2 +-
 4 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index e4b659f944..bd70714b50 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -3025,9 +3025,16 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
 				infomask2_old_tuple,
 				infomask_new_tuple,
 				infomask2_new_tuple;
+#ifdef USE_ASSERT_CHECKING
+	ItemPointerData original_otid;
+#endif
 
 	Assert(ItemPointerIsValid(otid));
 
+#ifdef USE_ASSERT_CHECKING
+	ItemPointerCopy(otid, &original_otid);
+#endif
+
 	/* Cheap, simplistic check that the tuple matches the rel's rowtype. */
 	Assert(HeapTupleHeaderGetNatts(newtup->t_data) <=
 		   RelationGetNumberOfAttributes(relation));
@@ -3149,6 +3156,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
 	}
 
 	/*
+	 * TODO: is this note the problem pointer?
 	 * Note: beyond this point, use oldtup not otid to refer to old tuple.
 	 * otid may very well point at newtup->t_self, which we will overwrite
 	 * with the new tuple's location, so there's great risk of confusion if we
@@ -3635,13 +3643,39 @@ l2:
 			if (newtupsize > pagefree)
 			{
 				/* It doesn't fit, must use RelationGetBufferForTuple. */
+				/* TODO: every time we call this we need to make sure we don't have pointers into the page */
 				newbuf = RelationGetBufferForTuple(relation, heaptup->t_len,
 												   buffer, 0, NULL,
 												   &vmbuffer_new, &vmbuffer,
 												   0);
+
+#ifdef USE_ASSERT_CHECKING
+				/*
+				 * About 500 lines ago in this function a comment claimed we
+				 * might not be able to rely on otid after that point, but it
+				 * appears we can.
+				 */
+				Assert(ItemPointerEquals(otid, &original_otid));
+#endif
+
+				/*
+				 * Getting a buffer may have pruned the page, so we can't rely
+				 * on our original pointer into the page.
+				 */
+				lp = PageGetItemId(page, ItemPointerGetOffsetNumber(otid));
+				Assert(ItemIdIsNormal(lp));
+
+				/*
+				 * Mirror what we filled into oldtup at the beginning
+				 * of this function.
+				 */
+				oldtup.t_data = (HeapTupleHeader) PageGetItem(page, lp);
+				oldtup.t_len = ItemIdGetLength(lp);
+
 				/* We're all done. */
 				break;
 			}
+
 			/* Acquire VM page pin if needed and we don't have it. */
 			if (vmbuffer == InvalidBuffer && PageIsAllVisible(page))
 				visibilitymap_pin(relation, block, &vmbuffer);
diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c
index c7248d7c68..bb377596ab 100644
--- a/src/backend/access/heap/hio.c
+++ b/src/backend/access/heap/hio.c
@@ -707,6 +707,26 @@ loop:
 			RelationSetTargetBlock(relation, targetBlock);
 			return buffer;
 		}
+		else
+		{
+			/*
+			 * Opportunistically prune and see if that frees up enough space to
+			 * avoid needing to build a new page.
+			 */
+			heap_page_prune_opt(relation, buffer, true);
+
+			/* If pruning freed up any slots, we can check free space again. */
+			if (PageHasFreeLinePointers(page))
+			{
+				pageFreeSpace = PageGetHeapFreeSpace(page);
+				if (targetFreeSpace <= pageFreeSpace)
+				{
+					/* use this page as future insert target, too */
+					RelationSetTargetBlock(relation, targetBlock);
+					return buffer;
+				}
+			}
+		}
 
 		/*
 		 * Not enough space, so we must give up our page locks and pin (if
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 1334ffec01..0c9c6f5d12 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -215,7 +215,7 @@ heap_page_prune_opt(Relation relation, Buffer buffer, bool already_locked)
  * tuples removed and the number of line pointers newly marked LP_DEAD.
  * heap_page_prune() is responsible for initializing it.
  */
-void
+int
 heap_page_prune(Relation relation, Buffer buffer,
 				GlobalVisState *vistest,
 				bool mark_unused_now,
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 18ce464a2a..ebc6a9d3ae 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -318,7 +318,7 @@ extern TransactionId heap_index_delete_tuples(Relation rel,
 /* in heap/pruneheap.c */
 struct GlobalVisState;
 extern void	heap_page_prune_opt(Relation relation, Buffer buffer, bool already_locked);
-extern void	heap_page_prune(Relation relation, Buffer buffer,
+extern int	heap_page_prune(Relation relation, Buffer buffer,
 							struct GlobalVisState *vistest,
 							bool mark_unused_now,
 							PruneResult *presult,
-- 
2.39.3 (Apple Git-145)