From 04d9f4dab4723a0736157542b1d1a24ad2b8aa1d Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 28 Jun 2010 02:07:02 +0000 Subject: [PATCH] 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. --- src/bin/pg_dump/pg_backup_archiver.c | 55 ++++++++++++++++++++++++------------ src/bin/pg_dump/pg_backup_custom.c | 12 ++++---- 2 files changed, 44 insertions(+), 23 deletions(-) diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index d0e30ad5a2..20d86fb1a8 100644 --- 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 11d71a6679..3c1678ca9c 100644 --- 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. */ } } -- 2.11.0