From dc2d24282a2fefb990d1e1e0a75e5a27ebce3d45 Mon Sep 17 00:00:00 2001 From: jian he Date: Thu, 8 Oct 2026 15:39:49 +0800 Subject: [PATCH v2 2/2] using ArchiveEntry not using fprintf for global objects * Run pgindent on pg_backup_archiver.c and pg_dumpall.c. * To keep the diff small, some former if/else branches are kept as bare { ... } blocks instead of being re-indented; they can be flattened later. * Keep OPF open in append mode and fflush() it before the archiver or pg_dump writes to the file, instead of repeatedly closing and reopening it. * As requested in [1], pg_dumpall.c should not write global objects with fprintf(OPF); they should be emitted through ArchiveEntry() in all output formats, in a uniform way. [1] https://www.postgresql.org/message-id/20260607000218.96.noahmisch%40microsoft.com --- src/bin/pg_dump/pg_backup_archiver.c | 26 +- src/bin/pg_dump/pg_dumpall.c | 478 ++++++++------------------- src/bin/pg_dump/t/007_pg_dumpall.pl | 7 +- 3 files changed, 167 insertions(+), 344 deletions(-) diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 0a755126838..c24c8bbc399 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -644,7 +644,9 @@ RestoreArchive(Archive *AHX, bool append_data) * object type. Most of the time this matches * te->desc, so search for that; however for the * different kinds of CONSTRAINTs, we know to - * search for hardcoded "DROP CONSTRAINT" instead. + * search for hardcoded "DROP CONSTRAINT" instead, + * and for pg_dumpall's DROP_GLOBAL entries, "DROP + * DATABASE". */ if (strcmp(te->desc, "DEFAULT") == 0 || strcmp(te->desc, "DATABASE PROPERTIES") == 0 || @@ -659,6 +661,8 @@ RestoreArchive(Archive *AHX, bool append_data) strcmp(te->desc, "CHECK CONSTRAINT") == 0 || strcmp(te->desc, "FK CONSTRAINT") == 0) strcpy(buffer, "DROP CONSTRAINT"); + else if (strcmp(te->desc, "DROP_GLOBAL") == 0) + strcpy(buffer, "DROP DATABASE"); else snprintf(buffer, sizeof(buffer), "DROP %s", te->desc); @@ -1670,7 +1674,7 @@ archputs(const char *s, Archive *AH) /* Public */ int -archprintf(Archive *AH, const char *fmt, ...) +archprintf(Archive *AH, const char *fmt,...) { int save_errno = errno; char *p; @@ -1774,7 +1778,7 @@ RestoreOutput(ArchiveHandle *AH, CompressFileHandle *savedOutput) * Print formatted text to the output file (usually stdout). */ int -ahprintf(ArchiveHandle *AH, const char *fmt, ...) +ahprintf(ArchiveHandle *AH, const char *fmt,...) { int save_errno = errno; char *p; @@ -1912,7 +1916,7 @@ ahwrite(const void *ptr, size_t size, size_t nmemb, ArchiveHandle *AH) /* on some error, we may decide to go on... */ void -warn_or_exit_horribly(ArchiveHandle *AH, const char *fmt, ...) +warn_or_exit_horribly(ArchiveHandle *AH, const char *fmt,...) { /* Stay quiet if this is a result of our own cancellation. */ if (!is_cancel_in_progress()) @@ -3020,7 +3024,8 @@ _tocEntryRequired(TocEntry *te, teSection curSection, ArchiveHandle *AH) /* These items are treated specially */ if (strcmp(te->desc, "ENCODING") == 0 || strcmp(te->desc, "STDSTRINGS") == 0 || - strcmp(te->desc, "SEARCHPATH") == 0) + strcmp(te->desc, "SEARCHPATH") == 0 || + strcmp(te->desc, "DEFAULT_TRANSACTION_READ_ONLY") == 0) return REQ_SPECIAL; if ((strcmp(te->desc, "STATISTICS DATA") == 0) || @@ -3054,9 +3059,9 @@ _tocEntryRequired(TocEntry *te, teSection curSection, ArchiveHandle *AH) * Global object TOC entries (e.g., ROLEs or TABLESPACEs) must not be * ignored. They have no te->namespace, so without this, the * --schema/--exclude-schema checks below would exclude them entirely. - * DROP_GLOBAL (pg_dumpall's cross-database drops) is included here - * rather than above with DATABASE/DATABASE PROPERTIES because it must - * be gated on dropSchema, not createDB. + * DROP_GLOBAL (pg_dumpall's cross-database drops) is included here rather + * than above with DATABASE/DATABASE PROPERTIES because it must be gated + * on dropSchema, not createDB. */ if (strcmp(te->desc, "ROLE") == 0 || strcmp(te->desc, "ROLE PROPERTIES") == 0 || @@ -3484,6 +3489,11 @@ _doSetFixedOutputState(ArchiveHandle *AH) { RestoreOptions *ropt = AH->public.ropt; + /* pg_dumpall archives: the restore must be able to write */ + for (TocEntry *te = AH->toc->next; te != AH->toc; te = te->next) + if (strcmp(te->desc, "DEFAULT_TRANSACTION_READ_ONLY") == 0) + ahprintf(AH, "%s", te->defn); + /* * Disable timeouts to allow for slow commands, idle parallel workers, etc */ diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c index f1152f46a42..38d3c8ca3e4 100644 --- a/src/bin/pg_dump/pg_dumpall.c +++ b/src/bin/pg_dump/pg_dumpall.c @@ -66,14 +66,12 @@ typedef struct static void help(void); -static void dropDBsText(PGconn *conn); -static void dropTablespacesText(PGconn *conn); -static void dropRolesText(PGconn *conn); static void dumpRoles(PGconn *conn); static void dumpRoleMembership(PGconn *conn); static void dumpRoleGUCPrivs(PGconn *conn); static void dumpTablespaces(PGconn *conn); static void dumpUserConfig(PGconn *conn, const char *username); +static void dropDatabases(PGconn *conn); static void dumpDatabases(PGconn *conn); static void dumpTimestamp(const char *msg); static int runPgDump(const char *dbname, const char *create_opts, char *dbfile); @@ -567,6 +565,17 @@ main(int argc, char *argv[]) if (!OPF) pg_fatal("could not open output file \"%s\": %m", filename); + fclose(OPF); + + /* + * Reopen in append mode, so that our writes always go to the end of + * the file even after the archiver or pg_dump has appended to it. + * Flushing OPF is then enough; we needn't close and reopen it around + * those writes. + */ + OPF = fopen(filename, PG_BINARY_A); + if (!OPF) + pg_fatal("could not open output file \"%s\": %m", filename); } else OPF = stdout; @@ -658,40 +667,60 @@ main(int argc, char *argv[]) executeCommand(conn, "SET quote_all_identifiers = true"); /* Create an archive file for global commands. */ - if (archDumpFormat != archNull) + if (archDumpFormat == archNull) + { + fprintf(OPF, "--\n-- PostgreSQL database cluster dump\n--\n\n"); + + if (verbose) + dumpTimestamp("Started on"); + + fflush(OPF); + } + + /* + * Check that no global object names contain newlines or carriage returns, + * which would break the map.dat file format. This is only needed for + * servers older than v19, which started prohibiting such names. + */ + if (server_version < 190000 && archDumpFormat != archNull) + check_for_invalid_global_names(conn, &database_exclude_names); + { PQExpBuffer qry = createPQExpBuffer(); - char global_path[MAXPGPATH]; const char *encname; pg_compress_specification compression_spec = {0}; - /* - * Check that no global object names contain newlines or carriage - * returns, which would break the map.dat file format. This is only - * needed for servers older than v19, which started prohibiting such - * names. - */ - if (server_version < 190000) - check_for_invalid_global_names(conn, &database_exclude_names); - - /* Set file path for global sql commands. */ - snprintf(global_path, MAXPGPATH, "%s/toc.glo", filename); - /* Open the output file */ - fout = CreateArchive(global_path, archCustom, compression_spec, - dosync, archModeWrite, NULL, DATA_DIR_SYNC_METHOD_FSYNC); + if (archDumpFormat != archNull) + { + char global_path[MAXPGPATH]; + + /* Set file path for global sql commands. */ + snprintf(global_path, MAXPGPATH, "%s/toc.glo", filename); + + fout = CreateArchive(global_path, archCustom, compression_spec, + dosync, archModeWrite, NULL, DATA_DIR_SYNC_METHOD_FSYNC); + } + else + fout = CreateArchive(filename, archNull, compression_spec, + dosync, archModeWrite, NULL, DATA_DIR_SYNC_METHOD_FSYNC); + + fout->std_strings = true; + fout->encoding = encoding; /* Make dump options accessible right away */ SetArchiveOptions(fout, &dopt, NULL); - ((ArchiveHandle *) fout)->connection = conn; ((ArchiveHandle *) fout)->public.numWorkers = 1; /* Register the cleanup hook */ on_exit_close_archive(fout); - /* Let the archiver know how noisy to be */ - fout->verbose = verbose; + /* + * Don't output TOC entry comments when dumping globals; they're + * purely informational and just add noise ahead of the SQL. + */ + ((ArchiveHandle *) fout)->noTocComments = 1; /* * We allow the server to be back to 9.2, and up to any minor release @@ -703,12 +732,12 @@ main(int argc, char *argv[]) fout->numWorkers = 1; /* Dump default_transaction_read_only. */ - appendPQExpBufferStr(qry, "SET default_transaction_read_only = off;\n\n"); + appendPQExpBufferStr(qry, "SET default_transaction_read_only = off;\n"); ArchiveEntry(fout, nilCatalogId, /* catalog ID */ createDumpId(), /* dump ID */ - ARCHIVE_OPTS(.tag = "default_transaction_read_only", - .description = "default_transaction_read_only", + ARCHIVE_OPTS(.tag = "DEFAULT_TRANSACTION_READ_ONLY", + .description = "DEFAULT_TRANSACTION_READ_ONLY", .section = SECTION_PRE_DATA, .createStmt = qry->data)); resetPQExpBuffer(qry); @@ -739,68 +768,9 @@ main(int argc, char *argv[]) .createStmt = qry->data)); destroyPQExpBuffer(qry); } - else - { - fprintf(OPF, "--\n-- PostgreSQL database cluster dump\n--\n\n"); - - if (verbose) - dumpTimestamp("Started on"); - - /* - * Enter restricted mode to block any unexpected psql meta-commands. A - * malicious source might try to inject a variety of things via bogus - * responses to queries. While we cannot prevent such sources from - * affecting the destination at restore time, we can block psql - * meta-commands so that the client machine that runs psql with the - * dump output remains unaffected. - */ - fprintf(OPF, "\\restrict %s\n\n", restrict_key); - - /* - * We used to emit \connect postgres here, but that served no purpose - * other than to break things for installations without a postgres - * database. Everything we're restoring here is a global, so - * whichever database we're connected to at the moment is fine. - */ - - /* Restore will need to write to the target cluster */ - fprintf(OPF, "SET default_transaction_read_only = off;\n\n"); - - /* Replicate encoding and standard_conforming_strings in output */ - fprintf(OPF, "SET client_encoding = '%s';\n", - pg_encoding_to_char(encoding)); - fprintf(OPF, "SET standard_conforming_strings = on;\n"); - fprintf(OPF, "\n"); - } if (!data_only && !statistics_only && !no_schema) { - /* - * For plain-text output with --clean, print the Drop databases/Drop - * tablespaces/Drop roles sections first, ahead of any creates (see - * dropDBsText() for why the order matters). For non-plain formats, - * dumpRoles()/dumpTablespaces()/dumpDatabases() instead attach a drop - * statement to each archive entry, and the archiver decides at - * restore time whether to use it. - * - * For non-text formats, pg_dumpall unconditionally process --clean - * option. In contrast, pg_restore only applies it if the user - * explicitly provides the flag. This discrepancy resolves corner - * cases where pg_restore requires cleanup instructions that may be - * missing from a standard pg_dumpall output. - */ - if (archDumpFormat == archNull && output_clean) - { - if (!globals_only && !roles_only && !tablespaces_only) - dropDBsText(conn); - - if (!roles_only && !no_tablespaces) - dropTablespacesText(conn); - - if (!tablespaces_only) - dropRolesText(conn); - } - if (!tablespaces_only) { /* Dump roles (users) */ @@ -817,24 +787,34 @@ main(int argc, char *argv[]) /* Dump tablespaces */ if (!roles_only && !no_tablespaces) dumpTablespaces(conn); + + if (!globals_only && !roles_only && !tablespaces_only) + dropDatabases(conn); } if (archDumpFormat == archNull) { - /* - * Exit restricted mode just before dumping the databases. pg_dump - * will handle entering restricted mode again as appropriate. - */ - fprintf(OPF, "\\unrestrict %s\n\n", restrict_key); + RestoreOptions *ropt = NewRestoreOptions(); + + ropt->filename = filename; + ropt->dropSchema = output_clean; + ropt->if_exists = if_exists; + ropt->restrict_key = restrict_key; + SetArchiveOptions(fout, &dopt, ropt); + ProcessArchiveRestoreOptions(fout); + + /* write out all global objects before any database */ + RestoreArchive(fout, true); + CloseArchive(fout); } if (!globals_only && !roles_only && !tablespaces_only) dumpDatabases(conn); + PQfinish(conn); + if (archDumpFormat == archNull) { - PQfinish(conn); - if (verbose) dumpTimestamp("Completed on"); fprintf(OPF, "--\n-- PostgreSQL database cluster dump complete\n--\n\n"); @@ -941,125 +921,6 @@ help(void) printf(_("%s home page: <%s>\n"), PACKAGE_NAME, PACKAGE_URL); } - -/* - * Print the "Drop databases"/"Drop tablespaces"/"Drop roles" plain-text - * sections, in that order, ahead of any "create" sections. The order - * matters: a database may depend on a tablespace or role, so those must - * go first. Non-plain formats need no such helper; the archiver's - * restore-time drop pass already runs all drops before any creates, - * regardless of registration order. - */ -static void -dropDBsText(PGconn *conn) -{ - PGresult *res; - int i; - - Assert(archDumpFormat == archNull); - - res = executeQuery(conn, - "SELECT datname " - "FROM pg_database d " - "WHERE datallowconn AND datconnlimit != -2 " - "ORDER BY datname"); - - if (PQntuples(res) > 0) - fprintf(OPF, "--\n-- Drop databases (except postgres and template1)\n--\n\n"); - - for (i = 0; i < PQntuples(res); i++) - { - char *dbname = PQgetvalue(res, i, 0); - - if (strcmp(dbname, "template1") != 0 && - strcmp(dbname, "template0") != 0 && - strcmp(dbname, "postgres") != 0) - fprintf(OPF, "DROP DATABASE %s%s;\n", - if_exists ? "IF EXISTS " : "", - fmtId(dbname)); - } - - PQclear(res); - - if (PQntuples(res) > 0) - fprintf(OPF, "\n\n"); -} - - -/* - * Drop tablespaces (see dropDBsText() for why this is plain-text only). - */ -static void -dropTablespacesText(PGconn *conn) -{ - PGresult *res; - int i; - - Assert(archDumpFormat == archNull); - - res = executeQuery(conn, "SELECT spcname " - "FROM pg_catalog.pg_tablespace " - "WHERE spcname !~ '^pg_' " - "ORDER BY 1"); - - if (PQntuples(res) > 0) - fprintf(OPF, "--\n-- Drop tablespaces\n--\n\n"); - - for (i = 0; i < PQntuples(res); i++) - { - char *spcname = PQgetvalue(res, i, 0); - - fprintf(OPF, "DROP TABLESPACE %s%s;\n", - if_exists ? "IF EXISTS " : "", - fmtId(spcname)); - } - - PQclear(res); - - if (PQntuples(res) > 0) - fprintf(OPF, "\n\n"); -} - - -/* - * Drop roles (see dropDBsText() for why this is plain-text only). - */ -static void -dropRolesText(PGconn *conn) -{ - PQExpBuffer buf = createPQExpBuffer(); - PGresult *res; - int i; - - Assert(archDumpFormat == archNull); - - printfPQExpBuffer(buf, - "SELECT rolname " - "FROM %s " - "WHERE rolname !~ '^pg_' " - "ORDER BY 1", role_catalog); - - res = executeQuery(conn, buf->data); - destroyPQExpBuffer(buf); - - if (PQntuples(res) > 0) - fprintf(OPF, "--\n-- Drop roles\n--\n\n"); - - for (i = 0; i < PQntuples(res); i++) - { - char *rolename = PQgetvalue(res, i, 0); - - fprintf(OPF, "DROP ROLE %s%s;\n", - if_exists ? "IF EXISTS " : "", - fmtId(rolename)); - } - - PQclear(res); - - if (PQntuples(res) > 0) - fprintf(OPF, "\n\n"); -} - /* * Drop and dump roles. */ @@ -1119,9 +980,6 @@ dumpRoles(PGconn *conn) i_rolcomment = PQfnumber(res, "rolcomment"); i_is_current_user = PQfnumber(res, "is_current_user"); - if (PQntuples(res) > 0 && archDumpFormat == archNull) - fprintf(OPF, "--\n-- Roles\n--\n\n"); - for (i = 0; i < PQntuples(res); i++) { const char *rolename; @@ -1229,15 +1087,6 @@ dumpRoles(PGconn *conn) "ROLE", rolename, seclabel_buf); - if (archDumpFormat == archNull) - { - fprintf(OPF, "%s", buf->data); - fprintf(OPF, "%s", comment_buf->data); - - if (seclabel_buf->data[0] != '\0') - fprintf(OPF, "%s", seclabel_buf->data); - } - else { char *tag = psprintf("ROLE %s", fmtId(rolename)); DumpId roleDumpId = createDumpId(); @@ -1279,17 +1128,10 @@ dumpRoles(PGconn *conn) * We do it this way because config settings for roles could mention the * names of other roles. */ - if (PQntuples(res) > 0 && archDumpFormat == archNull) - fprintf(OPF, "\n--\n-- User Configurations\n--\n"); - for (i = 0; i < PQntuples(res); i++) dumpUserConfig(conn, PQgetvalue(res, i, i_rolname)); PQclear(res); - - if (archDumpFormat == archNull) - fprintf(OPF, "\n\n"); - destroyPQExpBuffer(buf); destroyPQExpBuffer(delQry); destroyPQExpBuffer(comment_buf); @@ -1370,9 +1212,6 @@ dumpRoleMembership(PGconn *conn) i_inherit_option = PQfnumber(res, "inherit_option"); i_set_option = PQfnumber(res, "set_option"); - if (PQntuples(res) > 0 && archDumpFormat == archNull) - fprintf(OPF, "--\n-- Role memberships\n--\n\n"); - /* * We can't dump these GRANT commands in arbitrary order, because a role * that is named as a grantor must already have ADMIN OPTION on the role @@ -1545,16 +1384,13 @@ dumpRoleMembership(PGconn *conn) appendPQExpBuffer(querybuf, " GRANTED BY %s", fmtId(grantor)); appendPQExpBufferStr(querybuf, ";\n"); - if (archDumpFormat == archNull) - fprintf(OPF, "%s", querybuf->data); - else - ArchiveEntry(fout, - nilCatalogId, /* catalog ID */ - createDumpId(), /* dump ID */ - ARCHIVE_OPTS(.tag = psprintf("ROLE %s", fmtId(role)), - .description = "ROLE PROPERTIES", - .section = SECTION_PRE_DATA, - .createStmt = querybuf->data)); + ArchiveEntry(fout, + nilCatalogId, /* catalog ID */ + createDumpId(), /* dump ID */ + ARCHIVE_OPTS(.tag = psprintf("ROLE %s", fmtId(role)), + .description = "ROLE PROPERTIES", + .section = SECTION_PRE_DATA, + .createStmt = querybuf->data)); } } @@ -1567,9 +1403,6 @@ dumpRoleMembership(PGconn *conn) destroyPQExpBuffer(buf); destroyPQExpBuffer(querybuf); destroyPQExpBuffer(optbuf); - - if (archDumpFormat == archNull) - fprintf(OPF, "\n\n"); } @@ -1596,9 +1429,6 @@ dumpRoleGUCPrivs(PGconn *conn) "FROM pg_catalog.pg_parameter_acl " "ORDER BY 1"); - if (PQntuples(res) > 0 && archDumpFormat == archNull) - fprintf(OPF, "--\n-- Role privileges on configuration parameters\n--\n\n"); - for (i = 0; i < PQntuples(res); i++) { PQExpBuffer buf = createPQExpBuffer(); @@ -1621,25 +1451,19 @@ dumpRoleGUCPrivs(PGconn *conn) exit_nicely(1); } - if (archDumpFormat == archNull) - fprintf(OPF, "%s", buf->data); - else - ArchiveEntry(fout, - nilCatalogId, /* catalog ID */ - createDumpId(), /* dump ID */ - ARCHIVE_OPTS(.tag = psprintf("ROLE %s", fmtId(parowner)), - .description = "ROLE PROPERTIES", - .section = SECTION_PRE_DATA, - .createStmt = buf->data)); + ArchiveEntry(fout, + nilCatalogId, /* catalog ID */ + createDumpId(), /* dump ID */ + ARCHIVE_OPTS(.tag = psprintf("ROLE %s", fmtId(parowner)), + .description = "ROLE PROPERTIES", + .section = SECTION_PRE_DATA, + .createStmt = buf->data)); pg_free(fparname); destroyPQExpBuffer(buf); } PQclear(res); - - if (archDumpFormat == archNull) - fprintf(OPF, "\n\n"); } @@ -1669,9 +1493,6 @@ dumpTablespaces(PGconn *conn) "WHERE spcname !~ '^pg_' " "ORDER BY 1"); - if (PQntuples(res) > 0 && archDumpFormat == archNull) - fprintf(OPF, "--\n-- Tablespaces\n--\n\n"); - for (i = 0; i < PQntuples(res); i++) { PQExpBuffer buf = createPQExpBuffer(); @@ -1745,17 +1566,6 @@ dumpTablespaces(PGconn *conn) "TABLESPACE", spcname, seclabel_buf); - if (archDumpFormat == archNull) - { - fprintf(OPF, "%s", buf->data); - - if (comment_buf->data[0] != '\0') - fprintf(OPF, "%s", comment_buf->data); - - if (seclabel_buf->data[0] != '\0') - fprintf(OPF, "%s", seclabel_buf->data); - } - else { char *tag = psprintf("TABLESPACE %s", fmtId(fspcname)); DumpId spcDumpId = createDumpId(); @@ -1800,9 +1610,6 @@ dumpTablespaces(PGconn *conn) destroyPQExpBuffer(delQry); destroyPQExpBuffer(comment_buf); destroyPQExpBuffer(seclabel_buf); - - if (archDumpFormat == archNull) - fprintf(OPF, "\n\n"); } @@ -1824,15 +1631,6 @@ dumpUserConfig(PGconn *conn, const char *username) res = executeQuery(conn, buf->data); - if (PQntuples(res) > 0 && archDumpFormat == archNull) - { - char *sanitized; - - sanitized = sanitize_line(username, true); - fprintf(OPF, "\n--\n-- User Config \"%s\"\n--\n\n", sanitized); - free(sanitized); - } - for (int i = 0; i < PQntuples(res); i++) { resetPQExpBuffer(buf); @@ -1840,16 +1638,13 @@ dumpUserConfig(PGconn *conn, const char *username) "ROLE", username, NULL, NULL, buf); - if (archDumpFormat == archNull) - fprintf(OPF, "%s", buf->data); - else - ArchiveEntry(fout, - nilCatalogId, /* catalog ID */ - createDumpId(), /* dump ID */ - ARCHIVE_OPTS(.tag = psprintf("ROLE %s", fmtId(username)), - .description = "ROLE PROPERTIES", - .section = SECTION_PRE_DATA, - .createStmt = buf->data)); + ArchiveEntry(fout, + nilCatalogId, /* catalog ID */ + createDumpId(), /* dump ID */ + ARCHIVE_OPTS(.tag = psprintf("ROLE %s", fmtId(username)), + .description = "ROLE PROPERTIES", + .section = SECTION_PRE_DATA, + .createStmt = buf->data)); } PQclear(res); @@ -1911,6 +1706,55 @@ expand_dbname_patterns(PGconn *conn, destroyPQExpBuffer(query); } +/* + * Register a drop-only archive entry for each database that dumpDatabases() + * will recreate. A database may depend on a tablespace or role, so it must + * be dropped before them. The archiver's drop pass processes entries in + * reverse registration order, so this must be called after dumpRoles() and + * dumpTablespaces(). + * + * template1 and postgres are skipped: they are assumed to exist in the target + * cluster, and with --clean dumpDatabases() has pg_dump drop and recreate + * them instead. IF EXISTS, if requested, is added by the archiver. + */ +static void +dropDatabases(PGconn *conn) +{ + PQExpBuffer delQry = createPQExpBuffer(); + PGresult *res; + + /* This must agree with dumpDatabases() */ + res = executeQuery(conn, + "SELECT datname " + "FROM pg_catalog.pg_database " + "WHERE datallowconn AND datconnlimit != -2 " + "AND datname NOT IN ('template0', 'template1', 'postgres') " + "ORDER BY datname"); + + for (int i = 0; i < PQntuples(res); i++) + { + char *dbname = PQgetvalue(res, i, 0); + + /* Excluded databases are not recreated, so don't drop them either */ + if (simple_string_list_member(&database_exclude_names, dbname)) + continue; + + resetPQExpBuffer(delQry); + appendPQExpBuffer(delQry, "DROP DATABASE %s;\n", fmtId(dbname)); + + ArchiveEntry(fout, + nilCatalogId, /* catalog ID */ + createDumpId(), /* dump ID */ + ARCHIVE_OPTS(.tag = psprintf("DATABASE %s", fmtId(dbname)), + .description = "DROP_GLOBAL", + .section = SECTION_PRE_DATA, + .dropStmt = delQry->data)); + } + + PQclear(res); + destroyPQExpBuffer(delQry); +} + /* * Dump contents of databases. */ @@ -1918,7 +1762,6 @@ static void dumpDatabases(PGconn *conn) { PGresult *res; - PQExpBuffer delQry = createPQExpBuffer(); int i; char db_subdir[MAXPGPATH]; char dbfilepath[MAXPGPATH]; @@ -2009,11 +1852,11 @@ dumpDatabases(PGconn *conn) /* * We assume that "template1" and "postgres" already exist in the - * target installation and are never dropped above, for fear of - * dropping the DB the restore script is initially connected to. If - * --clean was specified, tell pg_dump to drop and recreate them; - * otherwise we'll merely restore their contents. Other databases - * should simply be created. + * target installation and are never dropped by dropDatabases(), for + * fear of dropping the DB the restore script is initially connected + * to. If --clean was specified, tell pg_dump to drop and recreate + * them; otherwise we'll merely restore their contents. Other + * databases should simply be created. */ if (strcmp(dbname, "template1") == 0 || strcmp(dbname, "postgres") == 0) { @@ -2028,9 +1871,6 @@ dumpDatabases(PGconn *conn) else create_opts = "--create"; - if (filename && archDumpFormat == archNull) - fclose(OPF); - if (archDumpFormat != archNull) { /* Compute this database's own archive file path. */ @@ -2043,36 +1883,11 @@ dumpDatabases(PGconn *conn) /* Put one line entry for dboid and dbname in map file. */ fprintf(map_file, "%s %s\n", oid, dbname); - - /* Let the archiver drop this database first, if --clean. */ - if (!data_only && !statistics_only && !no_schema && - strcmp(dbname, "template1") != 0 && strcmp(dbname, "postgres") != 0) - { - resetPQExpBuffer(delQry); - appendPQExpBuffer(delQry, "DROP DATABASE IF EXISTS %s;\n", - fmtId(dbname)); - - ArchiveEntry(fout, - nilCatalogId, /* catalog ID */ - createDumpId(), /* dump ID */ - ARCHIVE_OPTS(.tag = psprintf("DATABASE %s", fmtId(dbname)), - .description = "DROP_GLOBAL", - .section = SECTION_PRE_DATA, - .dropStmt = delQry->data)); - } } ret = runPgDump(dbname, create_opts, dbfilepath); if (ret != 0) pg_fatal("pg_dump failed on database \"%s\", exiting", dbname); - - if (filename && archDumpFormat == archNull) - { - OPF = fopen(filename, PG_BINARY_A); - if (!OPF) - pg_fatal("could not re-open the output file \"%s\": %m", - filename); - } } /* Close map file */ @@ -2090,7 +1905,6 @@ dumpDatabases(PGconn *conn) } PQclear(res); - destroyPQExpBuffer(delQry); } diff --git a/src/bin/pg_dump/t/007_pg_dumpall.pl b/src/bin/pg_dump/t/007_pg_dumpall.pl index debaf1e770b..7b261ae4867 100644 --- a/src/bin/pg_dump/t/007_pg_dumpall.pl +++ b/src/bin/pg_dump/t/007_pg_dumpall.pl @@ -445,8 +445,7 @@ unlike( 'commented out database in map.dat is not restored'); # Test 12: --clean without --if-exists should not add IF EXISTS (no -# auto-implication). DROP DATABASE always includes IF EXISTS, unlike -# ROLE/TABLESPACE. +# auto-implication). $node->command_ok( [ 'pg_restore', '-C', @@ -469,8 +468,8 @@ like( ); like( $clean_output, - qr/DROP DATABASE IF EXISTS/, - '--clean without --if-exists: DROP DATABASE IF EXISTS in output'); + qr/DROP DATABASE(?! IF EXISTS)/, + '--clean without --if-exists: DROP DATABASE without IF EXISTS in output'); # Test 13: --clean --if-exists adds IF EXISTS to all object types. $node->command_ok( -- 2.34.1