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

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
-- 
2.39.3 (Apple Git-145)