Fix traversing to the deleted GIN page via downlink

akorotkov · akorotkov · commit 1414821e1688 · 2019-11-20T00:03:04.000+03:00
Current GIN code appears to don't handle traversing to the deleted page via downlink. This commit fixes that by stepping right from the delete page like we do in nbtree. This commit also fixes setting 'deleted' flag to the GIN pages. Now other page flags are not erased once page is deleted. That helps to keep our assertions true if we arrive deleted page via downlink. Discussion: https://postgr.es/m/CAPpHfdvMvsw-NcE5bRS7R1BbvA4BxoDnVVjkXC5W0Czvy9LVrg%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Peter Geoghegan Backpatch-through: 9.4
diff --git a/src/backend/access/gin/ginbtree.c b/src/backend/access/gin/ginbtree.c
@@ -175,13 +175,6 @@ ginStepRight(Buffer buffer, Relation index, int lockmode)
 	if (isLeaf != GinPageIsLeaf(page) || isData != GinPageIsData(page))
 		elog(ERROR, "right sibling of GIN page is of different type");
 
-	/*
-	 * Given the proper lock sequence above, we should never land on a deleted
-	 * page.
-	 */
-	if (GinPageIsDeleted(page))
-		elog(ERROR, "right sibling of GIN page was deleted");
-
 	return nextbuffer;
 }
 
diff --git a/src/backend/access/gin/gindatapage.c b/src/backend/access/gin/gindatapage.c
@@ -236,6 +236,9 @@ dataIsMoveRight(GinBtree btree, Page page)
 	if (GinPageRightMost(page))
 		return FALSE;
 
+	if (GinPageIsDeleted(page))
+		return TRUE;
+
 	return (ginCompareItemPointers(&btree->itemptr, iptr) > 0) ? TRUE : FALSE;
 }
 
diff --git a/src/backend/access/gin/ginvacuum.c b/src/backend/access/gin/ginvacuum.c
@@ -285,7 +285,7 @@ ginDeletePage(GinVacuumState *gvs, BlockNumber deleteBlkno, BlockNumber leftBlkn
 	 * we shouldn't change rightlink field to save workability of running
 	 * search scan
 	 */
-	GinPageGetOpaque(page)->flags = GIN_DELETED;
+	GinPageSetDeleted(page);
 
 	MarkBufferDirty(pBuffer);
 	MarkBufferDirty(lBuffer);
diff --git a/src/backend/access/gin/ginxlog.c b/src/backend/access/gin/ginxlog.c
@@ -744,7 +744,7 @@ ginRedoDeletePage(XLogRecPtr lsn, XLogRecord *record)
 				Assert(GinPageIsData(page));
 				if (record->xl_len == sizeof(ginxlogDeletePage))
 					GinPageSetDeleteXid(page, data->deleteXid);
-				GinPageGetOpaque(page)->flags = GIN_DELETED;
+				GinPageSetDeleted(page);
 				PageSetLSN(page, lsn);
 				MarkBufferDirty(dbuffer);
 			}

Original file line number	Diff line number	Diff line change
`@@ -236,6 +236,9 @@ dataIsMoveRight(GinBtree btree, Page page)`
`236`	`236`	`if (GinPageRightMost(page))`
`237`	`237`	`return FALSE;`
`238`	`238`
	`239`	`+ if (GinPageIsDeleted(page))`
	`240`	`+ return TRUE;`
	`241`	`+`
`239`	`242`	`return (ginCompareItemPointers(&btree->itemptr, iptr) > 0) ? TRUE : FALSE;`
`240`	`243`	`}`
`241`	`244`
Original file line number	Diff line number	Diff line change
`@@ -744,7 +744,7 @@ ginRedoDeletePage(XLogRecPtr lsn, XLogRecord *record)`
`744`	`744`	`Assert(GinPageIsData(page));`
`745`	`745`	`if (record->xl_len == sizeof(ginxlogDeletePage))`
`746`	`746`	`GinPageSetDeleteXid(page, data->deleteXid);`
`747`		`- GinPageGetOpaque(page)->flags = GIN_DELETED;`
	`747`	`+ GinPageSetDeleted(page);`
`748`	`748`	`PageSetLSN(page, lsn);`
`749`	`749`	`MarkBufferDirty(dbuffer);`
`750`	`750`	`}`