From 9f971e7802d041573024eede7e0072372ef1d6e6 Mon Sep 17 00:00:00 2001
From: Siva R <sivasubr@amazon.com>
Date: Tue, 4 Sep 2018 19:19:16 +0000
Subject: [PATCH] Rewrite ginRedoRecompress to use a scratch space for
 processing replay actions instead of modifying contents on the actual page to
 avoid accidental overwrites of opaque data.

---
 src/backend/access/gin/ginxlog.c | 180 +++++++++++++++++++++------------------
 1 file changed, 98 insertions(+), 82 deletions(-)

diff --git a/src/backend/access/gin/ginxlog.c b/src/backend/access/gin/ginxlog.c
index 7515f8b..7ea798f 100644
--- a/src/backend/access/gin/ginxlog.c
+++ b/src/backend/access/gin/ginxlog.c
@@ -138,17 +138,21 @@ ginRedoRecompress(Page page, ginxlogRecompressDataLeaf *data)
 {
 	int			actionno;
 	int			segno;
-	GinPostingList *oldseg;
+	GinPostingList *segiter;
+	Pointer		segmentcopybegin;
 	Pointer		segmentend;
 	char	   *walbuf;
 	int			totalsize;
+	Pointer		scratch;
+	Pointer		scratchbegin;
+	bool		modified;
 
 	/*
 	 * If the page is in pre-9.4 format, convert to new format first.
 	 */
 	if (!GinPageIsCompressed(page))
 	{
-		ItemPointer uncompressed = (ItemPointer) GinDataPageGetData(page);
+		ItemPointer uncompressed = (ItemPointer)GinDataPageGetData(page);
 		int			nuncompressed = GinPageGetOpaque(page)->maxoff;
 		int			npacked;
 
@@ -180,63 +184,77 @@ ginRedoRecompress(Page page, ginxlogRecompressDataLeaf *data)
 		GinPageGetOpaque(page)->maxoff = InvalidOffsetNumber;
 	}
 
-	oldseg = GinDataLeafPageGetPostingList(page);
-	segmentend = (Pointer) oldseg + GinDataLeafPageGetPostingListSize(page);
-	segno = 0;
+	scratch = palloc(GinDataPageMaxDataSize);
+
+	segiter = GinDataLeafPageGetPostingList(page);
+	segmentend = (Pointer)segiter + GinDataLeafPageGetPostingListSize(page);
+	segmentcopybegin = segmentend;
 
-	walbuf = ((char *) data) + sizeof(ginxlogRecompressDataLeaf);
+	segno = 0;
+	modified = false;
+	scratchbegin = scratch;
+	walbuf = ((char *)data) + sizeof(ginxlogRecompressDataLeaf);
 	for (actionno = 0; actionno < data->nactions; actionno++)
 	{
-		uint8		a_segno = *((uint8 *) (walbuf++));
-		uint8		a_action = *((uint8 *) (walbuf++));
+		uint8		a_segno = *((uint8 *)(walbuf++));
+		uint8		a_action = *((uint8 *)(walbuf++));
 		GinPostingList *newseg = NULL;
 		int			newsegsize = 0;
 		ItemPointerData *items = NULL;
 		uint16		nitems = 0;
-		ItemPointerData *olditems;
-		int			nolditems;
-		ItemPointerData *newitems;
-		int			nnewitems;
-		int			segsize;
-		Pointer		segptr;
-		int			szleft;
-
-		/* Extract all the information we need from the WAL record */
-		if (a_action == GIN_SEGMENT_INSERT ||
-			a_action == GIN_SEGMENT_REPLACE)
-		{
-			newseg = (GinPostingList *) walbuf;
-			newsegsize = SizeOfGinPostingList(newseg);
-			walbuf += SHORTALIGN(newsegsize);
-		}
-
-		if (a_action == GIN_SEGMENT_ADDITEMS)
-		{
-			memcpy(&nitems, walbuf, sizeof(uint16));
-			walbuf += sizeof(uint16);
-			items = (ItemPointerData *) walbuf;
-			walbuf += nitems * sizeof(ItemPointerData);
-		}
 
 		/* Skip to the segment that this action concerns */
 		Assert(segno <= a_segno);
 		while (segno < a_segno)
 		{
-			oldseg = GinNextPostingListSegment(oldseg);
+			if (modified)
+			{
+				int			cursegsize;
+
+				/* Copy out the segment to scratch space */
+				cursegsize = SizeOfGinPostingList(segiter);
+				Assert(scratch + cursegsize - scratchbegin <= GinDataPageMaxDataSize);
+				memcpy(scratch, segiter, cursegsize);
+				scratch += cursegsize;
+			}
+
+			/* Move to next segment in the page */
+			segiter = GinNextPostingListSegment(segiter);
 			segno++;
 		}
 
 		/*
-		 * ADDITEMS action is handled like REPLACE, but the new segment to
-		 * replace the old one is reconstructed using the old segment from
-		 * disk and the new items from the WAL record.
+		 * Encountered first segment with an action. Every subsequent segment
+		 * needs to be in scratch space.
+		 */
+		if (!modified)
+		{
+			modified = true;
+			segmentcopybegin = (Pointer)segiter;
+		}
+
+		/*
+		 * Positioned after the last existing segment. Only INSERTs expected
+		 * here.
 		 */
+		if ((Pointer)segiter == segmentend)
+			Assert(a_action == GIN_SEGMENT_INSERT);
+
+		/* Process the action on the segment */
 		if (a_action == GIN_SEGMENT_ADDITEMS)
 		{
+			ItemPointerData *olditems;
+			int			nolditems;
+			ItemPointerData *newitems;
+			int			nnewitems;
 			int			npacked;
 
-			olditems = ginPostingListDecode(oldseg, &nolditems);
+			memcpy(&nitems, walbuf, sizeof(uint16));
+			walbuf += sizeof(uint16);
+			items = (ItemPointerData *)walbuf;
+			walbuf += nitems * sizeof(ItemPointerData);
 
+			olditems = ginPostingListDecode(segiter, &nolditems);
 			newitems = ginMergeItemPointers(items, nitems,
 											olditems, nolditems,
 											&nnewitems);
@@ -247,61 +265,59 @@ ginRedoRecompress(Page page, ginxlogRecompressDataLeaf *data)
 			Assert(npacked == nnewitems);
 
 			newsegsize = SizeOfGinPostingList(newseg);
-			a_action = GIN_SEGMENT_REPLACE;
+			memcpy(scratch, newseg, newsegsize);
+			scratch += newsegsize;
 		}
+		else if (a_action == GIN_SEGMENT_INSERT ||
+				 a_action == GIN_SEGMENT_REPLACE)
+		{
+			newseg = (GinPostingList *)walbuf;
+			newsegsize = SizeOfGinPostingList(newseg);
+			walbuf += SHORTALIGN(SizeOfGinPostingList(newseg));
 
-		segptr = (Pointer) oldseg;
-		if (segptr != segmentend)
-			segsize = SizeOfGinPostingList(oldseg);
+			/* Copy the new/replaced segment into scratch space */
+			memcpy(scratch, newseg, newsegsize);
+			scratch += newsegsize;
+		}
+		else if (a_action == GIN_SEGMENT_DELETE)
+		{
+			/* No op */
+		}
 		else
 		{
-			/*
-			 * Positioned after the last existing segment. Only INSERTs
-			 * expected here.
-			 */
-			Assert(a_action == GIN_SEGMENT_INSERT);
-			segsize = 0;
+			elog(ERROR, "unexpected GIN leaf action: %u", a_action);
 		}
-		szleft = segmentend - segptr;
 
-		switch (a_action)
+		/*
+		 * Advance to next segment on original page if the current segment
+		 * action is not insert.
+		 */
+		if (a_action != GIN_SEGMENT_INSERT)
 		{
-			case GIN_SEGMENT_DELETE:
-				memmove(segptr, segptr + segsize, szleft - segsize);
-				segmentend -= segsize;
-
-				segno++;
-				break;
-
-			case GIN_SEGMENT_INSERT:
-				/* make room for the new segment */
-				memmove(segptr + newsegsize, segptr, szleft);
-				/* copy the new segment in place */
-				memcpy(segptr, newseg, newsegsize);
-				segmentend += newsegsize;
-				segptr += newsegsize;
-				break;
-
-			case GIN_SEGMENT_REPLACE:
-				/* shift the segments that follow */
-				memmove(segptr + newsegsize,
-						segptr + segsize,
-						szleft - segsize);
-				/* copy the replacement segment in place */
-				memcpy(segptr, newseg, newsegsize);
-				segmentend -= segsize;
-				segmentend += newsegsize;
-				segptr += newsegsize;
-				segno++;
-				break;
-
-			default:
-				elog(ERROR, "unexpected GIN leaf action: %u", a_action);
+			segiter = GinNextPostingListSegment(segiter);
+			segno++;
 		}
-		oldseg = (GinPostingList *) segptr;
 	}
 
-	totalsize = segmentend - (Pointer) GinDataLeafPageGetPostingList(page);
+	/* Assert that there was some modification performed */
+	Assert(modified);
+
+	/* Copy out remainder of the page into scratch */
+	if ((Pointer)segiter < segmentend)
+		memcpy(scratch, segiter, (segmentend - ((Pointer)segiter)));
+
+	/*
+	 * Check that the new total size is within the max data page size allowed.
+	 * New total size is sum of scratch space size consumed and portion of
+	 * page that was unmodified.
+	 */
+	totalsize = (scratch - scratchbegin) + (segmentcopybegin - (Pointer)GinDataLeafPageGetPostingList(page));
+	Assert(totalsize <= GinDataPageMaxDataSize);
+
+	/* Copy the items back onto the page */
+	memcpy(segmentcopybegin, scratchbegin, (scratch - scratchbegin));
+
+	/* Set size on the page */
 	GinDataPageSetDataSize(page, totalsize);
 }
 
-- 
2.7.3.AMZN

