From b51df1f2584f99d67381a3702cec87cb3264e4a3 Mon Sep 17 00:00:00 2001 From: Andrew Dunstan Date: Fri, 28 Mar 2025 18:10:57 -0400 Subject: [PATCH 4/4] cleanups and use new simple list type --- src/bin/pg_dump/pg_dumpall.c | 28 ++--- src/bin/pg_dump/pg_restore.c | 208 +++++++---------------------------- 2 files changed, 56 insertions(+), 180 deletions(-) diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c index a3dcc585ace..6aab1bfe831 100644 --- a/src/bin/pg_dump/pg_dumpall.c +++ b/src/bin/pg_dump/pg_dumpall.c @@ -434,13 +434,13 @@ main(int argc, char *argv[]) archDumpFormat = parseDumpFormat(formatName); /* - * If non-plain format is specified then we must provide the - * file name to create one main directory. + * If a non-plain format is specified, a file name is also required as the + * path to the main directory. */ if (archDumpFormat != archNull && (!filename || strcmp(filename, "") == 0)) { - pg_log_error("options -F/--format=d|c|t requires option -f/--file with non-empty string"); + pg_log_error("option -F/--format=d|c|t requires option -f/--file"); pg_log_error_hint("Try \"%s --help\" for more information.", progname); exit_nicely(1); } @@ -513,14 +513,14 @@ main(int argc, char *argv[]) */ if (archDumpFormat != archNull) { - char toc_path[MAXPGPATH]; + char global_path[MAXPGPATH]; /* Create new directory or accept the empty existing directory. */ create_or_open_dir(filename); - snprintf(toc_path, MAXPGPATH, "%s/global.dat", filename); + snprintf(global_path, MAXPGPATH, "%s/global.dat", filename); - OPF = fopen(toc_path, PG_BINARY_W); + OPF = fopen(global_path, PG_BINARY_W); if (!OPF) pg_fatal("could not open global.dat file: %s", strerror(errno)); } @@ -1680,7 +1680,7 @@ dumpDatabases(PGconn *conn, ArchiveFormat archDumpFormat) } /* - * If this is non-plain dump format, then append dboid and dbname to + * If this is not a plain format dump, then append dboid and dbname to * the map.dat file. */ if (archDumpFormat != archNull) @@ -1688,7 +1688,7 @@ dumpDatabases(PGconn *conn, ArchiveFormat archDumpFormat) snprintf(dbfilepath, MAXPGPATH, "\"%s\"/\"%s\"", db_subdir, oid); /* Put one line entry for dboid and dbname in map file. */ - fprintf(map_file, "%s %s\n", oid, pg_strdup(dbname)); + fprintf(map_file, "%s %s\n", oid, dbname); } pg_log_info("dumping database \"%s\"", dbname); @@ -1734,17 +1734,17 @@ dumpDatabases(PGconn *conn, ArchiveFormat archDumpFormat) if (filename) { - char toc_path[MAXPGPATH]; + char global_path[MAXPGPATH]; if (archDumpFormat != archNull) - snprintf(toc_path, MAXPGPATH, "%s/global.dat", filename); + snprintf(global_path, MAXPGPATH, "%s/global.dat", filename); else - snprintf(toc_path, MAXPGPATH, "%s", filename); + snprintf(global_path, MAXPGPATH, "%s", filename); - OPF = fopen(toc_path, PG_BINARY_A); + OPF = fopen(global_path, PG_BINARY_A); if (!OPF) pg_fatal("could not re-open the output file \"%s\": %m", - toc_path); + global_path); } } @@ -1772,7 +1772,7 @@ runPgDump(const char *dbname, const char *create_opts, char *dbfile, initPQExpBuffer(&cmd); /* - * If this is non-plain format dump, then append file name and dump + * If this is not a plain format dump, then append file name and dump * format to the pg_dump command to get archive dump. */ if (archDumpFormat != archNull) diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c index e4093427e2f..44a24791a6e 100644 --- a/src/bin/pg_dump/pg_restore.c +++ b/src/bin/pg_dump/pg_restore.c @@ -46,8 +46,6 @@ #include #endif -#include "common/connect.h" -#include "compress_io.h" #include "common/string.h" #include "connectdb.h" #include "fe_utils/option_utils.h" @@ -55,47 +53,24 @@ #include "filter.h" #include "getopt_long.h" #include "parallel.h" -#include "pg_backup_archiver.h" #include "pg_backup_utils.h" -typedef struct SimpleDatabaseOidListCell -{ - struct SimpleDatabaseOidListCell *next; - Oid db_oid; - const char *db_name; -} SimpleDatabaseOidListCell; - -typedef struct SimpleDatabaseOidList -{ - SimpleDatabaseOidListCell *head; - SimpleDatabaseOidListCell *tail; -} SimpleDatabaseOidList; - static void usage(const char *progname); static void read_restore_filters(const char *filename, RestoreOptions *opts); -static bool IsFileExistsInDirectory(const char *dir, const char *filename); +static bool file_exists_in_directory(const char *dir, const char *filename); static int restoreOneDatabase(const char *inputFileSpec, RestoreOptions *opts, int numWorkers, bool append_data, int num); -static int ReadOneStatement(StringInfo inBuf, FILE *pfile); +static int read_one_statement(StringInfo inBuf, FILE *pfile); static int restoreAllDatabases(PGconn *conn, const char *dumpdirpath, SimpleStringList db_exclude_patterns, RestoreOptions *opts, int numWorkers); static int process_global_sql_commands(PGconn *conn, const char *dumpdirpath, const char *outfile); static void copy_or_print_global_file(const char *outfile, FILE *pfile); static int get_dbnames_list_to_restore(PGconn *conn, - SimpleDatabaseOidList *dbname_oid_list, + SimpleOidStringList *dbname_oid_list, SimpleStringList db_exclude_patterns); static int get_dbname_oid_list_from_mfile(const char *dumpdirpath, - SimpleDatabaseOidList *dbname_oid_list); -static void simple_db_oid_list_append(SimpleDatabaseOidList *list, Oid db_oid, - const char *dbname); -static void simple_string_full_list_delete(SimpleStringList *list); -static void simple_db_oid_full_list_delete(SimpleDatabaseOidList *list); -static void simple_db_oid_list_delete(SimpleDatabaseOidList *list, - SimpleDatabaseOidListCell *cell, - SimpleDatabaseOidListCell *prev); -static void simple_db_oid_list_append(SimpleDatabaseOidList *list, - Oid db_oid, const char *dbname); + SimpleOidStringList *dbname_oid_list); static size_t quote_literal_internal(char *dst, const char *src, size_t len); static char *quote_literal_cstr(const char *rawstr); static int on_exit_index = 0; @@ -521,8 +496,8 @@ main(int argc, char **argv) * databases from map.dat(if exist) file list and skip restoring for * --exclude-database patterns. */ - if (inputFileSpec != NULL && !IsFileExistsInDirectory(inputFileSpec, "toc.dat") && - IsFileExistsInDirectory(inputFileSpec, "global.dat")) + if (inputFileSpec != NULL && !file_exists_in_directory(inputFileSpec, "toc.dat") && + file_exists_in_directory(inputFileSpec, "global.dat")) { PGconn *conn = NULL; /* Connection to restore global sql commands. */ @@ -578,7 +553,7 @@ main(int argc, char **argv) } /* Free db pattern list. */ - simple_string_full_list_delete(&db_exclude_patterns); + simple_string_list_destroy(&db_exclude_patterns); } else /* process if global.dat file does not exist. */ { @@ -847,12 +822,12 @@ read_restore_filters(const char *filename, RestoreOptions *opts) } /* - * IsFileExistsInDirectory + * file_exists_in_directory * * Returns true if file exist in current directory. */ static bool -IsFileExistsInDirectory(const char *dir, const char *filename) +file_exists_in_directory(const char *dir, const char *filename) { struct stat st; char buf[MAXPGPATH]; @@ -864,7 +839,7 @@ IsFileExistsInDirectory(const char *dir, const char *filename) } /* - * ReadOneStatement + * read_one_statement * * This will start reading from passed file pointer using fgetc and read till * semicolon(sql statement terminator for global.dat file) @@ -873,7 +848,7 @@ IsFileExistsInDirectory(const char *dir, const char *filename) */ static int -ReadOneStatement(StringInfo inBuf, FILE *pfile) +read_one_statement(StringInfo inBuf, FILE *pfile) { int c; /* character read from getc() */ int m; @@ -941,27 +916,21 @@ ReadOneStatement(StringInfo inBuf, FILE *pfile) /* * get_dbnames_list_to_restore * - * This will remove entries from dbname_oid_list that pattern matching any - * in the db_exclude_patterns list. dbname_oid_list maybe inplace modified. + * This will mark for skipping any entries from dbname_oid_list that pattern match an + * entry in the db_exclude_patterns list. * - * returns, number of database will be restored. + * Returns the number of database to be restored. * */ static int get_dbnames_list_to_restore(PGconn *conn, - SimpleDatabaseOidList *dbname_oid_list, + SimpleOidStringList *dbname_oid_list, SimpleStringList db_exclude_patterns) { - SimpleDatabaseOidListCell *dboid_cell = dbname_oid_list->head; - SimpleDatabaseOidListCell *dboidprecell = NULL; int count_db = 0; PQExpBuffer query; PGresult *res; - /* Return 0 if there is no database to restore. */ - if (dboid_cell == NULL) - return 0; - query = createPQExpBuffer(); if (!conn) @@ -971,12 +940,12 @@ get_dbnames_list_to_restore(PGconn *conn, * Process one by one all dbnames and if specified to skip restoring, then * remove dbname from list. */ - while (dboid_cell != NULL) + for (SimpleOidStringListCell *db_cell = dbname_oid_list->head; + db_cell; db_cell = db_cell->next) { bool skip_db_restore = false; - SimpleDatabaseOidListCell *next = dboid_cell->next; - for (SimpleStringListCell *celldb = db_exclude_patterns.head; celldb; celldb = celldb->next) + for (SimpleStringListCell *pat_cell = db_exclude_patterns.head; pat_cell; pat_cell = pat_cell->next) { /* * the construct pattern matching query: @@ -990,21 +959,21 @@ get_dbnames_list_to_restore(PGconn *conn, * * If no db connection, then consider PATTERN as NAME. */ - if (pg_strcasecmp(dboid_cell->db_name, celldb->val) == 0) + if (pg_strcasecmp(db_cell->str, pat_cell->val) == 0) skip_db_restore = true; else if (conn) { int dotcnt; appendPQExpBufferStr(query, "SELECT 1 "); - processSQLNamePattern(conn, query, celldb->val, false, - false, NULL, quote_literal_cstr(dboid_cell->db_name), + processSQLNamePattern(conn, query, pat_cell->val, false, + false, NULL, quote_literal_cstr(db_cell->str), NULL, NULL, NULL, &dotcnt); if (dotcnt > 0) { pg_log_error("improper qualified name (too many dotted names): %s", - celldb->val); + db_cell->str); PQfinish(conn); exit_nicely(1); } @@ -1014,7 +983,7 @@ get_dbnames_list_to_restore(PGconn *conn, if ((PQresultStatus(res) == PGRES_TUPLES_OK) && PQntuples(res)) { skip_db_restore = true; - pg_log_info("database \"%s\" is matching with exclude pattern: \"%s\"", dboid_cell->db_name, celldb->val); + pg_log_info("database \"%s\" matches exclude pattern: \"%s\"", db_cell->str, pat_cell->val); } PQclear(res); @@ -1028,17 +997,13 @@ get_dbnames_list_to_restore(PGconn *conn, /* Increment count if database needs to be restored. */ if (skip_db_restore) { - pg_log_info("excluding database \"%s\"", dboid_cell->db_name); - simple_db_oid_list_delete(dbname_oid_list, dboid_cell, dboidprecell); + pg_log_info("excluding database \"%s\"", db_cell->str); + db_cell->oid = InvalidOid; } else { count_db++; - dboidprecell = dboid_cell; } - - /* Process next dbname from dbname list. */ - dboid_cell = next; } return count_db; @@ -1053,7 +1018,7 @@ get_dbnames_list_to_restore(PGconn *conn, * Returns, total number of database names in map.dat file. */ static int -get_dbname_oid_list_from_mfile(const char *dumpdirpath, SimpleDatabaseOidList *dbname_oid_list) +get_dbname_oid_list_from_mfile(const char *dumpdirpath, SimpleOidStringList *dbname_oid_list) { FILE *pfile; char map_file_path[MAXPGPATH]; @@ -1064,7 +1029,7 @@ get_dbname_oid_list_from_mfile(const char *dumpdirpath, SimpleDatabaseOidList *d * If there is only global.dat file in dump, then return from here as there * is no database to restore. */ - if (!IsFileExistsInDirectory(pg_strdup(dumpdirpath), "map.dat")) + if (!file_exists_in_directory(dumpdirpath, "map.dat")) { pg_log_info("databases restoring is skipped as map.dat file is not present in \"%s\"", dumpdirpath); return 0; @@ -1087,7 +1052,7 @@ get_dbname_oid_list_from_mfile(const char *dumpdirpath, SimpleDatabaseOidList *d /* Extract dboid. */ sscanf(line, "%u" , &db_oid); - sscanf(line, "%s" , db_oid_str); + sscanf(line, "%20s" , db_oid_str); /* Now copy dbname. */ strcpy(dbname, line + strlen(db_oid_str) + 1); @@ -1106,7 +1071,7 @@ get_dbname_oid_list_from_mfile(const char *dumpdirpath, SimpleDatabaseOidList *d * needs to skipped for restore or not but as of now, we are making * a list of all the databases. */ - simple_db_oid_list_append(dbname_oid_list, db_oid, dbname); + simple_oid_string_list_append(dbname_oid_list, db_oid, dbname); count++; } @@ -1132,8 +1097,7 @@ restoreAllDatabases(PGconn *conn, const char *dumpdirpath, SimpleStringList db_exclude_patterns, RestoreOptions *opts, int numWorkers) { - SimpleDatabaseOidList dbname_oid_list = {NULL, NULL}; - SimpleDatabaseOidListCell *dboid_cell; + SimpleOidStringList dbname_oid_list = {NULL, NULL}; int num_db_restore = 0; int num_total_db; int n_errors_total; @@ -1183,7 +1147,7 @@ restoreAllDatabases(PGconn *conn, const char *dumpdirpath, PQfinish(conn); /* Exit if no db needs to be restored. */ - if (dbname_oid_list.head == NULL) + if (dbname_oid_list.head == NULL || num_db_restore == 0) { pg_log_info("no database needs to restore out of %d databases", num_total_db); return n_errors_total; @@ -1196,13 +1160,16 @@ restoreAllDatabases(PGconn *conn, const char *dumpdirpath, * after skipping names of exclude-database. Now we can launch parallel * workers to restore these databases. */ - dboid_cell = dbname_oid_list.head; - - while(dboid_cell != NULL) + for (SimpleOidStringListCell *db_cell = dbname_oid_list.head; + db_cell; db_cell = db_cell->next) { char subdirpath[MAXPGPATH]; int n_errors; + /* ignore dbs marked for skipping */ + if (db_cell->oid == InvalidOid) + continue; + /* * We need to reset override_dbname so that objects can be restored into * already created database. (used with -d/--dbname option) @@ -1213,9 +1180,9 @@ restoreAllDatabases(PGconn *conn, const char *dumpdirpath, opts->cparams.override_dbname = NULL; } - snprintf(subdirpath, MAXPGPATH, "%s/databases/%u", dumpdirpath, dboid_cell->db_oid); + snprintf(subdirpath, MAXPGPATH, "%s/databases/%u", dumpdirpath, db_cell->oid); - pg_log_info("restoring database \"%s\"", dboid_cell->db_name); + pg_log_info("restoring database \"%s\"", db_cell->str); /* Restore single database. */ n_errors = restoreOneDatabase(subdirpath, opts, numWorkers, true, count); @@ -1224,10 +1191,9 @@ restoreAllDatabases(PGconn *conn, const char *dumpdirpath, if (n_errors) { n_errors_total += n_errors; - pg_log_warning("errors ignored on database \"%s\" restore: %d", dboid_cell->db_name, n_errors); + pg_log_warning("errors ignored on database \"%s\" restore: %d", db_cell->str, n_errors); } - dboid_cell = dboid_cell->next; count++; } @@ -1235,7 +1201,7 @@ restoreAllDatabases(PGconn *conn, const char *dumpdirpath, pg_log_info("number of restored databases are %d", num_db_restore); /* Free dbname and dboid list. */ - simple_db_oid_full_list_delete(&dbname_oid_list); + simple_oid_string_list_destroy(&dbname_oid_list); return n_errors_total; } @@ -1281,7 +1247,7 @@ process_global_sql_commands(PGconn *conn, const char *dumpdirpath, const char *o initStringInfo(&sqlstatement); /* Process file till EOF and execute sql statements. */ - while (ReadOneStatement(&sqlstatement, pfile) != EOF) + while (read_one_statement(&sqlstatement, pfile) != EOF) { pg_log_info("executing query: %s", sqlstatement.data); result = PQexec(conn, sqlstatement.data); @@ -1347,96 +1313,6 @@ copy_or_print_global_file(const char *outfile, FILE *pfile) fclose(OPF); } -/* - * simple_db_oid_list_append - * - * appends a node to the list in the end. - */ -static void -simple_db_oid_list_append(SimpleDatabaseOidList *list, Oid db_oid, - const char *dbname) -{ - SimpleDatabaseOidListCell *cell; - - cell = pg_malloc_object(SimpleDatabaseOidListCell); - - cell->next = NULL; - cell->db_oid = db_oid; - cell->db_name = pg_strdup(dbname); - - if (list->tail) - list->tail->next = cell; - else - list->head = cell; - list->tail = cell; -} - -/* - * simple_db_oid_full_list_delete - * - * delete all cell from dbname and dboid list. - */ -static void -simple_db_oid_full_list_delete(SimpleDatabaseOidList *list) -{ - SimpleDatabaseOidListCell *cell = list->head; - SimpleDatabaseOidListCell *nextcell = NULL; - - while (cell) - { - nextcell = cell->next; - pfree (cell); - cell = nextcell; - } - - list->head = NULL; - list->tail = NULL; -} - -/* - * simple_string_full_list_delete - * - * delete all cell from string list. - */ -static void -simple_string_full_list_delete(SimpleStringList *list) -{ - SimpleStringListCell *cell = list->head; - SimpleStringListCell *cellnext = NULL; - - while (cell) - { - cellnext = cell->next; - pfree(cell); - cell = cellnext; - } - - list->head = NULL; - list->tail = NULL; -} - -/* - * simple_db_oid_list_delete - * - * delete cell from database and oid list. - */ -static void -simple_db_oid_list_delete(SimpleDatabaseOidList *list, - SimpleDatabaseOidListCell *cell, - SimpleDatabaseOidListCell *prev) -{ - if (prev == NULL) - { - list->head = cell->next; - pfree(cell); - } - else - { - prev->next = cell->next; - pfree(cell); - } -} - /* * quote_literal_internal */ -- 2.34.1