Re: teach pg_upgrade to handle in-place tablespaces

On Mon, Jul 21, 2025 at 07:57:32PM -0500, Nathan Bossart wrote:
+ if (!defined($ENV{oldinstall}))
+ {
+ $new->append_conf('postgresql.conf',
+ "allow_in_place_tablespaces = true");
+ $old->append_conf('postgresql.conf',
+ "allow_in_place_tablespaces = true");
+ }

This would not choke as long as the old cluster is at least at v10,
but well why not.

- # For cross-version tests, we can also check that pg_upgrade handles
- # tablespaces.
+ my $tblspc = '';
if (defined($ENV{oldinstall}))
{
- my $tblspc = PostgreSQL::Test::Utils::tempdir_short();
- $old->safe_psql('postgres',
- "CREATE TABLESPACE test_tblspc LOCATION '$tblspc'");
- $old->safe_psql('postgres',
- "CREATE DATABASE testdb2 TABLESPACE test_tblspc");
- $old->safe_psql('postgres',
- "CREATE TABLE test3 TABLESPACE test_tblspc AS SELECT generate_series(300, 401)"
- );
- $old->safe_psql('testdb2',
- "CREATE TABLE test4 AS SELECT generate_series(400, 502)");
+ $tblspc = PostgreSQL::Test::Utils::tempdir_short();
}
+ $old->safe_psql('postgres',
+ "CREATE TABLESPACE test_tblspc LOCATION '$tblspc'");
+ $old->safe_psql('postgres',
+ "CREATE DATABASE testdb2 TABLESPACE test_tblspc");
+ $old->safe_psql('postgres',
+ "CREATE TABLE test3 TABLESPACE test_tblspc AS SELECT generate_series(300, 401)"
+ );
+ $old->safe_psql('testdb2',
+ "CREATE TABLE test4 AS SELECT generate_series(400, 502)");
$old->stop;

I would vote for having a total of two tablespaces when we can do so
safely: one non-inplace and one inplace, strengthening the
cross-checks for the prefixes assigned in the new paths when doing a
swap of the tablespace paths, even when an old installation is tested.

+ /*
+ * For now, we do not expect non-in-place tablespaces to move during
+ * upgrade. If that changes, it will likely become necessary to run
+ * the above query on the new cluster, too.
+ */

Curiosity matter: has this ever been discussed? Spoiler: I don't
recall so and why it would ever matter as tbspace OIDs should be fixed
across upgrades these days (right?).

@@ -53,19 +63,44 @@ get_tablespace_paths(void)
[...]
+ if (is_absolute_path(PQgetvalue(res, tblnum, i_spclocation)))
+ {

This use of is_absolute_path() should document that it is for in-place
tablespaces?

+ if (strcmp(new_tablespace, new_cluster.pgdata) == 0)
+ new_tblspc_suffix = "/base";

This knowledge is encapsulated in GetDatabasePath() currently. Not
new, just saying that it could be nice to reduce the footprint of this
knowledge in pg_upgrade. Perhaps not something to worry about here,
though.
--
Michael