Upgrade filespaces with multiple tablespaces

- dump tablespace with location including tablespace oid
- restore creates GPDB tablespace layout in the tablespace oid dir
- modify the old tablespace mapping file to include the tablespace oid
- modify test library to update symlinks to respective tablespace oids
- general test cleanup
- preserve tablespace oids during upgrade which helps copy from master

contrib/pg_upgrade/function.c

@@ -103,6 +103,12 @@ install_support_functions_in_new_db(const char *db_name)
 							  "RETURNS VOID "
 							  "AS '$libdir/pg_upgrade_support' "
 							  "LANGUAGE C STRICT;"));
+	PQclear(executeQueryOrDie(conn,
+	                          "CREATE OR REPLACE FUNCTION "
+	                          "binary_upgrade.set_next_preassigned_tablespace_oid(OID, TEXT) "
+	                          "RETURNS VOID "
+	                          "AS '$libdir/pg_upgrade_support' "
+	                          "LANGUAGE C STRICT;"));
 
 	PQfinish(conn);
 }

contrib/pg_upgrade/greenplum/info_gp.c

@@ -37,13 +37,11 @@ system_tablespace(void)
 }
 
 static GetTablespacePathResponse
-found_in_file(char *tablespace_path, Oid tablespace_oid)
+found_in_file(char *tablespace_path)
 {
 	return make_response(
 		GetTablespacePathResponse_FOUND_USER_DEFINED_TABLESPACE,
-		psprintf("%s/%u",
-			tablespace_path,
-			tablespace_oid));
+		tablespace_path);
 }
 
 GetTablespacePathResponse
@@ -60,8 +58,7 @@ gp_get_tablespace_path(OldTablespaceFileContents *oldTablespaceFileContents, Oid
 		return system_tablespace();
 
 	return found_in_file(
-		OldTablespaceRecord_GetDirectoryPath(record),
-		tablespace_oid);
+		OldTablespaceRecord_GetDirectoryPath(record));
 }
 
 /*

contrib/pg_upgrade/greenplum/old_tablespace_file_contents.c

@@ -221,3 +221,9 @@ OldTablespaceRecord_GetIsUserDefinedTablespace(OldTablespaceRecord *record)
 {
 	return record->is_user_defined;
 }
+
+Oid
+OldTablespaceRecord_GetOid(OldTablespaceRecord *record)
+{
+	return record->tablespace_oid;
+}

contrib/pg_upgrade/greenplum/old_tablespace_file_contents.h

@@ -57,6 +57,9 @@ OldTablespaceRecord_GetTablespaceName(OldTablespaceRecord *record);
 char *
 OldTablespaceRecord_GetDirectoryPath(OldTablespaceRecord *record);
 
+Oid
+OldTablespaceRecord_GetOid(OldTablespaceRecord *record);
+
 bool
 OldTablespaceRecord_GetIsUserDefinedTablespace(OldTablespaceRecord *record);
 

contrib/pg_upgrade/greenplum/tablespace_gp.c

@@ -30,12 +30,20 @@ get_generated_old_tablespaces_file_path(void)
 	return psprintf("%s/%s", current_working_directory, OLD_TABLESPACES_FILE);
 }
 
-static char *const OLD_TABLESPACE_QUERY = ""
-"copy ("
-"    select fsedbid, pg_tablespace.oid as tablespace_oid, spcname, fselocation, (spcname not in ('pg_default', 'pg_global')::int) as is_user_defined_tablespace"
-"    from pg_filespace_entry "
-"    inner join pg_tablespace on fsefsoid = spcfsoid "
-") to '%s' WITH CSV";
+static char *const OLD_TABLESPACE_QUERY = "copy ( "
+                                          "    select fsedbid, "
+                                          "    upgrade_tablespace.oid as tablespace_oid, "
+                                          "    spcname, "
+                                          "    case when is_user_defined_tablespace then location_with_oid else fselocation end, "
+                                          "    (is_user_defined_tablespace::int) as is_user_defined_tablespace "
+                                          "    from ( "
+                                          "        select pg_tablespace.oid, *, "
+                                          "        (fselocation || '/' || pg_tablespace.oid) as location_with_oid, "
+                                          "        (spcname not in ('pg_default', 'pg_global'))  as is_user_defined_tablespace "
+                                          "        from pg_tablespace "
+                                          "        inner join pg_filespace_entry on fsefsoid = spcfsoid "
+                                          "    ) upgrade_tablespace "
+                                          ") to '%s' WITH CSV;";
 
 static void
 dump_old_tablespaces(ClusterInfo *oldCluster,

contrib/pg_upgrade/test/integration/scenarios/filespaces_to_tablespaces.c

@@ -17,6 +17,22 @@
 
 #include "filespaces_to_tablespaces.h"
 
+static char * original_tablespace_oid = NULL;
+
+static char *
+get_tablespace_oid(PGconn *connection, char *tablespace_name)
+{
+	PGresult *response = executeQuery(
+		connection,
+		psprintf("select oid from pg_tablespace where spcname = '%s';", tablespace_name));
+
+	char *result = pg_strdup(PQgetvalue(response, 0, 0));
+
+	PQclear(response);
+
+	return result;
+}
+
 static void
 aFilespaceExistsInTheFiveClusterWithATableAndData(void)
 {
@@ -51,14 +67,23 @@ aFilespaceExistsInTheFiveClusterWithATableAndData(void)
 	                                               "8: '/tmp/gpdb-filespaces/fsdummy4/' );");
 	PQclear(result5);
 
-	result5 = executeQuery(connection5, "CREATE TABLESPACE some_tablespace FILESPACE some_filespace;");
+	/*
+	 * Populate a tablespace within the filespace
+	 */
+	PQclear(executeQuery(connection5, "CREATE TABLESPACE some_tablespace FILESPACE some_filespace;"));
 	PQclear(executeQuery(connection5, "CREATE SCHEMA five_to_six_upgrade;"));
 	PQclear(executeQuery(connection5, "set search_path to five_to_six_upgrade;"));
-	result5 = executeQuery(connection5, "CREATE TABLE users (id integer, name text) TABLESPACE some_tablespace;");
-	result5 = executeQuery(connection5, "insert into users VALUES (1, 'Joe');");
-	result5 = executeQuery(connection5, "insert into users VALUES (2, 'Janet');");
-	result5 = executeQuery(connection5, "insert into users VALUES (3, 'James');");
-	PQclear(result5);
+	PQclear(executeQuery(connection5, "CREATE TABLE users (id integer, name text) TABLESPACE some_tablespace;"));
+	PQclear(executeQuery(connection5, "insert into users VALUES (1, 'Joe');"));
+	PQclear(executeQuery(connection5, "insert into users VALUES (2, 'Janet');"));
+	PQclear(executeQuery(connection5, "insert into users VALUES (3, 'James');"));
+
+	original_tablespace_oid = get_tablespace_oid(connection5, "some_tablespace");
+
+	/*
+	 * Create another tablespace within the same filespace
+	 */
+	PQclear(executeQuery(connection5, "CREATE TABLESPACE some_other_tablespace FILESPACE some_filespace;"));
 
 	PQfinish(connection5);
 }
@@ -75,7 +100,6 @@ aDatabaseInAFilespaceExistsInTheFiveClusterWithATableAndData(void)
 	system("mkdir /tmp/gpdb-filespaces");
 
 	PGconn *connection5 = connectToFive();
-	PGresult *result5;
 
 	/*
 	* Create filespace and tablespace within the filespace.
@@ -86,7 +110,7 @@ aDatabaseInAFilespaceExistsInTheFiveClusterWithATableAndData(void)
 	* map. Thus, we supply dummy directories here just to make the syntax
 	* check happy.
 	*/
-	result5 = executeQuery(connection5, "CREATE FILESPACE some_filespace ( \n"
+	PQclear(executeQuery(connection5, "CREATE FILESPACE some_filespace ( \n"
 	                                               "1: '/tmp/gpdb-filespaces/fsseg-1/', \n"
 	                                               "2: '/tmp/gpdb-filespaces/fsseg0/', \n"
 	                                               "3: '/tmp/gpdb-filespaces/fsseg1/', \n"
@@ -94,11 +118,10 @@ aDatabaseInAFilespaceExistsInTheFiveClusterWithATableAndData(void)
 	                                               "5: '/tmp/gpdb-filespaces/fsdummy1/', \n"
 	                                               "6: '/tmp/gpdb-filespaces/fsdummy2/', \n"
 	                                               "7: '/tmp/gpdb-filespaces/fsdummy3/', \n"
-	                                               "8: '/tmp/gpdb-filespaces/fsdummy4/' );");
-	PQclear(result5);
+	                                               "8: '/tmp/gpdb-filespaces/fsdummy4/' );"));
 
-	result5 = executeQuery(connection5, "CREATE TABLESPACE some_tablespace FILESPACE some_filespace;");
-	executeQuery(connection5, "CREATE DATABASE database_in_filespace TABLESPACE some_tablespace;");
+	PQclear(executeQuery(connection5, "CREATE TABLESPACE some_tablespace FILESPACE some_filespace;"));
+	PQclear(executeQuery(connection5, "CREATE DATABASE database_in_filespace TABLESPACE some_tablespace;"));
 	PQfinish(connection5);
 
 	connection5 = connectToFiveOnDatabase("database_in_filespace"); 
@@ -266,10 +289,10 @@ expectTablespaceDirectoryToExist(char *directory_path)
 static void
 theTablespacesInTheNewClusterShouldBeCreatedInTheSameLocationAsTheOldClustersTablespaces(void)
 {
-	expectTablespaceDirectoryToExist("/tmp/gpdb-filespaces/fsseg-1/1");
-	expectTablespaceDirectoryToExist("/tmp/gpdb-filespaces/fsseg0/2");
-	expectTablespaceDirectoryToExist("/tmp/gpdb-filespaces/fsseg1/3");
-	expectTablespaceDirectoryToExist("/tmp/gpdb-filespaces/fsseg2/4");
+	expectTablespaceDirectoryToExist(psprintf("/tmp/gpdb-filespaces/fsseg-1/%s/1", original_tablespace_oid));
+	expectTablespaceDirectoryToExist(psprintf("/tmp/gpdb-filespaces/fsseg0/%s/2", original_tablespace_oid));
+	expectTablespaceDirectoryToExist(psprintf("/tmp/gpdb-filespaces/fsseg1/%s/3", original_tablespace_oid));
+	expectTablespaceDirectoryToExist(psprintf("/tmp/gpdb-filespaces/fsseg2/%s/4", original_tablespace_oid));
 }
 
 void

contrib/pg_upgrade/test/integration/scripts/pg-upgrade-copy-from-master.c

@@ -6,7 +6,7 @@
 
 #include "postgres_fe.h"
 #include "utilities/pg-upgrade-copy.h"
-#include "old_tablespace_file_parser_observer.h"
+#include "greenplum/old_tablespace_file_parser_observer.h"
 
 static void
 print_usage_header(char *message)

contrib/pg_upgrade/test/integration/tablespace_gp_test.c

@@ -27,6 +27,22 @@ ClusterInfo new_cluster;
 OSInfo os_info;
 UserOpts user_opts;
 
+static char * original_tablespace_oid = NULL;
+
+static char *
+get_tablespace_oid(PGconn *connection, char *tablespace_name)
+{
+	PGresult *response = executeQuery(
+		connection,
+		psprintf("select oid from pg_tablespace where spcname = '%s';", tablespace_name));
+
+	char *result = pg_strdup(PQgetvalue(response, 0, 0));
+
+	PQclear(response);
+
+	return result;
+}
+
 void 
 OldTablespaceFileParser_invalid_access_error_for_field(int row_index, int field_index)
 {
@@ -79,7 +95,7 @@ test_filespaces_on_a_gpdb_five_cluster_are_loaded_as_old_tablespace_file_content
 	PQclear(result5);
 
 	PQclear(executeQuery(connection, "CREATE TABLESPACE my_fast_tablespace FILESPACE my_fast_locations;"));
-
+	original_tablespace_oid = get_tablespace_oid(connection, "my_fast_tablespace");
 	PQfinish(connection);
 	
 	ClusterInfo cluster;
@@ -103,7 +119,7 @@ test_filespaces_on_a_gpdb_five_cluster_are_loaded_as_old_tablespace_file_content
 
 	assert_string_equal(
 		results[2],
-		"/tmp/tablespace-gp-test/fsseg0");
+		psprintf("/tmp/tablespace-gp-test/fsseg0/%s", original_tablespace_oid));
 
 	OldTablespaceRecord **records = OldTablespaceFileContents_GetTablespaceRecords(
 		get_old_tablespace_file_contents()

contrib/pg_upgrade/test/integration/utilities/pg-upgrade-copy.c

@@ -93,10 +93,11 @@ backup_configuration_files(PgUpgradeCopyOptions *options)
 }
 
 static void
-update_symlinks_for_tablespaces_from(char *segment_path, char *new_tablespace_path)
+update_symlinks_for_tablespaces_from(char *segment_path, Oid tablespace_oid, char *new_tablespace_path)
 {
-	system(psprintf("find %s/pg_tblspc/* | xargs -I '{}' ln -sfn %s '{}'",
+	system(psprintf("find %s/pg_tblspc/%u | xargs -I '{}' ln -sfn %s '{}'",
 	                segment_path,
+	                tablespace_oid,
 	                new_tablespace_path));
 }
 
@@ -137,6 +138,7 @@ copy_tablespaces_from_the_master(PgUpgradeCopyOptions *copy_options)
 
 		update_symlinks_for_tablespaces_from(
 			copy_options->new_segment_path,
+			OldTablespaceRecord_GetOid(current_segment_record),
 			segment_tablespace_location_directory_with_gp_dbid);
 	}
 }

contrib/pg_upgrade/test/unit/info_gp_test.c

@@ -108,7 +108,7 @@ test_it_returns_tablespace_path_when_tablespace_found_by_oid(void **state)
 	GetTablespacePathResponse response = gp_get_tablespace_path(make_fake_old_tablespace_file_contents(), tablespace_oid);
 
 	assert_int_equal(response.code, GetTablespacePathResponse_FOUND_USER_DEFINED_TABLESPACE);
-	assert_string_equal(response.tablespace_path, "some_path_to_tablespace/1234");
+	assert_string_equal(response.tablespace_path, "some_path_to_tablespace");
 }
 
 static void

contrib/pg_upgrade_support/pg_upgrade_support.c

@@ -19,6 +19,7 @@
 #include "catalog/pg_class.h"
 #include "catalog/pg_enum.h"
 #include "catalog/pg_namespace.h"
+#include "catalog/pg_tablespace.h"
 #include "catalog/pg_type.h"
 #include "cdb/cdbvars.h"
 #include "commands/extension.h"
@@ -54,6 +55,7 @@ PG_FUNCTION_INFO_V1(create_empty_extension);
 PG_FUNCTION_INFO_V1(set_next_pg_namespace_oid);
 
 PG_FUNCTION_INFO_V1(set_preassigned_oids);
+PG_FUNCTION_INFO_V1(set_next_preassigned_tablespace_oid);
 
 Datum
 set_next_pg_type_oid(PG_FUNCTION_ARGS)
@@ -254,3 +256,19 @@ set_preassigned_oids(PG_FUNCTION_ARGS)
 
 	PG_RETURN_VOID();
 }
+
+Datum
+set_next_preassigned_tablespace_oid(PG_FUNCTION_ARGS)
+{
+	Oid			tsoid = PG_GETARG_OID(0);
+	char	   *objname = GET_STR(PG_GETARG_TEXT_P(1));
+
+	if (Gp_role == GP_ROLE_UTILITY)
+	{
+		AddPreassignedOidFromBinaryUpgrade(tsoid, TableSpaceRelationId, objname,
+		                                   InvalidOid, InvalidOid, InvalidOid);
+	}
+
+	PG_RETURN_VOID();
+}
+

src/bin/pg_dump/pg_dumpall.c

@@ -1454,6 +1454,15 @@ dropTablespaces(PGconn *conn)
 	fprintf(OPF, "\n\n");
 }
 
+static void
+append_preassign_tablespace_oid(PQExpBuffer buffer, Oid tablespace_oid, char *tablespace_name)
+{
+	appendPQExpBuffer(buffer,
+	                  "select binary_upgrade.set_next_preassigned_tablespace_oid(%u, '%s');\n",
+	                  tablespace_oid,
+	                  tablespace_name);
+}
+
 /*
  * Dump tablespaces.
  */
@@ -1493,7 +1502,7 @@ dumpTablespaces(PGconn *conn)
 				       "join (SELECT ts.oid                                  AS oid, "
 				                    "ts.spcname                              AS spcname, "
 				                    "pg_catalog.Pg_get_userbyid(ts.spcowner) AS spcowner, "
-				                    "fs.fselocation                          AS location, "
+									"fs.fselocation || '/' || ts.oid         AS location, "
 				                    "ts.spcacl                               AS spcacl, "
 				                    "NULL                                    AS spcoptions, "
 				                    "pg_catalog.Shobj_description(ts.oid, 'pg_tablespace'), "
@@ -1552,6 +1561,8 @@ dumpTablespaces(PGconn *conn)
 		char	   *spccomment = PQgetvalue(res, i, 6);
 		char	   *fspcname;
 
+		append_preassign_tablespace_oid(buf, spcoid, spcname);
+
 		/* needed for buildACLCommands() */
 		fspcname = pg_strdup(fmtId(spcname));