From 0e1a5075fdaec2f42f96cf2a1938c99fcc6b026c Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 11 Jan 2002 23:21:55 +0000 Subject: [PATCH] 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. --- src/bin/pg_dump/common.c | 4 +- src/bin/pg_dump/pg_dump.c | 82 +++++++++++++++++++++++++++++++-------- src/bin/pg_dump/pg_dump.h | 9 +++-- 3 files changed, 74 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c index d9d358892c..1a32e22b22 100644 --- 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 5c2c904555..8fb0812dd6 100644 --- 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 3a59d5edf1..3af39b8d2e 100644 --- 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);