Teach pg_upgrade to handle in-place tablespaces.

nathan-bossart · Commitfest Bot · commit 4506c74d5043 · 2025-05-15T12:35:23.000Z
Presently, pg_upgrade assumes that all non-default tablespaces
don't move to different directories during upgrade.  Unfortunately,
this isn't true for in-place tablespaces, which move to the new
cluster's pg_tblspc directory.  This commit teaches pg_upgrade to
handle in-place tablespaces by retrieving the tablespace
directories for both the old and new clusters.  In turn, we can
relax the prohibition on non-default tablespaces for same-version
upgrades, i.e., if all non-default tablespaces are in-place,
pg_upgrade may proceed.

This change is primarily intended to enable additional pg_upgrade
testing with non-default tablespaces, as is done in
006_transfer_modes.pl.
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
@@ -946,12 +946,12 @@ check_for_new_tablespace_dir(void)
 
 	prep_status("Checking for new cluster tablespace directories");
 
-	for (tblnum = 0; tblnum < os_info.num_old_tablespaces; tblnum++)
+	for (tblnum = 0; tblnum < new_cluster.num_tablespaces; tblnum++)
 	{
 		struct stat statbuf;
 
 		snprintf(new_tablespace_dir, MAXPGPATH, "%s%s",
-				 os_info.old_tablespaces[tblnum],
+				 new_cluster.tablespaces[tblnum],
 				 new_cluster.tablespace_suffix);
 
 		if (stat(new_tablespace_dir, &statbuf) == 0 || errno != ENOENT)
@@ -1003,17 +1003,17 @@ create_script_for_old_cluster_deletion(char **deletion_script_file_name)
 	 * directory.  We can't create a proper old cluster delete script in that
 	 * case.
 	 */
-	for (tblnum = 0; tblnum < os_info.num_old_tablespaces; tblnum++)
+	for (tblnum = 0; tblnum < new_cluster.num_tablespaces; tblnum++)
 	{
-		char		old_tablespace_dir[MAXPGPATH];
+		char		new_tablespace_dir[MAXPGPATH];
 
-		strlcpy(old_tablespace_dir, os_info.old_tablespaces[tblnum], MAXPGPATH);
-		canonicalize_path(old_tablespace_dir);
-		if (path_is_prefix_of_path(old_cluster_pgdata, old_tablespace_dir))
+		strlcpy(new_tablespace_dir, new_cluster.tablespaces[tblnum], MAXPGPATH);
+		canonicalize_path(new_tablespace_dir);
+		if (path_is_prefix_of_path(old_cluster_pgdata, new_tablespace_dir))
 		{
 			/* reproduce warning from CREATE TABLESPACE that is in the log */
 			pg_log(PG_WARNING,
-				   "\nWARNING:  user-defined tablespace locations should not be inside the data directory, i.e. %s", old_tablespace_dir);
+				   "\nWARNING:  user-defined tablespace locations should not be inside the data directory, i.e. %s", new_tablespace_dir);
 
 			/* Unlink file in case it is left over from a previous run. */
 			unlink(*deletion_script_file_name);
@@ -1041,9 +1041,9 @@ create_script_for_old_cluster_deletion(char **deletion_script_file_name)
 	/* delete old cluster's alternate tablespaces */
 	old_tblspc_suffix = pg_strdup(old_cluster.tablespace_suffix);
 	fix_path_separator(old_tblspc_suffix);
-	for (tblnum = 0; tblnum < os_info.num_old_tablespaces; tblnum++)
+	for (tblnum = 0; tblnum < old_cluster.num_tablespaces; tblnum++)
 		fprintf(script, RMDIR_CMD " %c%s%s%c\n", PATH_QUOTE,
-				fix_path_separator(os_info.old_tablespaces[tblnum]),
+				fix_path_separator(old_cluster.tablespaces[tblnum]),
 				old_tblspc_suffix, PATH_QUOTE);
 	pfree(old_tblspc_suffix);
 
diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c
@@ -443,10 +443,24 @@ get_db_infos(ClusterInfo *cluster)
 
 	for (tupnum = 0; tupnum < ntups; tupnum++)
 	{
+		char	   *spcloc = PQgetvalue(res, tupnum, i_spclocation);
+		bool		inplace = spcloc[0] && !is_absolute_path(spcloc);
+
 		dbinfos[tupnum].db_oid = atooid(PQgetvalue(res, tupnum, i_oid));
 		dbinfos[tupnum].db_name = pg_strdup(PQgetvalue(res, tupnum, i_datname));
-		snprintf(dbinfos[tupnum].db_tablespace, sizeof(dbinfos[tupnum].db_tablespace), "%s",
-				 PQgetvalue(res, tupnum, i_spclocation));
+
+		/*
+		 * For in-place tablespaces, pg_tablespace_location() returns a path
+		 * relative to the data directory.
+		 */
+		if (inplace)
+			snprintf(dbinfos[tupnum].db_tablespace,
+					 sizeof(dbinfos[tupnum].db_tablespace),
+					 "%s/%s", cluster->pgdata, spcloc);
+		else
+			snprintf(dbinfos[tupnum].db_tablespace,
+					 sizeof(dbinfos[tupnum].db_tablespace),
+					 "%s", spcloc);
 	}
 	PQclear(res);
 
@@ -616,11 +630,21 @@ process_rel_infos(DbInfo *dbinfo, PGresult *res, void *arg)
 		/* Is the tablespace oid non-default? */
 		if (atooid(PQgetvalue(res, relnum, i_reltablespace)) != 0)
 		{
+			char	   *spcloc = PQgetvalue(res, relnum, i_spclocation);
+			bool		inplace = spcloc[0] && !is_absolute_path(spcloc);
+
 			/*
 			 * The tablespace location might be "", meaning the cluster
-			 * default location, i.e. pg_default or pg_global.
+			 * default location, i.e. pg_default or pg_global.  For in-place
+			 * tablespaces, pg_tablespace_location() returns a path relative
+			 * to the data directory.
 			 */
-			tablespace = PQgetvalue(res, relnum, i_spclocation);
+			if (inplace)
+				tablespace = psprintf("%s/%s",
+									  os_info.running_cluster->pgdata,
+									  PQgetvalue(res, relnum, i_spclocation));
+			else
+				tablespace = PQgetvalue(res, relnum, i_spclocation);
 
 			/* Can we reuse the previous string allocation? */
 			if (last_tablespace && strcmp(tablespace, last_tablespace) == 0)
@@ -630,6 +654,10 @@ process_rel_infos(DbInfo *dbinfo, PGresult *res, void *arg)
 				last_tablespace = curr->tablespace = pg_strdup(tablespace);
 				curr->tblsp_alloc = true;
 			}
+
+			/* Free palloc'd string for in-place tablespaces. */
+			if (inplace)
+				pfree(tablespace);
 		}
 		else
 			/* A zero reltablespace oid indicates the database tablespace. */
diff --git a/src/bin/pg_upgrade/parallel.c b/src/bin/pg_upgrade/parallel.c
@@ -40,6 +40,7 @@ typedef struct
 	char	   *old_pgdata;
 	char	   *new_pgdata;
 	char	   *old_tablespace;
+	char	   *new_tablespace;
 } transfer_thread_arg;
 
 static exec_thread_arg **exec_thread_args;
@@ -171,7 +172,7 @@ win32_exec_prog(exec_thread_arg *args)
 void
 parallel_transfer_all_new_dbs(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr,
 							  char *old_pgdata, char *new_pgdata,
-							  char *old_tablespace)
+							  char *old_tablespace, char *new_tablespace)
 {
 #ifndef WIN32
 	pid_t		child;
@@ -181,7 +182,7 @@ parallel_transfer_all_new_dbs(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr,
 #endif
 
 	if (user_opts.jobs <= 1)
-		transfer_all_new_dbs(old_db_arr, new_db_arr, old_pgdata, new_pgdata, NULL);
+		transfer_all_new_dbs(old_db_arr, new_db_arr, old_pgdata, new_pgdata, NULL, NULL);
 	else
 	{
 		/* parallel */
@@ -225,7 +226,7 @@ parallel_transfer_all_new_dbs(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr,
 		if (child == 0)
 		{
 			transfer_all_new_dbs(old_db_arr, new_db_arr, old_pgdata, new_pgdata,
-								 old_tablespace);
+								 old_tablespace, new_tablespace);
 			/* if we take another exit path, it will be non-zero */
 			/* use _exit to skip atexit() functions */
 			_exit(0);
@@ -246,6 +247,7 @@ parallel_transfer_all_new_dbs(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr,
 		new_arg->new_pgdata = pg_strdup(new_pgdata);
 		pg_free(new_arg->old_tablespace);
 		new_arg->old_tablespace = old_tablespace ? pg_strdup(old_tablespace) : NULL;
+		new_arg->new_tablespace = new_tablespace ? pg_strdup(new_tablespace) : NULL;
 
 		child = (HANDLE) _beginthreadex(NULL, 0, (void *) win32_transfer_all_new_dbs,
 										new_arg, 0, NULL);
@@ -263,7 +265,8 @@ DWORD
 win32_transfer_all_new_dbs(transfer_thread_arg *args)
 {
 	transfer_all_new_dbs(args->old_db_arr, args->new_db_arr, args->old_pgdata,
-						 args->new_pgdata, args->old_tablespace);
+						 args->new_pgdata, args->old_tablespace,
+						 args->new_tablespace);
 
 	/* terminates thread */
 	return 0;
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
@@ -300,6 +300,8 @@ typedef struct
 	uint32		major_version;	/* PG_VERSION of cluster */
 	char		major_version_str[64];	/* string PG_VERSION of cluster */
 	uint32		bin_version;	/* version returned from pg_ctl */
+	char	  **tablespaces;	/* tablespace directories */
+	int			num_tablespaces;
 	const char *tablespace_suffix;	/* directory specification */
 	int			nsubs;			/* number of subscriptions */
 } ClusterInfo;
@@ -354,8 +356,6 @@ typedef struct
 	const char *progname;		/* complete pathname for this program */
 	char	   *user;			/* username for clusters */
 	bool		user_specified; /* user specified on command-line */
-	char	  **old_tablespaces;	/* tablespaces */
-	int			num_old_tablespaces;
 	LibraryInfo *libraries;		/* loadable libraries */
 	int			num_libraries;
 	ClusterInfo *running_cluster;
@@ -455,7 +455,7 @@ void		transfer_all_new_tablespaces(DbInfoArr *old_db_arr,
 										 DbInfoArr *new_db_arr, char *old_pgdata, char *new_pgdata);
 void		transfer_all_new_dbs(DbInfoArr *old_db_arr,
 								 DbInfoArr *new_db_arr, char *old_pgdata, char *new_pgdata,
-								 char *old_tablespace);
+								 char *old_tablespace, char *new_tablespace);
 
 /* tablespace.c */
 
@@ -503,7 +503,7 @@ void		parallel_exec_prog(const char *log_file, const char *opt_log_file,
 							   const char *fmt,...) pg_attribute_printf(3, 4);
 void		parallel_transfer_all_new_dbs(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr,
 										  char *old_pgdata, char *new_pgdata,
-										  char *old_tablespace);
+										  char *old_tablespace, char *new_tablespace);
 bool		reap_child(bool wait_for_child);
 
 /* task.c */
diff --git a/src/bin/pg_upgrade/relfilenumber.c b/src/bin/pg_upgrade/relfilenumber.c
@@ -17,7 +17,7 @@
 #include "common/logging.h"
 #include "pg_upgrade.h"
 
-static void transfer_single_new_db(FileNameMap *maps, int size, char *old_tablespace);
+static void transfer_single_new_db(FileNameMap *maps, int size, char *old_tablespace, char *new_tablespace);
 static void transfer_relfile(FileNameMap *map, const char *type_suffix, bool vm_must_add_frozenbit);
 
 /*
@@ -136,21 +136,22 @@ transfer_all_new_tablespaces(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr,
 	 */
 	if (user_opts.jobs <= 1)
 		parallel_transfer_all_new_dbs(old_db_arr, new_db_arr, old_pgdata,
-									  new_pgdata, NULL);
+									  new_pgdata, NULL, NULL);
 	else
 	{
 		int			tblnum;
 
 		/* transfer default tablespace */
 		parallel_transfer_all_new_dbs(old_db_arr, new_db_arr, old_pgdata,
-									  new_pgdata, old_pgdata);
+									  new_pgdata, old_pgdata, new_pgdata);
 
-		for (tblnum = 0; tblnum < os_info.num_old_tablespaces; tblnum++)
+		for (tblnum = 0; tblnum < old_cluster.num_tablespaces; tblnum++)
 			parallel_transfer_all_new_dbs(old_db_arr,
 										  new_db_arr,
 										  old_pgdata,
 										  new_pgdata,
-										  os_info.old_tablespaces[tblnum]);
+										  old_cluster.tablespaces[tblnum],
+										  new_cluster.tablespaces[tblnum]);
 		/* reap all children */
 		while (reap_child(true) == true)
 			;
@@ -169,7 +170,8 @@ transfer_all_new_tablespaces(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr,
  */
 void
 transfer_all_new_dbs(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr,
-					 char *old_pgdata, char *new_pgdata, char *old_tablespace)
+					 char *old_pgdata, char *new_pgdata,
+					 char *old_tablespace, char *new_tablespace)
 {
 	int			old_dbnum,
 				new_dbnum;
@@ -204,7 +206,7 @@ transfer_all_new_dbs(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr,
 									new_pgdata);
 		if (n_maps)
 		{
-			transfer_single_new_db(mappings, n_maps, old_tablespace);
+			transfer_single_new_db(mappings, n_maps, old_tablespace, new_tablespace);
 		}
 		/* We allocate something even for n_maps == 0 */
 		pg_free(mappings);
@@ -234,10 +236,10 @@ transfer_all_new_dbs(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr,
  *	moved_db_dir: Destination for the pg_restore-generated database directory.
  */
 static bool
-prepare_for_swap(const char *old_tablespace, Oid db_oid,
-				 char *old_catalog_dir, char *new_db_dir, char *moved_db_dir)
+prepare_for_swap(const char *old_tablespace, const char *new_tablespace,
+				 Oid db_oid, char *old_catalog_dir, char *new_db_dir,
+				 char *moved_db_dir)
 {
-	const char *new_tablespace;
 	const char *old_tblspc_suffix;
 	const char *new_tblspc_suffix;
 	char		old_tblspc[MAXPGPATH];
@@ -247,24 +249,14 @@ prepare_for_swap(const char *old_tablespace, Oid db_oid,
 	struct stat st;
 
 	if (strcmp(old_tablespace, old_cluster.pgdata) == 0)
-	{
-		new_tablespace = new_cluster.pgdata;
-		new_tblspc_suffix = "/base";
 		old_tblspc_suffix = "/base";
-	}
 	else
-	{
-		/*
-		 * XXX: The below line is a hack to deal with the fact that we
-		 * presently don't have an easy way to find the corresponding new
-		 * tablespace's path.  This will need to be fixed if/when we add
-		 * pg_upgrade support for in-place tablespaces.
-		 */
-		new_tablespace = old_tablespace;
+		old_tblspc_suffix = old_cluster.tablespace_suffix;
 
+	if (strcmp(new_tablespace, new_cluster.pgdata) == 0)
+		new_tblspc_suffix = "/base";
+	else
 		new_tblspc_suffix = new_cluster.tablespace_suffix;
-		old_tblspc_suffix = old_cluster.tablespace_suffix;
-	}
 
 	/* Old and new cluster paths. */
 	snprintf(old_tblspc, sizeof(old_tblspc), "%s%s", old_tablespace, old_tblspc_suffix);
@@ -450,7 +442,7 @@ swap_catalog_files(FileNameMap *maps, int size, const char *old_catalog_dir,
  * during pg_restore.
  */
 static void
-do_swap(FileNameMap *maps, int size, char *old_tablespace)
+do_swap(FileNameMap *maps, int size, char *old_tablespace, char *new_tablespace)
 {
 	char		old_catalog_dir[MAXPGPATH];
 	char		new_db_dir[MAXPGPATH];
@@ -470,21 +462,23 @@ do_swap(FileNameMap *maps, int size, char *old_tablespace)
 	 */
 	if (old_tablespace)
 	{
-		if (prepare_for_swap(old_tablespace, maps[0].db_oid,
+		if (prepare_for_swap(old_tablespace, new_tablespace, maps[0].db_oid,
 							 old_catalog_dir, new_db_dir, moved_db_dir))
 			swap_catalog_files(maps, size,
 							   old_catalog_dir, new_db_dir, moved_db_dir);
 	}
 	else
 	{
-		if (prepare_for_swap(old_cluster.pgdata, maps[0].db_oid,
+		if (prepare_for_swap(old_cluster.pgdata, new_cluster.pgdata, maps[0].db_oid,
 							 old_catalog_dir, new_db_dir, moved_db_dir))
 			swap_catalog_files(maps, size,
 							   old_catalog_dir, new_db_dir, moved_db_dir);
 
-		for (int tblnum = 0; tblnum < os_info.num_old_tablespaces; tblnum++)
+		for (int tblnum = 0; tblnum < old_cluster.num_tablespaces; tblnum++)
 		{
-			if (prepare_for_swap(os_info.old_tablespaces[tblnum], maps[0].db_oid,
+			if (prepare_for_swap(old_cluster.tablespaces[tblnum],
+								 new_cluster.tablespaces[tblnum],
+								 maps[0].db_oid,
 								 old_catalog_dir, new_db_dir, moved_db_dir))
 				swap_catalog_files(maps, size,
 								   old_catalog_dir, new_db_dir, moved_db_dir);
@@ -498,7 +492,8 @@ do_swap(FileNameMap *maps, int size, char *old_tablespace)
  * create links for mappings stored in "maps" array.
  */
 static void
-transfer_single_new_db(FileNameMap *maps, int size, char *old_tablespace)
+transfer_single_new_db(FileNameMap *maps, int size,
+					   char *old_tablespace, char *new_tablespace)
 {
 	int			mapnum;
 	bool		vm_must_add_frozenbit = false;
@@ -520,7 +515,7 @@ transfer_single_new_db(FileNameMap *maps, int size, char *old_tablespace)
 		 */
 		Assert(!vm_must_add_frozenbit);
 
-		do_swap(maps, size, old_tablespace);
+		do_swap(maps, size, old_tablespace, new_tablespace);
 		return;
 	}
 
diff --git a/src/bin/pg_upgrade/t/006_transfer_modes.pl b/src/bin/pg_upgrade/t/006_transfer_modes.pl
diff --git a/src/bin/pg_upgrade/tablespace.c b/src/bin/pg_upgrade/tablespace.c
