0004-cleanups-and-use-new-simple-list-type.patch

From b51df1f2584f99d67381a3702cec87cb3264e4a3 Mon Sep 17 00:00:00 2001
From: Andrew Dunstan <andrew@dunslane.net>
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 <termios.h>
 #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