miscellaneous pg_upgrade cleanup

From c958311e5fd6df460e0c1289646fe99b9dd5f7f7 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 17 Sep 2024 11:30:50 -0500
Subject: [PATCH v1 1/3] remove extra whitespace in pg_upgrade report

---
 src/bin/pg_upgrade/check.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 1c277ce17b..c9a3f06b80 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -419,7 +419,7 @@ process_data_type_check(DbInfo *dbinfo, PGresult *res, void *arg)
 		 */
 		if (!(*state->result))
 		{
-			pg_log(PG_REPORT, "    failed check: %s", _(state->check->status));
+			pg_log(PG_REPORT, "failed check: %s", _(state->check->status));
 			appendPQExpBuffer(*state->report, "\n%s\n%s    %s\n",
 							  _(state->check->report_text),
 							  _("A list of the problem columns is in the file:"),
-- 
2.39.3 (Apple Git-146)

From 6f3cd97974eb6f73c1513c4e6093c1222268c42c Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 17 Sep 2024 11:57:52 -0500
Subject: [PATCH v1 2/3] improve style of upgrade task callback functions

---
 src/bin/pg_upgrade/check.c   | 205 +++++++++++++++--------------------
 src/bin/pg_upgrade/version.c |  29 +++--
 2 files changed, 99 insertions(+), 135 deletions(-)

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index c9a3f06b80..bc4c9b11a0 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -390,69 +390,55 @@ process_data_type_check(DbInfo *dbinfo, PGresult *res, void *arg)
 {
 	struct data_type_check_state *state = (struct data_type_check_state *) arg;
 	int			ntups = PQntuples(res);
+	char		output_path[MAXPGPATH];
+	int			i_nspname = PQfnumber(res, "nspname");
+	int			i_relname = PQfnumber(res, "relname");
+	int			i_attname = PQfnumber(res, "attname");
+	FILE	   *script = NULL;
 
 	AssertVariableIsOfType(&process_data_type_check, UpgradeTaskProcessCB);
 
-	if (ntups)
-	{
-		char		output_path[MAXPGPATH];
-		int			i_nspname;
-		int			i_relname;
-		int			i_attname;
-		FILE	   *script = NULL;
-		bool		db_used = false;
+	if (ntups == 0)
+		return;
 
-		snprintf(output_path, sizeof(output_path), "%s/%s",
-				 log_opts.basedir,
-				 state->check->report_filename);
+	snprintf(output_path, sizeof(output_path), "%s/%s",
+			 log_opts.basedir,
+			 state->check->report_filename);
 
-		/*
-		 * Make sure we have a buffer to save reports to now that we found a
-		 * first failing check.
-		 */
-		if (*state->report == NULL)
-			*state->report = createPQExpBuffer();
+	/*
+	 * Make sure we have a buffer to save reports to now that we found a first
+	 * failing check.
+	 */
+	if (*state->report == NULL)
+		*state->report = createPQExpBuffer();
 
-		/*
-		 * If this is the first time we see an error for the check in question
-		 * then print a status message of the failure.
-		 */
-		if (!(*state->result))
-		{
-			pg_log(PG_REPORT, "failed check: %s", _(state->check->status));
-			appendPQExpBuffer(*state->report, "\n%s\n%s    %s\n",
-							  _(state->check->report_text),
-							  _("A list of the problem columns is in the file:"),
-							  output_path);
-		}
-		*state->result = true;
+	/*
+	 * If this is the first time we see an error for the check in question
+	 * then print a status message of the failure.
+	 */
+	if (!(*state->result))
+	{
+		pg_log(PG_REPORT, "failed check: %s", _(state->check->status));
+		appendPQExpBuffer(*state->report, "\n%s\n%s    %s\n",
+						  _(state->check->report_text),
+						  _("A list of the problem columns is in the file:"),
+						  output_path);
+	}
+	*state->result = true;
 
-		i_nspname = PQfnumber(res, "nspname");
-		i_relname = PQfnumber(res, "relname");
-		i_attname = PQfnumber(res, "attname");
+	if (script == NULL &&
+		(script = fopen_priv(output_path, "a")) == NULL)
+		pg_fatal("could not open file \"%s\": %m", output_path);
 
-		for (int rowno = 0; rowno < ntups; rowno++)
-		{
-			if (script == NULL && (script = fopen_priv(output_path, "a")) == NULL)
-				pg_fatal("could not open file \"%s\": %m", output_path);
+	fprintf(script, "In database: %s\n", dbinfo->db_name);
 
-			if (!db_used)
-			{
-				fprintf(script, "In database: %s\n", dbinfo->db_name);
-				db_used = true;
-			}
-			fprintf(script, "  %s.%s.%s\n",
-					PQgetvalue(res, rowno, i_nspname),
-					PQgetvalue(res, rowno, i_relname),
-					PQgetvalue(res, rowno, i_attname));
-		}
+	for (int rowno = 0; rowno < ntups; rowno++)
+		fprintf(script, "  %s.%s.%s\n",
+				PQgetvalue(res, rowno, i_nspname),
+				PQgetvalue(res, rowno, i_relname),
+				PQgetvalue(res, rowno, i_attname));
 
-		if (script)
-		{
-			fclose(script);
-			script = NULL;
-		}
-	}
+	fclose(script);
 }
 
 /*
@@ -1234,7 +1220,6 @@ check_for_prepared_transactions(ClusterInfo *cluster)
 static void
 process_isn_and_int8_passing_mismatch(DbInfo *dbinfo, PGresult *res, void *arg)
 {
-	bool		db_used = false;
 	int			ntups = PQntuples(res);
 	int			i_nspname = PQfnumber(res, "nspname");
 	int			i_proname = PQfnumber(res, "proname");
@@ -1243,20 +1228,19 @@ process_isn_and_int8_passing_mismatch(DbInfo *dbinfo, PGresult *res, void *arg)
 	AssertVariableIsOfType(&process_isn_and_int8_passing_mismatch,
 						   UpgradeTaskProcessCB);
 
+	if (ntups == 0)
+		return;
+
+	if (report->file == NULL &&
+		(report->file = fopen_priv(report->path, "w")) == NULL)
+		pg_fatal("could not open file \"%s\": %m", report->path);
+
+	fprintf(report->file, "In database: %s\n", dbinfo->db_name);
+
 	for (int rowno = 0; rowno < ntups; rowno++)
-	{
-		if (report->file == NULL &&
-			(report->file = fopen_priv(report->path, "w")) == NULL)
-			pg_fatal("could not open file \"%s\": %m", report->path);
-		if (!db_used)
-		{
-			fprintf(report->file, "In database: %s\n", dbinfo->db_name);
-			db_used = true;
-		}
 		fprintf(report->file, "  %s.%s\n",
 				PQgetvalue(res, rowno, i_nspname),
 				PQgetvalue(res, rowno, i_proname));
-	}
 }
 
 /*
@@ -1324,7 +1308,6 @@ process_user_defined_postfix_ops(DbInfo *dbinfo, PGresult *res, void *arg)
 {
 	UpgradeTaskReport *report = (UpgradeTaskReport *) arg;
 	int			ntups = PQntuples(res);
-	bool		db_used = false;
 	int			i_oproid = PQfnumber(res, "oproid");
 	int			i_oprnsp = PQfnumber(res, "oprnsp");
 	int			i_oprname = PQfnumber(res, "oprname");
@@ -1334,26 +1317,22 @@ process_user_defined_postfix_ops(DbInfo *dbinfo, PGresult *res, void *arg)
 	AssertVariableIsOfType(&process_user_defined_postfix_ops,
 						   UpgradeTaskProcessCB);
 
-	if (!ntups)
+	if (ntups == 0)
 		return;
 
+	if (report->file == NULL &&
+		(report->file = fopen_priv(report->path, "w")) == NULL)
+		pg_fatal("could not open file \"%s\": %m", report->path);
+
+	fprintf(report->file, "In database: %s\n", dbinfo->db_name);
+
 	for (int rowno = 0; rowno < ntups; rowno++)
-	{
-		if (report->file == NULL &&
-			(report->file = fopen_priv(report->path, "w")) == NULL)
-			pg_fatal("could not open file \"%s\": %m", report->path);
-		if (!db_used)
-		{
-			fprintf(report->file, "In database: %s\n", dbinfo->db_name);
-			db_used = true;
-		}
 		fprintf(report->file, "  (oid=%s) %s.%s (%s.%s, NONE)\n",
 				PQgetvalue(res, rowno, i_oproid),
 				PQgetvalue(res, rowno, i_oprnsp),
 				PQgetvalue(res, rowno, i_oprname),
 				PQgetvalue(res, rowno, i_typnsp),
 				PQgetvalue(res, rowno, i_typname));
-	}
 }
 
 /*
@@ -1422,7 +1401,6 @@ static void
 process_incompat_polymorphics(DbInfo *dbinfo, PGresult *res, void *arg)
 {
 	UpgradeTaskReport *report = (UpgradeTaskReport *) arg;
-	bool		db_used = false;
 	int			ntups = PQntuples(res);
 	int			i_objkind = PQfnumber(res, "objkind");
 	int			i_objname = PQfnumber(res, "objname");
@@ -1430,21 +1408,19 @@ process_incompat_polymorphics(DbInfo *dbinfo, PGresult *res, void *arg)
 	AssertVariableIsOfType(&process_incompat_polymorphics,
 						   UpgradeTaskProcessCB);
 
-	for (int rowno = 0; rowno < ntups; rowno++)
-	{
-		if (report->file == NULL &&
-			(report->file = fopen_priv(report->path, "w")) == NULL)
-			pg_fatal("could not open file \"%s\": %m", report->path);
-		if (!db_used)
-		{
-			fprintf(report->file, "In database: %s\n", dbinfo->db_name);
-			db_used = true;
-		}
+	if (ntups == 0)
+		return;
+
+	if (report->file == NULL &&
+		(report->file = fopen_priv(report->path, "w")) == NULL)
+		pg_fatal("could not open file \"%s\": %m", report->path);
 
+	fprintf(report->file, "In database: %s\n", dbinfo->db_name);
+
+	for (int rowno = 0; rowno < ntups; rowno++)
 		fprintf(report->file, "  %s: %s\n",
 				PQgetvalue(res, rowno, i_objkind),
 				PQgetvalue(res, rowno, i_objname));
-	}
 }
 
 /*
@@ -1558,30 +1534,25 @@ static void
 process_with_oids_check(DbInfo *dbinfo, PGresult *res, void *arg)
 {
 	UpgradeTaskReport *report = (UpgradeTaskReport *) arg;
-	bool		db_used = false;
 	int			ntups = PQntuples(res);
 	int			i_nspname = PQfnumber(res, "nspname");
 	int			i_relname = PQfnumber(res, "relname");
 
 	AssertVariableIsOfType(&process_with_oids_check, UpgradeTaskProcessCB);
 
-	if (!ntups)
+	if (ntups == 0)
 		return;
 
+	if (report->file == NULL &&
+		(report->file = fopen_priv(report->path, "w")) == NULL)
+		pg_fatal("could not open file \"%s\": %m", report->path);
+
+	fprintf(report->file, "In database: %s\n", dbinfo->db_name);
+
 	for (int rowno = 0; rowno < ntups; rowno++)
-	{
-		if (report->file == NULL &&
-			(report->file = fopen_priv(report->path, "w")) == NULL)
-			pg_fatal("could not open file \"%s\": %m", report->path);
-		if (!db_used)
-		{
-			fprintf(report->file, "In database: %s\n", dbinfo->db_name);
-			db_used = true;
-		}
 		fprintf(report->file, "  %s.%s\n",
 				PQgetvalue(res, rowno, i_nspname),
 				PQgetvalue(res, rowno, i_relname));
-	}
 }
 
 /*
@@ -1693,7 +1664,6 @@ static void
 process_user_defined_encoding_conversions(DbInfo *dbinfo, PGresult *res, void *arg)
 {
 	UpgradeTaskReport *report = (UpgradeTaskReport *) arg;
-	bool		db_used = false;
 	int			ntups = PQntuples(res);
 	int			i_conoid = PQfnumber(res, "conoid");
 	int			i_conname = PQfnumber(res, "conname");
@@ -1702,24 +1672,20 @@ process_user_defined_encoding_conversions(DbInfo *dbinfo, PGresult *res, void *a
 	AssertVariableIsOfType(&process_user_defined_encoding_conversions,
 						   UpgradeTaskProcessCB);
 
-	if (!ntups)
+	if (ntups == 0)
 		return;
 
+	if (report->file == NULL &&
+		(report->file = fopen_priv(report->path, "w")) == NULL)
+		pg_fatal("could not open file \"%s\": %m", report->path);
+
+	fprintf(report->file, "In database: %s\n", dbinfo->db_name);
+
 	for (int rowno = 0; rowno < ntups; rowno++)
-	{
-		if (report->file == NULL &&
-			(report->file = fopen_priv(report->path, "w")) == NULL)
-			pg_fatal("could not open file \"%s\": %m", report->path);
-		if (!db_used)
-		{
-			fprintf(report->file, "In database: %s\n", dbinfo->db_name);
-			db_used = true;
-		}
 		fprintf(report->file, "  (oid=%s) %s.%s\n",
 				PQgetvalue(res, rowno, i_conoid),
 				PQgetvalue(res, rowno, i_nspname),
 				PQgetvalue(res, rowno, i_conname));
-	}
 }
 
 /*
@@ -1971,7 +1937,7 @@ static void
 process_old_sub_state_check(DbInfo *dbinfo, PGresult *res, void *arg)
 {
 	UpgradeTaskReport *report = (UpgradeTaskReport *) arg;
-	int			ntup = PQntuples(res);
+	int			ntups = PQntuples(res);
 	int			i_srsubstate = PQfnumber(res, "srsubstate");
 	int			i_subname = PQfnumber(res, "subname");
 	int			i_nspname = PQfnumber(res, "nspname");
@@ -1979,19 +1945,20 @@ process_old_sub_state_check(DbInfo *dbinfo, PGresult *res, void *arg)
 
 	AssertVariableIsOfType(&process_old_sub_state_check, UpgradeTaskProcessCB);
 
-	for (int i = 0; i < ntup; i++)
-	{
-		if (report->file == NULL &&
-			(report->file = fopen_priv(report->path, "w")) == NULL)
-			pg_fatal("could not open file \"%s\": %m", report->path);
+	if (ntups == 0)
+		return;
 
+	if (report->file == NULL &&
+		(report->file = fopen_priv(report->path, "w")) == NULL)
+		pg_fatal("could not open file \"%s\": %m", report->path);
+
+	for (int i = 0; i < ntups; i++)
 		fprintf(report->file, "The table sync state \"%s\" is not allowed for database:\"%s\" subscription:\"%s\" schema:\"%s\" relation:\"%s\"\n",
 				PQgetvalue(res, i, i_srsubstate),
 				dbinfo->db_name,
 				PQgetvalue(res, i, i_subname),
 				PQgetvalue(res, i, i_nspname),
 				PQgetvalue(res, i, i_relname));
-	}
 }
 
 /*
diff --git a/src/bin/pg_upgrade/version.c b/src/bin/pg_upgrade/version.c
index 5084b08805..2a3b6ebb34 100644
--- a/src/bin/pg_upgrade/version.c
+++ b/src/bin/pg_upgrade/version.c
@@ -147,31 +147,28 @@ old_9_6_invalidate_hash_indexes(ClusterInfo *cluster, bool check_mode)
 static void
 process_extension_updates(DbInfo *dbinfo, PGresult *res, void *arg)
 {
-	bool		db_used = false;
 	int			ntups = PQntuples(res);
 	int			i_name = PQfnumber(res, "name");
 	UpgradeTaskReport *report = (UpgradeTaskReport *) arg;
+	PQExpBufferData connectbuf;
 
 	AssertVariableIsOfType(&process_extension_updates, UpgradeTaskProcessCB);
 
-	for (int rowno = 0; rowno < ntups; rowno++)
-	{
-		if (report->file == NULL &&
-			(report->file = fopen_priv(report->path, "w")) == NULL)
-			pg_fatal("could not open file \"%s\": %m", report->path);
-		if (!db_used)
-		{
-			PQExpBufferData connectbuf;
+	if (ntups == 0)
+		return;
 
-			initPQExpBuffer(&connectbuf);
-			appendPsqlMetaConnect(&connectbuf, dbinfo->db_name);
-			fputs(connectbuf.data, report->file);
-			termPQExpBuffer(&connectbuf);
-			db_used = true;
-		}
+	if (report->file == NULL &&
+		(report->file = fopen_priv(report->path, "w")) == NULL)
+		pg_fatal("could not open file \"%s\": %m", report->path);
+
+	initPQExpBuffer(&connectbuf);
+	appendPsqlMetaConnect(&connectbuf, dbinfo->db_name);
+	fputs(connectbuf.data, report->file);
+	termPQExpBuffer(&connectbuf);
+
+	for (int rowno = 0; rowno < ntups; rowno++)
 		fprintf(report->file, "ALTER EXTENSION %s UPDATE;\n",
 				quote_identifier(PQgetvalue(res, rowno, i_name)));
-	}
 }
 
 /*
-- 
2.39.3 (Apple Git-146)

From 516b02f446572a8ae6550b39213bba3360263baf Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 17 Sep 2024 13:57:21 -0500
Subject: [PATCH v1 3/3] add names to some upgrade task structs

---
 src/bin/pg_upgrade/task.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/bin/pg_upgrade/task.c b/src/bin/pg_upgrade/task.c
index 8ef5d26eb5..ba1726c25e 100644
--- a/src/bin/pg_upgrade/task.c
+++ b/src/bin/pg_upgrade/task.c
@@ -89,7 +89,7 @@ struct UpgradeTask
 /*
  * The different states for a parallel slot.
  */
-typedef enum
+typedef enum UpgradeTaskSlotState
 {
 	FREE,						/* slot available for use in a new database */
 	CONNECTING,					/* waiting for connection to be established */
@@ -99,7 +99,7 @@ typedef enum
 /*
  * We maintain an array of user_opts.jobs slots to execute the task.
  */
-typedef struct
+typedef struct UpgradeTaskSlot
 {
 	UpgradeTaskSlotState state; /* state of the slot */
 	int			db_idx;			/* index of the database assigned to slot */
-- 
2.39.3 (Apple Git-146)