Add simple sanity checks on newly-read pages to GiST, too.

Author: tglsfdc

src/backend/access/gist/gist.c

@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/gist/gist.c,v 1.127 2005/10/18 01:06:22 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/gist/gist.c,v 1.128 2005/11/06 22:39:20 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -529,12 +529,12 @@ gistfindleaf(GISTInsertState *state, GISTSTATE *giststate)
 	 * ready to recheck path in a bad case... We remember, that page->lsn
 	 * should never be invalid.
 	 */
-	while (true)
+	for (;;)
 	{
-
 		if (XLogRecPtrIsInvalid(state->stack->lsn))
 			state->stack->buffer = ReadBuffer(state->r, state->stack->blkno);
 		LockBuffer(state->stack->buffer, GIST_SHARE);
+		gistcheckpage(state->r, state->stack->buffer);
 
 		state->stack->page = (Page) BufferGetPage(state->stack->buffer);
 		opaque = GistPageGetOpaque(state->stack->page);
@@ -555,7 +555,6 @@ gistfindleaf(GISTInsertState *state, GISTSTATE *giststate)
 			continue;
 		}
 
-
 		if (!GistPageIsLeaf(state->stack->page))
 		{
 			/*
@@ -649,14 +648,18 @@ gistReadAndLockBuffer(Relation r, BlockNumber blkno)
 }
 
 /*
- * Traverse the tree to find path from root page,
- * to prevent deadlocks, it should lock only one page simultaneously.
- * Function uses in recovery and usial mode, so should work with different
- * read functions (gistReadAndLockBuffer and XLogReadBuffer)
+ * Traverse the tree to find path from root page.
+ *
  * returns from the begining of closest parent;
+ *
+ * Function is used in both regular and recovery mode, so must work with
+ * different read functions (gistReadAndLockBuffer and XLogReadBuffer)
+ *
+ * To prevent deadlocks, this should lock only one page simultaneously.
  */
 GISTInsertStack *
-gistFindPath(Relation r, BlockNumber child, Buffer (*myReadBuffer) (Relation, BlockNumber))
+gistFindPath(Relation r, BlockNumber child,
+			 Buffer (*myReadBuffer) (Relation, BlockNumber))
 {
 	Page		page;
 	Buffer		buffer;
@@ -674,7 +677,8 @@ gistFindPath(Relation r, BlockNumber child, Buffer (*myReadBuffer) (Relation, Bl
 
 	while (top && top->blkno != child)
 	{
-		buffer = myReadBuffer(r, top->blkno);	/* buffer locked */
+		buffer = myReadBuffer(r, top->blkno);	/* locks buffer */
+		gistcheckpage(r, buffer);
 		page = (Page) BufferGetPage(buffer);
 
 		if (GistPageIsLeaf(page))
@@ -771,9 +775,9 @@ gistFindCorrectParent(Relation r, GISTInsertStack *child)
 	GISTInsertStack *parent = child->parent;
 
 	LockBuffer(parent->buffer, GIST_EXCLUSIVE);
+	gistcheckpage(r, parent->buffer);
 	parent->page = (Page) BufferGetPage(parent->buffer);
 
-
 	/* here we don't need to distinguish between split and page update */
 	if (parent->childoffnum == InvalidOffsetNumber || !XLByteEQ(parent->lsn, PageGetLSN(parent->page)))
 	{
@@ -811,6 +815,7 @@ gistFindCorrectParent(Relation r, GISTInsertStack *child)
 				break;
 			parent->buffer = ReadBuffer(r, parent->blkno);
 			LockBuffer(parent->buffer, GIST_EXCLUSIVE);
+			gistcheckpage(r, parent->buffer);
 			parent->page = (Page) BufferGetPage(parent->buffer);
 		}
 
@@ -831,7 +836,8 @@ gistFindCorrectParent(Relation r, GISTInsertStack *child)
 		ptr = parent = gistFindPath(r, child->blkno, gistReadAndLockBuffer);
 		Assert(ptr != NULL);
 
-		/* read all buffers as supposed in caller */
+		/* read all buffers as expected by caller */
+		/* note we don't lock them or gistcheckpage them here! */
 		while (ptr)
 		{
 			ptr->buffer = ReadBuffer(r, ptr->blkno);

src/backend/access/gist/gistget.c

@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/gist/gistget.c,v 1.52 2005/10/06 02:29:07 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/gist/gistget.c,v 1.53 2005/11/06 22:39:20 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -40,6 +40,7 @@ killtuple(Relation r, GISTScanOpaque so, ItemPointer iptr)
 					maxoff;
 
 		LockBuffer(buffer, GIST_SHARE);
+		gistcheckpage(r, buffer);
 		p = (Page) BufferGetPage(buffer);
 
 		if (buffer == so->curbuf && XLByteEQ(so->stack->lsn, PageGetLSN(p)))
@@ -176,6 +177,7 @@ gistnext(IndexScanDesc scan, ScanDirection dir, ItemPointer tids, int maxtids, b
 		/* First of all, we need lock buffer */
 		Assert(so->curbuf != InvalidBuffer);
 		LockBuffer(so->curbuf, GIST_SHARE);
+		gistcheckpage(scan->indexRelation, so->curbuf);
 		p = BufferGetPage(so->curbuf);
 		opaque = GistPageGetOpaque(p);
 		resetoffset = false;
@@ -224,7 +226,8 @@ gistnext(IndexScanDesc scan, ScanDirection dir, ItemPointer tids, int maxtids, b
 			continue;
 		}
 
-		if (!GistPageIsLeaf(p) || resetoffset || ItemPointerIsValid(&scan->currentItemData) == false)
+		if (!GistPageIsLeaf(p) || resetoffset ||
+			!ItemPointerIsValid(&scan->currentItemData))
 		{
 			if (ScanDirectionIsBackward(dir))
 				n = PageGetMaxOffsetNumber(p);
@@ -268,7 +271,8 @@ gistnext(IndexScanDesc scan, ScanDirection dir, ItemPointer tids, int maxtids, b
 					return ntids;
 				}
 
-				so->curbuf = ReleaseAndReadBuffer(so->curbuf, scan->indexRelation,
+				so->curbuf = ReleaseAndReadBuffer(so->curbuf,
+												  scan->indexRelation,
 												  stk->block);
 				/* XXX	go up */
 				break;

src/backend/access/gist/gistutil.c

@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *			$PostgreSQL: pgsql/src/backend/access/gist/gistutil.c,v 1.7 2005/09/22 20:44:36 momjian Exp $
+ *			$PostgreSQL: pgsql/src/backend/access/gist/gistutil.c,v 1.8 2005/11/06 22:39:20 tgl Exp $
  *-------------------------------------------------------------------------
  */
 #include "postgres.h"
@@ -798,23 +798,6 @@ gistpenalty(GISTSTATE *giststate, int attno,
 					  PointerGetDatum(penalty));
 }
 
-void
-GISTInitBuffer(Buffer b, uint32 f)
-{
-	GISTPageOpaque opaque;
-	Page		page;
-	Size		pageSize;
-
-	pageSize = BufferGetPageSize(b);
-	page = BufferGetPage(b);
-	PageInit(page, pageSize, sizeof(GISTPageOpaqueData));
-
-	opaque = GistPageGetOpaque(page);
-	opaque->flags = f;
-	opaque->rightlink = InvalidBlockNumber;
-	memset(&(opaque->nsn), 0, sizeof(GistNSN));
-}
-
 void
 gistUserPicksplit(Relation r, GistEntryVector *entryvec, GIST_SPLITVEC *v,
 				  IndexTuple *itup, int len, GISTSTATE *giststate)
@@ -864,36 +847,108 @@ gistUserPicksplit(Relation r, GistEntryVector *entryvec, GIST_SPLITVEC *v,
 	}
 }
 
+/*
+ * Initialize a new index page
+ */
+void
+GISTInitBuffer(Buffer b, uint32 f)
+{
+	GISTPageOpaque opaque;
+	Page		page;
+	Size		pageSize;
+
+	pageSize = BufferGetPageSize(b);
+	page = BufferGetPage(b);
+	PageInit(page, pageSize, sizeof(GISTPageOpaqueData));
+
+	opaque = GistPageGetOpaque(page);
+	opaque->flags = f;
+	opaque->rightlink = InvalidBlockNumber;
+	/* page was already zeroed by PageInit, so this is not needed: */
+	/* memset(&(opaque->nsn), 0, sizeof(GistNSN)); */
+}
+
+/*
+ * Verify that a freshly-read page looks sane.
+ */
+void
+gistcheckpage(Relation rel, Buffer buf)
+{
+	Page		page = BufferGetPage(buf);
+
+	/*
+	 * ReadBuffer verifies that every newly-read page passes PageHeaderIsValid,
+	 * which means it either contains a reasonably sane page header or is
+	 * all-zero.  We have to defend against the all-zero case, however.
+	 */
+	if (PageIsNew(page))
+		ereport(ERROR,
+				(errcode(ERRCODE_INDEX_CORRUPTED),
+				 errmsg("index \"%s\" contains unexpected zero page at block %u",
+						RelationGetRelationName(rel),
+						BufferGetBlockNumber(buf)),
+				 errhint("Please REINDEX it.")));
+
+	/*
+	 * Additionally check that the special area looks sane.
+	 */
+	if (((PageHeader) (page))->pd_special !=
+		(BLCKSZ - MAXALIGN(sizeof(GISTPageOpaqueData))))
+		ereport(ERROR,
+				(errcode(ERRCODE_INDEX_CORRUPTED),
+				 errmsg("index \"%s\" contains corrupted page at block %u",
+						RelationGetRelationName(rel),
+						BufferGetBlockNumber(buf)),
+				 errhint("Please REINDEX it.")));
+}
+
+
+/*
+ * Allocate a new page (either by recycling, or by extending the index file)
+ *
+ * The returned buffer is already pinned and exclusive-locked
+ *
+ * Caller is responsible for initializing the page by calling GISTInitBuffer
+ */
 Buffer
 gistNewBuffer(Relation r)
 {
-	Buffer		buffer = InvalidBuffer;
+	Buffer		buffer;
 	bool		needLock;
 
-	while (true)
+	/* First, try to get a page from FSM */
+	for (;;)
 	{
 		BlockNumber blkno = GetFreeIndexPage(&r->rd_node);
 
 		if (blkno == InvalidBlockNumber)
-			break;
+			break;				/* nothing left in FSM */
 
 		buffer = ReadBuffer(r, blkno);
+		/*
+		 * We have to guard against the possibility that someone else already
+		 * recycled this page; the buffer may be locked if so.
+		 */
 		if (ConditionalLockBuffer(buffer))
 		{
 			Page		page = BufferGetPage(buffer);
 
+			if (PageIsNew(page))
+				return buffer;	/* OK to use, if never initialized */
+
+			gistcheckpage(r, buffer);
+
 			if (GistPageIsDeleted(page))
-			{
-				GistPageSetNonDeleted(page);
-				return buffer;
-			}
-			else
-				LockBuffer(buffer, GIST_UNLOCK);
+				return buffer;	/* OK to use */
+
+			LockBuffer(buffer, GIST_UNLOCK);
 		}
 
+		/* Can't use it, so release buffer and try again */
 		ReleaseBuffer(buffer);
 	}
 
+	/* Must extend the file */
 	needLock = !RELATION_IS_LOCAL(r);
 
 	if (needLock)

src/backend/access/gist/gistvacuum.c

@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/gist/gistvacuum.c,v 1.9 2005/09/22 20:44:36 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/gist/gistvacuum.c,v 1.10 2005/11/06 22:39:20 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -65,6 +65,11 @@ gistVacuumUpdate(GistVacuum *gv, BlockNumber blkno, bool needunion)
 				lencompleted = 16;
 
 	buffer = ReadBuffer(gv->index, blkno);
+	/*
+	 * This is only used during VACUUM FULL, so we need not bother to lock
+	 * individual index pages
+	 */
+	gistcheckpage(gv->index, buffer);
 	page = (Page) BufferGetPage(buffer);
 	maxoff = PageGetMaxOffsetNumber(page);
 
@@ -378,9 +383,10 @@ gistvacuumcleanup(PG_FUNCTION_ARGS)
 
 	needFullVacuum = false;
 
-	needLock = !RELATION_IS_LOCAL(rel);
 	if (info->vacuum_full)
 		needLock = false;		/* relation locked with AccessExclusiveLock */
+	else
+		needLock = !RELATION_IS_LOCAL(rel);
 
 	/* try to find deleted pages */
 	if (needLock)
@@ -403,7 +409,7 @@ gistvacuumcleanup(PG_FUNCTION_ARGS)
 		LockBuffer(buffer, GIST_SHARE);
 		page = (Page) BufferGetPage(buffer);
 
-		if (GistPageIsDeleted(page))
+		if (PageIsNew(page) || GistPageIsDeleted(page))
 		{
 			if (nFreePages < maxFreePages)
 			{
@@ -513,6 +519,7 @@ gistbulkdelete(PG_FUNCTION_ARGS)
 		ItemId		iid;
 
 		LockBuffer(buffer, GIST_SHARE);
+		gistcheckpage(rel, buffer);
 		page = (Page) BufferGetPage(buffer);
 
 		if (GistPageIsLeaf(page))

src/include/access/gist_private.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/access/gist_private.h,v 1.8 2005/10/15 02:49:42 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/access/gist_private.h,v 1.9 2005/11/06 22:39:21 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -263,6 +263,7 @@ extern Datum gistgettuple(PG_FUNCTION_ARGS);
 extern Datum gistgetmulti(PG_FUNCTION_ARGS);
 
 /* gistutil.c */
+extern void gistcheckpage(Relation rel, Buffer buf);
 extern Buffer gistNewBuffer(Relation r);
 extern OffsetNumber gistfillbuffer(Relation r, Page page, IndexTuple *itup,
 			   int len, OffsetNumber off);