They are two different problems; the TOC entry is important for any

multiline command  or to rerun the command easily later.

Whereas displaying the failed SQL command is a matter of fixing the
error
messages.

The latter is complicated by failed COPY commands which, with
die-on-errors
off, results in the data being processed as a command, so dumping the
command will dump all of the data.

In the case of long commands, should the whole command be dumped? eg.
(eg.
several pages of function definition).

In the case of the COPY command, I'm not sure what to do. Obviously, it
would be best to avoid sending the data, but the data and command are
combined (from memory). Also, the 'data' may be in the form of INSERT
statements.

Attached patch produces the first 125 chars of the command:

pg_restore: [archiver (db)] Error while PROCESSING TOC:
pg_restore: [archiver (db)] Error from TOC Entry 26; 1255 16449270
FUNCTION
plpgsql_call_handler() pjw
pg_restore: [archiver (db)] could not execute query: ERROR:  function
"plpgsql_call_handler" already exists with same argument types
     Command was: CREATE FUNCTION plpgsql_call_handler() RETURNS
language_handler
     AS '/var/lib/pgsql-8.0b1/lib/plpgsql', 'plpgsql_call_han...
pg_restore: [archiver (db)] Error from TOC Entry 27; 1255 16449271
FUNCTION
plpgsql_validator(oid) pjw
pg_restore: [archiver (db)] could not execute query: ERROR:  function
"plpgsql_validator" already exists with same argument types
     Command was: CREATE FUNCTION plpgsql_validator(oid) RETURNS void
     AS '/var/lib/pgsql-8.0b1/lib/plpgsql', 'plpgsql_validator'
     LANGU...

Philip Warner

diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 5c610f962bce5eaa1b405213d433fca0db4a7366..698e7fd524e69205ab1fd126934afcbb42406c8d 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.93 2004/08/20 04:20:22 momjian Exp $
+ *     $PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_archiver.c,v 1.94 2004/08/20 20:00:34 momjian Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -137,6 +137,7 @@ RestoreArchive(Archive *AHX, RestoreOptions *ropt)
    bool        defnDumped;
 
    AH->ropt = ropt;
+   AH->stage = STAGE_INITIALIZING;
 
    /*
     * Check for nonsensical option combinations.
@@ -166,6 +167,8 @@ RestoreArchive(Archive *AHX, RestoreOptions *ropt)
        ConnectDatabase(AHX, ropt->dbname,
                        ropt->pghost, ropt->pgport, ropt->username,
                        ropt->requirePassword, ropt->ignoreVersion);
+       /* If we're talking to the DB directly, don't send comments since they obscure SQL when displaying errors */
+       AH->noTocComments = 1;
    }
 
    /*
@@ -211,12 +214,16 @@ RestoreArchive(Archive *AHX, RestoreOptions *ropt)
     */
    _doSetFixedOutputState(AH);
 
+   AH->stage = STAGE_PROCESSING;
+
    /*
     * Drop the items at the start, in reverse order
     */
    if (ropt->dropSchema)
    {
        te = AH->toc->prev;
+       AH->currentTE = te;
+
        while (te != AH->toc)
        {
            reqs = _tocEntryRequired(te, ropt, false);
@@ -240,6 +247,8 @@ RestoreArchive(Archive *AHX, RestoreOptions *ropt)
    te = AH->toc->next;
    while (te != AH->toc)
    {
+       AH->currentTE = te;
+
        /* Work out what, if anything, we want from this entry */
        reqs = _tocEntryRequired(te, ropt, false);
 
@@ -375,6 +384,8 @@ RestoreArchive(Archive *AHX, RestoreOptions *ropt)
    te = AH->toc->next;
    while (te != AH->toc)
    {
+       AH->currentTE = te;
+
        /* Work out what, if anything, we want from this entry */
        reqs = _tocEntryRequired(te, ropt, true);
 
@@ -391,6 +402,8 @@ RestoreArchive(Archive *AHX, RestoreOptions *ropt)
    /*
     * Clean up & we're done.
     */
+   AH->stage = STAGE_FINALIZING;
+
    if (ropt->filename || ropt->compression)
        ResetOutput(AH, sav);
 
@@ -1227,6 +1240,39 @@ warn_or_die_horribly(ArchiveHandle *AH,
                     const char *modulename, const char *fmt, ...)
 {
    va_list ap;
+
+   switch(AH->stage) {
+
+       case STAGE_NONE:
+           /* Do nothing special */
+           break;
+
+       case STAGE_INITIALIZING:
+           if (AH->stage != AH->lastErrorStage) {
+               write_msg(modulename, "Error while INITIALIZING:\n");
+           }
+           break;
+
+       case STAGE_PROCESSING:
+           if (AH->stage != AH->lastErrorStage) {
+               write_msg(modulename, "Error while PROCESSING TOC:\n");
+           }
+           break;
+
+       case STAGE_FINALIZING:
+           if (AH->stage != AH->lastErrorStage) {
+               write_msg(modulename, "Error while FINALIZING:\n");
+           }
+           break;
+   }
+   if (AH->currentTE != NULL && AH->currentTE != AH->lastErrorTE) {
+       write_msg(modulename, "Error from TOC Entry %d; %u %u %s %s %s\n", AH->currentTE->dumpId,
+                AH->currentTE->catalogId.tableoid, AH->currentTE->catalogId.oid,
+                AH->currentTE->desc, AH->currentTE->tag, AH->currentTE->owner);
+   }
+   AH->lastErrorStage = AH->stage;
+   AH->lastErrorTE = AH->currentTE;
+
    va_start(ap, fmt);
    if (AH->public.exit_on_error)
    {
@@ -2026,6 +2072,9 @@ _doSetFixedOutputState(ArchiveHandle *AH)
    /* Make sure function checking is disabled */
    ahprintf(AH, "SET check_function_bodies = false;\n");
 
+   /* Avoid annoying notices etc */
+   ahprintf(AH, "SET client_min_messages = warning;\n");
+
    ahprintf(AH, "\n");
 }
 
@@ -2317,6 +2366,9 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isDat
            return;
    }
 
+   if (AH->noTocComments) 
+       return;
+
    /*
     * Avoid dumping the public schema, as it will already be created ...
     * unless we are using --clean mode, in which case it's been deleted

diff --git a/src/bin/pg_dump/pg_backup_archiver.h b/src/bin/pg_dump/pg_backup_archiver.h
index 7f6740376e78ec9fa6e6750774890fa98dc6cccb..9664ab23651b4e54634629e2d52778adab415eb1 100644 (file)
--- a/src/bin/pg_dump/pg_backup_archiver.h
+++ b/src/bin/pg_dump/pg_backup_archiver.h
@@ -17,7 +17,7 @@
  *
  *
  * IDENTIFICATION
- *     $PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_archiver.h,v 1.59 2004/08/20 16:07:15 momjian Exp $
+ *     $PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_archiver.h,v 1.60 2004/08/20 20:00:34 momjian Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -152,6 +152,14 @@ typedef struct
    PQExpBuffer tagBuf;
 } sqlparseInfo;
 
+typedef enum 
+{
+    STAGE_NONE = 0,
+    STAGE_INITIALIZING,
+    STAGE_PROCESSING,
+    STAGE_FINALIZING
+} ArchiverStage;
+
 typedef struct _archiveHandle
 {
    Archive     public;         /* Public part of archive */
@@ -254,6 +262,12 @@ typedef struct _archiveHandle
    void       *lo_buf;
    size_t      lo_buf_used;
    size_t      lo_buf_size;
+
+   int     noTocComments;
+   ArchiverStage       stage;
+   ArchiverStage       lastErrorStage;
+   struct _tocEntry    *currentTE;
+   struct _tocEntry    *lastErrorTE;
 } ArchiveHandle;
 
 typedef struct _tocEntry

diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c
index 984c2f844c3bb0fa10a8000fa65dcbded54939d5..6013a90974567ce5cf8fa52cff25b3d29b06eba0 100644 (file)
--- a/src/bin/pg_dump/pg_backup_db.c
+++ b/src/bin/pg_dump/pg_backup_db.c
@@ -5,7 +5,7 @@
  * Implements the basic DB functions used by the archiver.
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_db.c,v 1.54 2004/08/20 16:07:15 momjian Exp $
+ *   $PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_db.c,v 1.55 2004/08/20 20:00:34 momjian Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -40,6 +40,8 @@ static char *_sendCopyLine(ArchiveHandle *AH, char *qry, char *eos);
 static int _isIdentChar(char c);
 static int _isDQChar(char c, int atStart);
 
+#define DB_MAX_ERR_STMT 128
+
 static int
 _parse_version(ArchiveHandle *AH, const char *versionString)
 {
@@ -302,6 +304,7 @@ static int
 _executeSqlCommand(ArchiveHandle *AH, PGconn *conn, PQExpBuffer qry, char *desc)
 {
    PGresult   *res;
+   char        errStmt[DB_MAX_ERR_STMT];
 
    /* fprintf(stderr, "Executing: '%s'\n\n", qry->data); */
    res = PQexec(conn, qry->data);
@@ -318,8 +321,18 @@ _executeSqlCommand(ArchiveHandle *AH, PGconn *conn, PQExpBuffer qry, char *desc)
            AH->pgCopyIn = 1;
        }
        else
-           warn_or_die_horribly(AH, modulename, "%s: %s",
-                                desc, PQerrorMessage(AH->connection));
+       {
+           strncpy(errStmt, qry->data, DB_MAX_ERR_STMT);
+           if (errStmt[DB_MAX_ERR_STMT-1] != '\0') {
+               errStmt[DB_MAX_ERR_STMT-4] = '.';
+               errStmt[DB_MAX_ERR_STMT-3] = '.';
+               errStmt[DB_MAX_ERR_STMT-2] = '.';
+               errStmt[DB_MAX_ERR_STMT-1] = '\0';
+           }
+           warn_or_die_horribly(AH, modulename, "%s: %s    Command was: %s\n",
+                       desc, PQerrorMessage(AH->connection),
+                       errStmt);
+       }
    }
 
    PQclear(res);