improve pg_restore warning on text dump input

Started by Andrew Dunstanabout 14 years ago3 messages
#1Andrew Dunstan
andrew@dunslane.net
1 attachment(s)

From time to time there are complaints because people mistakenly feed a
text format dump to pg_restore and get back a somewhat cryptic message
about the file not being a valid archive. It's been suggested that we
should have pg_restore run the file through psql, but that would involve
more work than I at least care to give the problem. However, I think we
should give a nicer message, suggesting the user try feeding the file to
psql instead. The attached small patch does that.

cheers

andrew

Attachments:

resstorewarn.patchtext/x-patch; name=resstorewarn.patchDownload
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 7d895c4..9918c4d 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -77,6 +77,9 @@ typedef struct _parallel_slot
 
 #define NO_SLOT (-1)
 
+#define TEXT_DUMP_HEADER "--\n-- PostgreSQL database dump\n--\n\n"
+#define TEXT_DUMPALL_HEADER "--\n-- PostgreSQL database cluster dump\n--\n\n"
+
 /* state needed to save/restore an archive's output target */
 typedef struct _outputContext
 {
@@ -1872,7 +1875,19 @@ _discoverArchiveFormat(ArchiveHandle *AH)
 			die_horribly(AH, modulename, "input file does not appear to be a valid archive (too short?)\n");
 
 		if (!isValidTarHeader(AH->lookahead))
-			die_horribly(AH, modulename, "input file does not appear to be a valid archive\n");
+		{
+			if (strncmp(AH->lookahead, TEXT_DUMP_HEADER, strlen(TEXT_DUMP_HEADER)) == 0 ||
+				strncmp(AH->lookahead, TEXT_DUMPALL_HEADER, strlen(TEXT_DUMPALL_HEADER)) == 0)
+			{
+				/* looks like it's probably a text format dump. so suggest they try psql */
+				die_horribly(AH, modulename, "input file appears to be a text format dump. Please use psql.\n");
+			}
+			else
+			{
+				/* we have no idea what this is */
+				die_horribly(AH, modulename, "input file does not appear to be a valid archive\n");
+			}
+		}
 
 		AH->format = archTar;
 	}
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#1)
Re: improve pg_restore warning on text dump input

Andrew Dunstan <andrew@dunslane.net> writes:

From time to time there are complaints because people mistakenly feed a
text format dump to pg_restore and get back a somewhat cryptic message
about the file not being a valid archive. It's been suggested that we
should have pg_restore run the file through psql, but that would involve
more work than I at least care to give the problem. However, I think we
should give a nicer message, suggesting the user try feeding the file to
psql instead. The attached small patch does that.

It would probably be better if you put this test before the one that
insists the file is at least 512 bytes.

regards, tom lane

#3Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#2)
Re: improve pg_restore warning on text dump input

On 01/03/2012 01:55 PM, Tom Lane wrote:

Andrew Dunstan<andrew@dunslane.net> writes:

From time to time there are complaints because people mistakenly feed a
text format dump to pg_restore and get back a somewhat cryptic message
about the file not being a valid archive. It's been suggested that we
should have pg_restore run the file through psql, but that would involve
more work than I at least care to give the problem. However, I think we
should give a nicer message, suggesting the user try feeding the file to
psql instead. The attached small patch does that.

It would probably be better if you put this test before the one that
insists the file is at least 512 bytes.

Hmm, yeah. I guess we're pretty much certain that these markers can't
reasonably appear at the start of a tar archive.

cheers

andrew