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 d0e30ad5a217ab2d800b5499226ba47457516fb6..20d86fb1a8fb2df06548cb47406873905f4dcbfd 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.185 2010/05/15 21:41:16 tgl Exp $
+ *     $PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_archiver.c,v 1.186 2010/06/28 02:07:02 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1755,6 +1755,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);
 
@@ -2981,7 +2986,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)
@@ -3000,7 +3010,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);
@@ -3068,27 +3077,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 11d71a667987d0b6fc8c609029d70ff2d5581e8c..3c1678ca9cd16d58cee28137c65b91a1ba3836d7 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.45 2010/06/27 19:07:24 tgl Exp $
+ *     $PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_custom.c,v 1.46 2010/06/28 02:07:02 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.
             */
        }
    }