Re: teach pg_upgrade to handle in-place tablespaces

[Michael Paquier]

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

signature.asc
Description: PGP signature