From 5cff5b577918cf29e0ba3a35158c27cfe2de280c Mon Sep 17 00:00:00 2001
From: Bruce Momjian <bruce@momjian.us>
Date: Wed, 5 Jan 2011 11:37:08 -0500
Subject: [PATCH] Clarify pg_upgrade's creation of the map file structure. 
 Also clean up pg_dump's calling of pg_upgrade_support functions.

---
 contrib/pg_upgrade/info.c            | 72 ++++++++++------------------
 contrib/pg_upgrade/pg_upgrade.h      | 18 ++++---
 contrib/pg_upgrade/version_old_8_3.c | 16 +++----
 src/bin/pg_dump/pg_dump.c            | 43 +++++++++--------
 4 files changed, 69 insertions(+), 80 deletions(-)

diff --git a/contrib/pg_upgrade/info.c b/contrib/pg_upgrade/info.c
index 8d566c0b2d6..83afb926887 100644
--- a/contrib/pg_upgrade/info.c
+++ b/contrib/pg_upgrade/info.c
@@ -33,8 +33,6 @@ static RelInfo *relarr_lookup_rel_oid(ClusterInfo *cluster, RelInfoArr *rel_arr,
  * generates database mappings for "old_db" and "new_db". Returns a malloc'ed
  * array of mappings. nmaps is a return parameter which refers to the number
  * mappings.
- *
- * NOTE: Its the Caller's responsibility to free the returned array.
  */
 FileNameMap *
 gen_db_file_maps(DbInfo *old_db, DbInfo *new_db,
@@ -45,19 +43,19 @@ gen_db_file_maps(DbInfo *old_db, DbInfo *new_db,
 	int			num_maps = 0;
 
 	maps = (FileNameMap *) pg_malloc(sizeof(FileNameMap) *
-									 new_db->rel_arr.nrels);
+									 old_db->rel_arr.nrels);
 
-	for (relnum = 0; relnum < new_db->rel_arr.nrels; relnum++)
+	for (relnum = 0; relnum < old_db->rel_arr.nrels; relnum++)
 	{
-		RelInfo    *newrel = &new_db->rel_arr.rels[relnum];
-		RelInfo    *oldrel;
+		RelInfo    *oldrel = &old_db->rel_arr.rels[relnum];
+		RelInfo    *newrel;
 
-		/* toast tables are handled by their parent */
-		if (strcmp(newrel->nspname, "pg_toast") == 0)
+		/* toast tables are handled by their parents */
+		if (strcmp(oldrel->nspname, "pg_toast") == 0)
 			continue;
 
-		oldrel = relarr_lookup_rel_name(&old_cluster, &old_db->rel_arr,
-								   newrel->nspname, newrel->relname);
+		newrel = relarr_lookup_rel_name(&old_cluster, &old_db->rel_arr,
+								   oldrel->nspname, oldrel->relname);
 
 		create_rel_filename_map(old_pgdata, new_pgdata, old_db, new_db,
 				oldrel, newrel, maps + num_maps);
@@ -65,52 +63,36 @@ gen_db_file_maps(DbInfo *old_db, DbInfo *new_db,
 
 		/*
 		 * So much for mapping this relation;  now we need a mapping
-		 * for its corresponding toast relation, if any.
+		 * for its corresponding toast relation and toast index, if any.
 		 */
 		if (oldrel->toastrelid > 0)
 		{
-			RelInfo    *new_toast;
-			RelInfo    *old_toast;
-			char		new_name[MAXPGPATH];
-			char		old_name[MAXPGPATH];
-
-			/* construct the new and old relnames for the toast relation */
-			snprintf(old_name, sizeof(old_name), "pg_toast_%u", oldrel->reloid);
-			snprintf(new_name, sizeof(new_name), "pg_toast_%u", newrel->reloid);
+			char		old_name[MAXPGPATH], new_name[MAXPGPATH];
+			RelInfo    *old_toast, *new_toast;
 
-			/* look them up in their respective arrays */
 			old_toast = relarr_lookup_rel_oid(&old_cluster, &old_db->rel_arr,
-											 oldrel->toastrelid);
-			new_toast = relarr_lookup_rel_name(&new_cluster, &new_db->rel_arr,
-										  "pg_toast", new_name);
+											  oldrel->toastrelid);
+			new_toast = relarr_lookup_rel_oid(&new_cluster, &new_db->rel_arr,
+											  newrel->toastrelid);
 
-			/* finally create a mapping for them */
 			create_rel_filename_map(old_pgdata, new_pgdata, old_db, new_db,
 					old_toast, new_toast, maps + num_maps);
 			num_maps++;
 
 			/*
-			 * also need to provide a mapping for the index of this toast
+			 * We also need to provide a mapping for the index of this toast
 			 * relation. The procedure is similar to what we did above for
 			 * toast relation itself, the only difference being that the
 			 * relnames need to be appended with _index.
 			 */
-
-			/*
-			 * construct the new and old relnames for the toast index
-			 * relations
-			 */
 			snprintf(old_name, sizeof(old_name), "%s_index", old_toast->relname);
-			snprintf(new_name, sizeof(new_name), "pg_toast_%u_index",
-					 newrel->reloid);
+			snprintf(new_name, sizeof(new_name), "%s_index", new_toast->relname);
 
-			/* look them up in their respective arrays */
 			old_toast = relarr_lookup_rel_name(&old_cluster, &old_db->rel_arr,
 										  "pg_toast", old_name);
 			new_toast = relarr_lookup_rel_name(&new_cluster, &new_db->rel_arr,
 										  "pg_toast", new_name);
 
-			/* finally create a mapping for them */
 			create_rel_filename_map(old_pgdata, new_pgdata, old_db,
 					new_db, old_toast, new_toast, maps + num_maps);
 			num_maps++;
@@ -133,15 +115,6 @@ create_rel_filename_map(const char *old_data, const char *new_data,
 			  const RelInfo *old_rel, const RelInfo *new_rel,
 			  FileNameMap *map)
 {
-	map->old_relfilenode = old_rel->relfilenode;
-	map->new_relfilenode = new_rel->relfilenode;
-
-	snprintf(map->old_nspname, sizeof(map->old_nspname), "%s", old_rel->nspname);
-	snprintf(map->new_nspname, sizeof(map->new_nspname), "%s", new_rel->nspname);
-
-	snprintf(map->old_relname, sizeof(map->old_relname), "%s", old_rel->relname);
-	snprintf(map->new_relname, sizeof(map->new_relname), "%s", new_rel->relname);
-
 	if (strlen(old_rel->tablespace) == 0)
 	{
 		/*
@@ -155,14 +128,21 @@ create_rel_filename_map(const char *old_data, const char *new_data,
 	}
 	else
 	{
-		/*
-		 * relation belongs to some tablespace, so use the tablespace location
-		 */
+		/* relation belongs to a tablespace, so use the tablespace location */
 		snprintf(map->old_dir, sizeof(map->old_dir), "%s%s/%u", old_rel->tablespace,
 				 old_cluster.tablespace_suffix, old_db->db_oid);
 		snprintf(map->new_dir, sizeof(map->new_dir), "%s%s/%u", new_rel->tablespace,
 				 new_cluster.tablespace_suffix, new_db->db_oid);
 	}
+
+	map->old_relfilenode = old_rel->relfilenode;
+	map->new_relfilenode = new_rel->relfilenode;
+
+	/* used only for logging and error reporing */
+	snprintf(map->old_nspname, sizeof(map->old_nspname), "%s", old_rel->nspname);
+	snprintf(map->new_nspname, sizeof(map->new_nspname), "%s", new_rel->nspname);
+	snprintf(map->old_relname, sizeof(map->old_relname), "%s", old_rel->relname);
+	snprintf(map->new_relname, sizeof(map->new_relname), "%s", new_rel->relname);
 }
 
 
diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h
index 3bbddee824e..d863155b9b2 100644
--- a/contrib/pg_upgrade/pg_upgrade.h
+++ b/contrib/pg_upgrade/pg_upgrade.h
@@ -87,12 +87,18 @@ typedef struct
 {
 	char		old_dir[MAXPGPATH];
 	char		new_dir[MAXPGPATH];
-	Oid			old_relfilenode;	/* Relfilenode of the old relation */
-	Oid			new_relfilenode;	/* Relfilenode of the new relation */
-	char		old_nspname[NAMEDATALEN];		/* old name of the namespace */
-	char		old_relname[NAMEDATALEN];		/* old name of the relation */
-	char		new_nspname[NAMEDATALEN];		/* new name of the namespace */
-	char		new_relname[NAMEDATALEN];		/* new name of the relation */
+	/*
+	 * old/new relfilenodes might differ for pg_largeobject(_metadata) indexes
+	 * due to VACUUM FULL or REINDEX.  Other relfilenodes are preserved.
+	 */
+	Oid			old_relfilenode;
+	Oid			new_relfilenode;
+	/* the rest are used only for logging and error reporting */
+	char		old_nspname[NAMEDATALEN];		/* namespaces */
+	char		new_nspname[NAMEDATALEN];
+	/* old/new relnames differ for toast tables and toast indexes */
+	char		old_relname[NAMEDATALEN];
+	char		new_relname[NAMEDATALEN];
 } FileNameMap;
 
 /*
diff --git a/contrib/pg_upgrade/version_old_8_3.c b/contrib/pg_upgrade/version_old_8_3.c
index 664380b47bc..5b226b218df 100644
--- a/contrib/pg_upgrade/version_old_8_3.c
+++ b/contrib/pg_upgrade/version_old_8_3.c
@@ -222,8 +222,8 @@ old_8_3_rebuild_tsvector_tables(ClusterInfo *cluster, bool check_mode)
 	{
 		PGresult   *res;
 		bool		db_used = false;
-		char		old_nspname[NAMEDATALEN] = "",
-					old_relname[NAMEDATALEN] = "";
+		char		nspname[NAMEDATALEN] = "",
+					relname[NAMEDATALEN] = "";
 		int			ntups;
 		int			rowno;
 		int			i_nspname,
@@ -283,10 +283,10 @@ old_8_3_rebuild_tsvector_tables(ClusterInfo *cluster, bool check_mode)
 				}
 
 				/* Rebuild all tsvector collumns with one ALTER TABLE command */
-				if (strcmp(PQgetvalue(res, rowno, i_nspname), old_nspname) != 0 ||
-				 strcmp(PQgetvalue(res, rowno, i_relname), old_relname) != 0)
+				if (strcmp(PQgetvalue(res, rowno, i_nspname), nspname) != 0 ||
+				 strcmp(PQgetvalue(res, rowno, i_relname), relname) != 0)
 				{
-					if (strlen(old_nspname) != 0 || strlen(old_relname) != 0)
+					if (strlen(nspname) != 0 || strlen(relname) != 0)
 						fprintf(script, ";\n\n");
 					fprintf(script, "ALTER TABLE %s.%s\n",
 						 quote_identifier(PQgetvalue(res, rowno, i_nspname)),
@@ -294,8 +294,8 @@ old_8_3_rebuild_tsvector_tables(ClusterInfo *cluster, bool check_mode)
 				}
 				else
 					fprintf(script, ",\n");
-				strlcpy(old_nspname, PQgetvalue(res, rowno, i_nspname), sizeof(old_nspname));
-				strlcpy(old_relname, PQgetvalue(res, rowno, i_relname), sizeof(old_relname));
+				strlcpy(nspname, PQgetvalue(res, rowno, i_nspname), sizeof(nspname));
+				strlcpy(relname, PQgetvalue(res, rowno, i_relname), sizeof(relname));
 
 				fprintf(script, "ALTER COLUMN %s "
 				/* This could have been a custom conversion function call. */
@@ -304,7 +304,7 @@ old_8_3_rebuild_tsvector_tables(ClusterInfo *cluster, bool check_mode)
 						quote_identifier(PQgetvalue(res, rowno, i_attname)));
 			}
 		}
-		if (strlen(old_nspname) != 0 || strlen(old_relname) != 0)
+		if (strlen(nspname) != 0 || strlen(relname) != 0)
 			fprintf(script, ";\n\n");
 
 		PQclear(res);
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index b0a0dc56072..95218ee8317 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -2354,34 +2354,37 @@ binary_upgrade_set_relfilenodes(PQExpBuffer upgrade_buffer, Oid pg_class_oid,
 					"\n-- For binary upgrade, must preserve relfilenodes\n");
 
 	if (!is_index)
+	{
 		appendPQExpBuffer(upgrade_buffer,
 						  "SELECT binary_upgrade.set_next_heap_relfilenode('%u'::pg_catalog.oid);\n",
 						  pg_class_relfilenode);
+		/* only tables have toast tables, not indexes */
+		if (OidIsValid(pg_class_reltoastrelid))
+		{
+			/*
+			 * One complexity is that the table definition might not require the
+			 * creation of a TOAST table, and the TOAST table might have been
+			 * created long after table creation, when the table was loaded with
+			 * wide data.  By setting the TOAST relfilenode we force creation of
+			 * the TOAST heap and TOAST index by the backend so we can cleanly
+			 * migrate the files during binary migration.
+			 */
+	
+			appendPQExpBuffer(upgrade_buffer,
+							  "SELECT binary_upgrade.set_next_toast_relfilenode('%u'::pg_catalog.oid);\n",
+							  pg_class_reltoastrelid);
+	
+			/* every toast table has an index */
+			appendPQExpBuffer(upgrade_buffer,
+							  "SELECT binary_upgrade.set_next_index_relfilenode('%u'::pg_catalog.oid);\n",
+							  pg_class_reltoastidxid);
+		}
+	}
 	else
 		appendPQExpBuffer(upgrade_buffer,
 						  "SELECT binary_upgrade.set_next_index_relfilenode('%u'::pg_catalog.oid);\n",
 						  pg_class_relfilenode);
 
-	if (OidIsValid(pg_class_reltoastrelid))
-	{
-		/*
-		 * One complexity is that the table definition might not require the
-		 * creation of a TOAST table, and the TOAST table might have been
-		 * created long after table creation, when the table was loaded with
-		 * wide data.  By setting the TOAST relfilenode we force creation of
-		 * the TOAST heap and TOAST index by the backend so we can cleanly
-		 * migrate the files during binary migration.
-		 */
-
-		appendPQExpBuffer(upgrade_buffer,
-						  "SELECT binary_upgrade.set_next_toast_relfilenode('%u'::pg_catalog.oid);\n",
-						  pg_class_reltoastrelid);
-
-		/* every toast table has an index */
-		appendPQExpBuffer(upgrade_buffer,
-						  "SELECT binary_upgrade.set_next_index_relfilenode('%u'::pg_catalog.oid);\n",
-						  pg_class_reltoastidxid);
-	}
 	appendPQExpBuffer(upgrade_buffer, "\n");
 
 	PQclear(upgrade_res);
-- 
GitLab