From 0615979a9d86bb23353b8542e3940a88539251df Mon Sep 17 00:00:00 2001
From: jcoleman <jtc331@gmail.com>
Date: Fri, 2 Jun 2023 10:02:07 -0400
Subject: [PATCH v1 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 |  5 +++--
 3 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 51f645dda0..b106d55c53 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -3017,9 +3017,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));
@@ -3141,6 +3148,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
@@ -3625,13 +3633,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 c275b08494..9c145e1720 100644
--- a/src/backend/access/heap/hio.c
+++ b/src/backend/access/heap/hio.c
@@ -688,6 +688,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 af052f94af..a34b21770f 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -229,8 +229,9 @@ heap_page_prune_opt(Relation relation, Buffer buffer, bool already_locked)
 											   ndeleted - nnewlpdead);
 		}
 
-		/* And release buffer lock */
-		LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+		/* And release buffer lock if we acquired it */
+		if (!already_locked)
+			LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
 
 		/*
 		 * We avoid reuse of any free space created on the page by unrelated
-- 
2.39.2 (Apple Git-143)