Clean up smgr.c/md.c APIs as per discussion a couple months ago. Instead of

having md.c return a success/failure boolean to smgr.c, which was just going
to elog anyway, let md.c issue the elog messages itself.  This allows better
error reporting, particularly in cases such as "short read" or "short write"
which Peter was complaining of.  Also, remove the kluge of allowing mdread()
to return zeroes from a read-beyond-EOF: this is now an error condition
except when InRecovery or zero_damaged_pages = true.  (Hash indexes used to
require that behavior, but no more.)  Also, enforce that mdwrite() is to be
used for rewriting existing blocks while mdextend() is to be used for
extending the relation EOF.  This restriction lets us get rid of the old
ad-hoc defense against creating huge files by an accidental reference to
a bogus block number: we'll only create new segments in mdextend() not
mdwrite() or mdread().  (Again, when InRecovery we allow it anyway, since
we need to allow updates of blocks that were later truncated away.)
Also, clean up the original makeshift patch for bug #2737: move the
responsibility for padding relation segments to full length into md.c.

2 files changed, 20 insertions, 31 deletions

diff --git a/src/backend/access/hash/hashpage.c b/src/backend/access/hash/hashpage.c
index 0f643836a1c..b9569e58af4 100644
--- a/src/backend/access/hash/hashpage.c
+++ b/src/backend/access/hash/hashpage.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/hash/hashpage.c,v 1.61 2006/11/19 21:33:23 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/hash/hashpage.c,v 1.62 2007/01/03 18:11:01 tgl Exp $
  *
  * NOTES
  *	  Postgres hash pages look like ordinary relation pages.  The opaque
@@ -533,10 +533,8 @@ fail:
  *
  * This does not need to initialize the new bucket pages; we'll do that as
  * each one is used by _hash_expandtable().  But we have to extend the logical
- * EOF to the end of the splitpoint; otherwise the first overflow page
- * allocated beyond the splitpoint will represent a noncontiguous access,
- * which can confuse md.c (and will probably be forbidden by future changes
- * to md.c).
+ * EOF to the end of the splitpoint; this keeps smgr's idea of the EOF in
+ * sync with ours, so that overflow-page allocation works correctly.
  *
  * We do this by writing a page of zeroes at the end of the splitpoint range.
  * We expect that the filesystem will ensure that the intervening pages read
@@ -559,7 +557,6 @@ _hash_alloc_buckets(Relation rel, uint32 nblocks)
 {
 	BlockNumber	firstblock;
 	BlockNumber	lastblock;
-	BlockNumber	endblock;
 	char		zerobuf[BLCKSZ];
 
 	/*
@@ -577,24 +574,9 @@ _hash_alloc_buckets(Relation rel, uint32 nblocks)
 	if (lastblock < firstblock || lastblock == InvalidBlockNumber)
 		return InvalidBlockNumber;
 
-	/* Note: we assume RelationGetNumberOfBlocks did RelationOpenSmgr for us */
-
 	MemSet(zerobuf, 0, sizeof(zerobuf));
 
-	/*
-	 * XXX If the extension results in creation of new segment files,
-	 * we have to make sure that each non-last file is correctly filled out to
-	 * RELSEG_SIZE blocks.  This ought to be done inside mdextend, but
-	 * changing the smgr API seems best left for development cycle not late
-	 * beta.  Temporary fix for bug #2737.
-	 */
-#ifndef LET_OS_MANAGE_FILESIZE
-	for (endblock = firstblock | (RELSEG_SIZE - 1);
-		 endblock < lastblock;
-		 endblock += RELSEG_SIZE)
-		smgrextend(rel->rd_smgr, endblock, zerobuf, rel->rd_istemp);
-#endif
-
+	/* Note: we assume RelationGetNumberOfBlocks did RelationOpenSmgr for us */
 	smgrextend(rel->rd_smgr, lastblock, zerobuf, rel->rd_istemp);
 
 	return firstblock;
diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
index 4951dca2182..4f886e8b07e 100644
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -36,9 +36,9 @@
  * that is of no value (since other backends have no interest in them yet)
  * and it created locking problems for CHECKPOINT, because the upper-level
  * pages were held exclusive-locked for long periods.  Now we just build
- * the pages in local memory and smgrwrite() them as we finish them.  They
- * will need to be re-read into shared buffers on first use after the build
- * finishes.
+ * the pages in local memory and smgrwrite or smgrextend them as we finish
+ * them.  They will need to be re-read into shared buffers on first use after
+ * the build finishes.
  *
  * Since the index will never be used unless it is completely built,
  * from a crash-recovery point of view there is no need to WAL-log the
@@ -57,7 +57,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/nbtree/nbtsort.c,v 1.107 2006/10/04 00:29:49 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/nbtree/nbtsort.c,v 1.108 2007/01/03 18:11:01 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -309,9 +309,9 @@ _bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno)
 	{
 		if (!wstate->btws_zeropage)
 			wstate->btws_zeropage = (Page) palloc0(BLCKSZ);
-		smgrwrite(wstate->index->rd_smgr, wstate->btws_pages_written++,
-				  (char *) wstate->btws_zeropage,
-				  true);
+		smgrextend(wstate->index->rd_smgr, wstate->btws_pages_written++,
+				   (char *) wstate->btws_zeropage,
+				   true);
 	}
 
 	/*
@@ -319,10 +319,17 @@ _bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno)
 	 * index, because there's no need for smgr to schedule an fsync for this
 	 * write; we'll do it ourselves before ending the build.
 	 */
-	smgrwrite(wstate->index->rd_smgr, blkno, (char *) page, true);
-
 	if (blkno == wstate->btws_pages_written)
+	{
+		/* extending the file... */
+		smgrextend(wstate->index->rd_smgr, blkno, (char *) page, true);
 		wstate->btws_pages_written++;
+	}
+	else
+	{
+		/* overwriting a block we zero-filled before */
+		smgrwrite(wstate->index->rd_smgr, blkno, (char *) page, true);
+	}
 
 	pfree(page);
 }