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 - dave@bensoft.com - 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 - dave@bensoft.com - 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);