From d9ed510ef64bb1962b2084f5b43aad03f1bd5cfa Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Tue, 27 Oct 2020 14:31:37 -0300 Subject: [PATCH] pg_dump: Lock all relations, not just plain tables MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Now that LOCK TABLE can take any relation type, acquire lock on all relations that are to be dumped. This prevents schema changes or deadlock errors that could cause a dump to fail after expending much effort. The server is tested to have the capability and the feature disabled if it doesn't, so that a patched pg_dump doesn't fail when connecting to an unpatched server. Backpatch to 9.5. Author: Álvaro Herrera Reviewed-by: Tom Lane Reported-by: Wells Oliver Discussion: https://postgr.es/m/20201021200659.GA32358@alvherre.pgsql --- src/bin/pg_dump/pg_backup.h | 2 ++ src/bin/pg_dump/pg_backup_db.c | 65 ++++++++++++++++++++++++++++++++++ src/bin/pg_dump/pg_backup_db.h | 2 ++ src/bin/pg_dump/pg_dump.c | 11 ++++-- 4 files changed, 77 insertions(+), 3 deletions(-) diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h index 8ae1388be41..876a97b5087 100644 --- a/src/bin/pg_dump/pg_backup.h +++ b/src/bin/pg_dump/pg_backup.h @@ -187,6 +187,8 @@ typedef struct Archive int minRemoteVersion; /* allowable range */ int maxRemoteVersion; + bool hasGenericLockTable; /* can LOCK TABLE do non-table rels */ + int numWorkers; /* number of parallel processes */ char *sync_snapshot_id; /* sync snapshot id for parallel * operation */ diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c index 6559151f518..2fd6789ccea 100644 --- a/src/bin/pg_dump/pg_backup_db.c +++ b/src/bin/pg_dump/pg_backup_db.c @@ -551,6 +551,71 @@ EndDBCopyMode(Archive *AHX, const char *tocEntryTag) } } +/* + * Does LOCK TABLE work on non-table relations on this server? + * + * Note: assumes it is called out of any transaction + */ +bool +IsLockTableGeneric(Archive *AHX) +{ + ArchiveHandle *AH = (ArchiveHandle *) AHX; + PGresult *res; + char *sqlstate; + bool retval; + + if (AHX->remoteVersion >= 140000) + return true; + else if (AHX->remoteVersion < 90500) + return false; + + StartTransaction(AHX); + + /* + * Try a LOCK TABLE on a well-known non-table catalog; WRONG_OBJECT_TYPE + * tells us that this server doesn't support locking non-table rels, while + * LOCK_NOT_AVAILABLE and INSUFFICIENT_PRIVILEGE tell us that it does. + * Report anything else as a fatal problem. + */ +#define ERRCODE_INSUFFICIENT_PRIVILEGE "42501" +#define ERRCODE_WRONG_OBJECT_TYPE "42809" +#define ERRCODE_LOCK_NOT_AVAILABLE "55P03" + res = PQexec(AH->connection, + "LOCK TABLE pg_catalog.pg_class_tblspc_relfilenode_index IN ACCESS SHARE MODE NOWAIT"); + switch (PQresultStatus(res)) + { + case PGRES_COMMAND_OK: + retval = true; + break; + case PGRES_FATAL_ERROR: + sqlstate = PQresultErrorField(res, PG_DIAG_SQLSTATE); + if (strcmp(sqlstate, ERRCODE_WRONG_OBJECT_TYPE) == 0) + { + retval = false; + break; + } + else if (strcmp(sqlstate, ERRCODE_LOCK_NOT_AVAILABLE) == 0 || + strcmp(sqlstate, ERRCODE_INSUFFICIENT_PRIVILEGE) == 0) + { + retval = true; + break; + } + /* else, falls through */ + default: + warn_or_exit_horribly(AH, modulename, + "LOCK TABLE failed for \"%s\": %s", + "pg_catalog.pg_class_tblspc_relfilenode_index", + PQerrorMessage(AH->connection)); + retval = false; /* not reached */ + break; + } + PQclear(res); + + CommitTransaction(AHX); + + return retval; +} + void StartTransaction(Archive *AHX) { diff --git a/src/bin/pg_dump/pg_backup_db.h b/src/bin/pg_dump/pg_backup_db.h index 527449e0440..9e7a84fdee9 100644 --- a/src/bin/pg_dump/pg_backup_db.h +++ b/src/bin/pg_dump/pg_backup_db.h @@ -20,6 +20,8 @@ extern PGresult *ExecuteSqlQueryForSingleRow(Archive *fout, char *query); extern void EndDBCopyMode(Archive *AHX, const char *tocEntryTag); +extern bool IsLockTableGeneric(Archive *AHX); + extern void StartTransaction(Archive *AHX); extern void CommitTransaction(Archive *AHX); diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index b30b87ae66d..c886ffefbd0 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -1062,6 +1062,9 @@ setup_connection(Archive *AH, const char *dumpencoding, ExecuteSqlStatement(AH, "SET row_security = off"); } + /* Detect whether LOCK TABLE can handle non-table relations */ + AH->hasGenericLockTable = IsLockTableGeneric(AH); + /* * Start transaction-snapshot mode transaction to dump consistent data. */ @@ -5398,10 +5401,12 @@ getTables(Archive *fout, int *numTables) * assume our lock on the child is enough to prevent schema * alterations to parent tables. * - * NOTE: it'd be kinda nice to lock other relations too, not only - * plain tables, but the backend doesn't presently allow that. + * On server versions that support it, we lock all relations not just + * plain tables. */ - if (tblinfo[i].dobj.dump && tblinfo[i].relkind == RELKIND_RELATION) + if (tblinfo[i].dobj.dump && + (fout->hasGenericLockTable || + tblinfo[i].relkind == RELKIND_RELATION)) { resetPQExpBuffer(query); appendPQExpBuffer(query, -- 2.39.5