suggested_edits.diff

diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index a8ed11a1858..2f477aa43b1 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -143,7 +143,7 @@ static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber o
 static void heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum, bool was_normal);
 
 static void heap_prune_record_unchanged(Page page, PruneState *prstate, OffsetNumber offnum);
-static void heap_prune_record_unchanged_lp_dead(PruneState *prstate, OffsetNumber offnum);
+static void heap_prune_record_unchanged_lp_dead(ItemId itemid, PruneState *prstate, OffsetNumber offnum);
 static void heap_prune_record_unchanged_lp_redirect(PruneState *prstate, OffsetNumber offnum);
 
 static void page_verify_redirects(Page page);
@@ -766,7 +766,8 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 	 * we can advance relfrozenxid and relminmxid to the values in
 	 * pagefrz->FreezePageRelfrozenXid and pagefrz->FreezePageRelminMxid.
 	 * MFIXME: which one should be pick if presult->nfrozen == 0 and
-	 * presult->all_frozen = True.
+	 * presult->all_frozen = True. MTODO: see Peter's response here
+	 * https://www.postgresql.org/message-id/CAH2-Wz%3DLmOs%3DiJ%3DFfCERnma0q7QjaNSnCgWEp7zOK7hD24YC_w%40mail.gmail.com
 	 */
 	if (new_relfrozen_xid)
 	{
@@ -868,9 +869,12 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
 	OffsetNumber latestdead = InvalidOffsetNumber,
 				maxoff = PageGetMaxOffsetNumber(dp),
 				offnum;
+
+	/* TODO: while maybe self-explanatory, I would prefer if chainitems and */
+	/* nchain had a comment up here */
 	OffsetNumber chainitems[MaxHeapTuplesPerPage];
-	int			nchain = 0,
-				i;
+	int			nchain = 0;
+	int			i = 0;
 
 	rootlp = PageGetItemId(dp, rootoffnum);
 
@@ -943,6 +947,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
 			 */
 			prstate->revisit[prstate->nrevisit++] = rootoffnum;
 
+			/* TODO: I don't like this comment now */
 			/* Nothing more to do */
 			return;
 		}
@@ -1020,7 +1025,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
 			if (unlikely(prstate->actions & PRUNE_DO_MARK_UNUSED_NOW))
 				heap_prune_record_unused(prstate, offnum, false);
 			else
-				heap_prune_record_unchanged_lp_dead(prstate, offnum);
+				heap_prune_record_unchanged_lp_dead(lp, prstate, offnum);
 			break;
 		}
 
@@ -1044,6 +1049,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
 		 */
 		tupdead = recent_dead = false;
 
+		/* TODO: maybe this should just be an if statement now */
 		switch (htsv_get_valid_status(prstate->htsv[offnum]))
 		{
 			case HEAPTUPLE_DEAD:
@@ -1132,42 +1138,34 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
 		 * dead and the root line pointer can be marked dead.  Otherwise just
 		 * redirect the root to the correct chain member.
 		 */
-		if (i >= nchain)
-			heap_prune_record_dead_or_unused(prstate, rootoffnum, ItemIdIsNormal(rootlp));
-		else
-		{
+		if (i < nchain)
 			heap_prune_record_redirect(prstate, rootoffnum, chainitems[i], ItemIdIsNormal(rootlp));
-
-			/* the rest of tuples in the chain are normal, unchanged tuples */
-			for (; i < nchain; i++)
-				heap_prune_record_unchanged(dp, prstate, chainitems[i]);
-		}
+		else
+			heap_prune_record_dead_or_unused(prstate, rootoffnum, ItemIdIsNormal(rootlp));
 	}
-	else
+	else if ((i = ItemIdIsRedirected(rootlp)))
 	{
-		i = 0;
-		if (ItemIdIsRedirected(rootlp))
+		if (i < nchain)
 		{
-			if (nchain < 2)
-			{
-				/*
-				 * We found a redirect item that doesn't point to a valid
-				 * follow-on item.  This can happen if the loop in
-				 * heap_page_prune_and_freeze() caused us to visit the dead
-				 * successor of a redirect item before visiting the redirect
-				 * item.  We can clean up by setting the redirect item to DEAD
-				 * state or LP_UNUSED if the caller indicated.
-				 */
-				heap_prune_record_dead_or_unused(prstate, rootoffnum, false);
-			}
-			else
-				heap_prune_record_unchanged_lp_redirect(prstate, rootoffnum);
-			i++;
+			heap_prune_record_unchanged_lp_redirect(prstate, rootoffnum);
+		}
+		else
+		{
+			/*
+			 * We found a redirect item that doesn't point to a valid
+			 * follow-on item.  This can happen if the loop in
+			 * heap_page_prune_and_freeze() caused us to visit the dead
+			 * successor of a redirect item before visiting the redirect item.
+			 * We can clean up by setting the redirect item to DEAD state or
+			 * LP_UNUSED if the caller indicated.
+			 */
+			heap_prune_record_dead_or_unused(prstate, rootoffnum, false);
 		}
-		/* the rest of tuples in the chain are normal, unchanged tuples */
-		for (; i < nchain; i++)
-			heap_prune_record_unchanged(dp, prstate, chainitems[i]);
 	}
+
+	/* the rest of tuples in the chain are normal, unchanged tuples */
+	for (; i < nchain; i++)
+		heap_prune_record_unchanged(dp, prstate, chainitems[i]);
 }
 
 /* Record lowest soon-prunable XID */
@@ -1486,7 +1484,7 @@ heap_prune_record_unchanged(Page page, PruneState *prstate, OffsetNumber offnum)
  * Record line pointer that was already LP_DEAD and is left unchanged.
  */
 static void
-heap_prune_record_unchanged_lp_dead(PruneState *prstate, OffsetNumber offnum)
+heap_prune_record_unchanged_lp_dead(ItemId itemid, PruneState *prstate, OffsetNumber offnum)
 {
 	/*
 	 * Deliberately don't set hastup for LP_DEAD items.  We make the soft
@@ -1506,6 +1504,7 @@ heap_prune_record_unchanged_lp_dead(PruneState *prstate, OffsetNumber offnum)
 	Assert(!prstate->marked[offnum]);
 	prstate->marked[offnum] = true;
 
+	Assert(!ItemIdHasStorage(itemid));
 	prstate->deadoffsets[prstate->lpdead_items++] = offnum;
 }