Fix pg_dump to read-lock all tables to be dumped as soon as it's read

their names from pg_class.  This considerably reduces the window wherein
someone could DROP or ALTER a table that pg_dump is intending to dump.
Not a perfect solution, but definitely an improvement.  Per complaints
from Marc Fournier; patch by Brent Verner with some kibitzing by Tom Lane.

diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
index d9d358892cb335b2a6eb7ec7f9852f106415eb77..1a32e22b22b3747a7c1bc8a90e7dbb4fcc5da147 100644 (file)
--- a/src/bin/pg_dump/common.c
+++ b/src/bin/pg_dump/common.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/bin/pg_dump/common.c,v 1.60 2001/10/25 05:49:52 momjian Exp $
+ *   $Header: /cvsroot/pgsql/src/bin/pg_dump/common.c,v 1.61 2002/01/11 23:21:55 tgl Exp $
  *
  * Modifications - 6/12/96 - [email protected] - version 1.13.dhb.2
  *
@@ -309,7 +309,7 @@ dumpSchema(Archive *fout,
 
    if (g_verbose)
        write_msg(NULL, "reading user-defined tables\n");
-   tblinfo = getTables(&numTables, finfo, numFuncs);
+   tblinfo = getTables(&numTables, finfo, numFuncs, tablename);
 
    if (g_verbose)
        write_msg(NULL, "reading index information\n");

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 5c2c904555930a603341ef94a67b6966991b08d1..8fb0812dd6b28b996ab423424db56ded63c0548f 100644 (file)
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -22,7 +22,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/bin/pg_dump/pg_dump.c,v 1.236 2001/10/28 06:25:58 momjian Exp $
+ *   $Header: /cvsroot/pgsql/src/bin/pg_dump/pg_dump.c,v 1.237 2002/01/11 23:21:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2035,13 +2035,14 @@ getFuncs(int *numFuncs)
  * numTables is set to the number of tables read in
  */
 TableInfo *
-getTables(int *numTables, FuncInfo *finfo, int numFuncs)
+getTables(int *numTables, FuncInfo *finfo, int numFuncs, const char* tablename)
 {
    PGresult   *res;
    int         ntups;
    int         i;
    PQExpBuffer query = createPQExpBuffer();
    PQExpBuffer delqry = createPQExpBuffer();
+   PQExpBuffer lockquery = createPQExpBuffer();
    TableInfo  *tblinfo;
 
    int         i_reloid;
@@ -2054,11 +2055,6 @@ getTables(int *numTables, FuncInfo *finfo, int numFuncs)
    int         i_relhasindex;
    int         i_relhasoids;
 
-   char        relkindview[2];
-
-   relkindview[0] = RELKIND_VIEW;
-   relkindview[1] = '\0';
-
    /*
     * find all the user-defined tables (no indexes and no catalogs),
     * ordering by oid is important so that we always process the parent
@@ -2129,6 +2125,17 @@ getTables(int *numTables, FuncInfo *finfo, int numFuncs)
 
    *numTables = ntups;
 
+   /*
+    * First pass: extract data from result and lock tables.  We do the
+    * locking before anything else, to minimize the window wherein a table
+    * could disappear under us.
+    *
+    * Note that we have to collect info about all tables here, even when
+    * dumping only one, because we don't know which tables might be
+    * inheritance ancestors of the target table.  Possible future
+    * improvement: suppress later collection of schema info about tables
+    * that are determined not to be either targets or ancestors of targets.
+    */
    tblinfo = (TableInfo *) malloc(ntups * sizeof(TableInfo));
 
    i_reloid = PQfnumber(res, "oid");
@@ -2146,17 +2153,63 @@ getTables(int *numTables, FuncInfo *finfo, int numFuncs)
        tblinfo[i].oid = strdup(PQgetvalue(res, i, i_reloid));
        tblinfo[i].relname = strdup(PQgetvalue(res, i, i_relname));
        tblinfo[i].relacl = strdup(PQgetvalue(res, i, i_relacl));
-       tblinfo[i].sequence = (strcmp(PQgetvalue(res, i, i_relkind), "S") == 0);
+       tblinfo[i].relkind = *(PQgetvalue(res, i, i_relkind));
+       tblinfo[i].sequence = (tblinfo[i].relkind == RELKIND_SEQUENCE);
+       tblinfo[i].hasindex = (strcmp(PQgetvalue(res, i, i_relhasindex), "t") == 0);
+       tblinfo[i].hasoids = (strcmp(PQgetvalue(res, i, i_relhasoids), "t") == 0);
        tblinfo[i].usename = strdup(PQgetvalue(res, i, i_usename));
        tblinfo[i].ncheck = atoi(PQgetvalue(res, i, i_relchecks));
        tblinfo[i].ntrig = atoi(PQgetvalue(res, i, i_reltriggers));
 
+       /*
+        * Read-lock target tables to make sure they aren't DROPPED or
+        * altered in schema before we get around to dumping them.
+        *
+        * If no target tablename was specified, lock all tables we see,
+        * otherwise lock only the specified table.  (This is incomplete
+        * because we'll still try to collect schema info about all tables,
+        * and could possibly lose during that phase.  But for the typical
+        * use where we're dumping all tables anyway, it matters not.)
+        *
+        * NOTE: it'd be kinda nice to lock views and sequences too, not
+        * only plain tables, but the backend doesn't presently allow that.
+        */
+       if ((tblinfo[i].relkind == RELKIND_RELATION) &&
+           (tablename == NULL || strcmp(tblinfo[i].relname, tablename) == 0))
+       {
+           PGresult *lres;
+
+           resetPQExpBuffer(lockquery);
+           appendPQExpBuffer(lockquery,
+                             "LOCK TABLE %s IN ACCESS SHARE MODE",
+                             fmtId(tblinfo[i].relname, force_quotes));
+           lres = PQexec(g_conn,lockquery->data);
+           if (!lres || PQresultStatus(lres) != PGRES_COMMAND_OK)
+           {
+               write_msg(NULL, "Attempt to lock table \"%s\" failed.  %s", 
+                         tblinfo[i].relname, PQerrorMessage(g_conn));
+               exit_nicely();
+           }
+           PQclear(lres);
+       }
+   }
+
+   PQclear(res);
+   res = NULL;
+
+   /*
+    * Second pass: pick up additional information about each table,
+    * as required.
+    */
+   for (i = 0; i < *numTables; i++)
+   {
+       /* Emit notice if join for owner failed */
        if (strlen(tblinfo[i].usename) == 0)
            write_msg(NULL, "WARNING: owner of table \"%s\" appears to be invalid\n",
                      tblinfo[i].relname);
 
-       /* Get view definition */
-       if (strcmp(PQgetvalue(res, i, i_relkind), relkindview) == 0)
+       /* Get definition if it's a view */
+       if (tblinfo[i].relkind == RELKIND_VIEW)
        {
            PGresult   *res2;
 
@@ -2208,6 +2261,7 @@ getTables(int *numTables, FuncInfo *finfo, int numFuncs)
                          tblinfo[i].relname);
                exit_nicely();
            }
+           PQclear(res2);
        }
        else
            tblinfo[i].viewdef = NULL;
@@ -2291,7 +2345,7 @@ getTables(int *numTables, FuncInfo *finfo, int numFuncs)
            tblinfo[i].check_expr = NULL;
 
        /* Get primary key */
-       if (strcmp(PQgetvalue(res, i, i_relhasindex), "t") == 0)
+       if (tblinfo[i].hasindex)
        {
            PGresult   *res2;
 
@@ -2323,9 +2377,6 @@ getTables(int *numTables, FuncInfo *finfo, int numFuncs)
        else
            tblinfo[i].pkIndexOid = NULL;
 
-       /* Has it got OIDs? */
-       tblinfo[i].hasoids = (strcmp(PQgetvalue(res, i, i_relhasoids), "t") == 0);
-
        /* Get primary key name (if primary key exist) */
        if (tblinfo[i].pkIndexOid != NULL)
        {
@@ -2643,10 +2694,9 @@ getTables(int *numTables, FuncInfo *finfo, int numFuncs)
 
    }
 
-   PQclear(res);
-
    destroyPQExpBuffer(query);
    destroyPQExpBuffer(delqry);
+   destroyPQExpBuffer(lockquery);
 
    return tblinfo;
 }

diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 3a59d5edf16eb9f727fbdabba1f5463ca23d29d4..3af39b8d2e59979132a84995d19f4a313dfc7764 100644 (file)
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: pg_dump.h,v 1.76 2001/11/05 17:46:30 momjian Exp $
+ * $Id: pg_dump.h,v 1.77 2002/01/11 23:21:55 tgl Exp $
  *
  * Modifications - 6/12/96 - [email protected] - version 1.13.dhb.2
  *
@@ -96,7 +96,9 @@ typedef struct _tableInfo
                                 * constructed manually from rules, and
                                 * rule may ref things created after the
                                 * base table was created. */
-   bool        sequence;
+   char        relkind;
+   bool        sequence;       /* this is redundant with relkind... */
+   bool        hasindex;       /* does it have any indexes? */
    bool        hasoids;        /* does it have OIDs? */
    int         numatts;        /* number of attributes */
    int        *inhAttrs;       /* an array of flags, one for each
@@ -254,7 +256,8 @@ extern void clearOprInfo(OprInfo *, int);
 extern void clearTypeInfo(TypeInfo *, int);
 
 extern OprInfo *getOperators(int *numOperators);
-extern TableInfo *getTables(int *numTables, FuncInfo *finfo, int numFuncs);
+extern TableInfo *getTables(int *numTables, FuncInfo *finfo, int numFuncs,
+                           const char* tablename);
 extern InhInfo *getInherits(int *numInherits);
 extern void getTableAttrs(TableInfo *tbinfo, int numTables);
 extern IndInfo *getIndexes(int *numIndexes);