Improve pg_dump's checkSeek() function to verify the functioning of ftello

as well as fseeko, and to not assume that fseeko(fp, 0, SEEK_CUR) proves
anything.  Also improve some related comments.  Per my observation that
the SEEK_CUR test didn't actually work on some platforms, and subsequent
discussion with Robert Haas.

Back-patch to 8.4.  In earlier releases it's not that important whether
we get the hasSeek test right, but with parallel restore it matters.

diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index cc10e8cf4ad8aa32b3ea9c1bbeff665a156c4f56..c0d326d821c7cd80b674228f3dfb36a734ad15a9 100644 (file)
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -15,7 +15,7 @@
  *
  *
  * IDENTIFICATION
- *             $PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_archiver.c,v 1.172.2.1 2010/01/19 18:39:26 tgl Exp $
+ *             $PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_archiver.c,v 1.172.2.2 2010/06/28 02:07:09 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1723,6 +1723,11 @@ _discoverArchiveFormat(ArchiveHandle *AH)
 
        if (strncmp(sig, "PGDMP", 5) == 0)
        {
+               /*
+                * Finish reading (most of) a custom-format header.
+                *
+                * NB: this code must agree with ReadHead().
+                */
                AH->vmaj = fgetc(fh);
                AH->vmin = fgetc(fh);
 
@@ -2910,7 +2915,12 @@ ReadHead(ArchiveHandle *AH)
        int                     fmt;
        struct tm       crtm;
 
-       /* If we haven't already read the header... */
+       /*
+        * If we haven't already read the header, do so.
+        *
+        * NB: this code must agree with _discoverArchiveFormat().  Maybe find
+        * a way to unify the cases?
+        */
        if (!AH->readHeader)
        {
                if ((*AH->ReadBufPtr) (AH, tmpMag, 5) != 5)
@@ -2929,7 +2939,6 @@ ReadHead(ArchiveHandle *AH)
 
                AH->version = ((AH->vmaj * 256 + AH->vmin) * 256 + AH->vrev) * 256 + 0;
 
-
                if (AH->version < K_VERS_1_0 || AH->version > K_VERS_MAX)
                        die_horribly(AH, modulename, "unsupported version (%d.%d) in file header\n",
                                                 AH->vmaj, AH->vmin);
@@ -2998,27 +3007,37 @@ ReadHead(ArchiveHandle *AH)
 
 /*
  * checkSeek
- *       check to see if fseek can be performed.
+ *       check to see if ftell/fseek can be performed.
  */
 bool
 checkSeek(FILE *fp)
 {
-       if (fseeko(fp, 0, SEEK_CUR) != 0)
-               return false;
-       else if (sizeof(pgoff_t) > sizeof(long))
-       {
-               /*
-                * At this point, pgoff_t is too large for long, so we return based on
-                * whether an pgoff_t version of fseek is available.
-                */
-#ifdef HAVE_FSEEKO
-               return true;
-#else
+       pgoff_t         tpos;
+
+       /*
+        * If pgoff_t is wider than long, we must have "real" fseeko and not
+        * an emulation using fseek.  Otherwise report no seek capability.
+        */
+#ifndef HAVE_FSEEKO
+       if (sizeof(pgoff_t) > sizeof(long))
                return false;
 #endif
-       }
-       else
-               return true;
+
+       /* Check that ftello works on this file */
+       errno = 0;
+       tpos = ftello(fp);
+       if (errno)
+               return false;
+
+       /*
+        * Check that fseeko(SEEK_SET) works, too.  NB: we used to try to test
+        * this with fseeko(fp, 0, SEEK_CUR).  But some platforms treat that as a
+        * successful no-op even on files that are otherwise unseekable.
+        */
+       if (fseeko(fp, tpos, SEEK_SET) != 0)
+               return false;
+
+       return true;
 }
 
 

diff --git a/src/bin/pg_dump/pg_backup_custom.c b/src/bin/pg_dump/pg_backup_custom.c
index f4f67e077ad47f030937baf569631e749d5248e0..e233e68e025844ce985b08fbbefed309f46a39a8 100644 (file)
--- a/src/bin/pg_dump/pg_backup_custom.c
+++ b/src/bin/pg_dump/pg_backup_custom.c
@@ -19,7 +19,7 @@
  *
  *
  * IDENTIFICATION
- *             $PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_custom.c,v 1.42.2.1 2010/06/27 19:07:30 tgl Exp $
+ *             $PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_custom.c,v 1.42.2.2 2010/06/28 02:07:09 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -835,9 +835,10 @@ _CloseArchive(ArchiveHandle *AH)
                WriteDataChunks(AH);
 
                /*
-                * This is not an essential operation - it is really only needed if we
-                * expect to be doing seeks to read the data back - it may be ok to
-                * just use the existing self-consistent block formatting.
+                * If possible, re-write the TOC in order to update the data offset
+                * information.  This is not essential, as pg_restore can cope in
+                * most cases without it; but it can make pg_restore significantly
+                * faster in some situations (especially parallel restore).
                 */
                if (ctx->hasSeek &&
                        fseeko(AH->FH, tpos, SEEK_SET) == 0)
@@ -914,7 +915,8 @@ _getFilePos(ArchiveHandle *AH, lclContext *ctx)
 
                        /*
                         * Prior to 1.7 (pg7.3) we relied on the internally maintained
-                        * pointer. Now we rely on pgoff_t always. pos = ctx->filePos;
+                        * pointer. Now we rely on ftello() always, unless the file has
+                        * been found to not support it.
                         */
                }
        }