Re: pg_upgrade fails with in-place tablespace

On Tue, Aug 08, 2023 at 04:32:45PM +0900, Michael Paquier wrote:
> The pg_dump part should be OK to allow the restore of in-place
> tablespaces, so I'll get that fixed first, but I think that we should
> add a regression test as well.

I have added a test for that in 002_pg_dump.pl, and applied the fix
for pg_dumpall.

> I'll try to look at the binary upgrade path and pg_upgrade after this
> is done.

+ /* No need to check in-place tablespace. */
snprintf(query, sizeof(query),
"SELECT pg_catalog.pg_tablespace_location(oid) AS spclocation "
"FROM pg_catalog.pg_tablespace "
"WHERE spcname != 'pg_default' AND "
- " spcname != 'pg_global'");
+ " spcname != 'pg_global' AND "
+ " pg_catalog.pg_tablespace_location(oid) != 'pg_tblspc/' || oid");

This does not really explain the reason why in-place tablespaces need
to be skipped (in short they don't need a separate creation or check
like the others in create_script_for_old_cluster_deletion because they
are part of the data folder). Anyway, the more I think about it, the
less excited I get about the need to support pg_upgrade with in-place
tablespaces, especially regarding the fact that the patch blindly
enforces allows_in_place_tablespaces, assuming that it is OK to do so.
So what about the case where one would want to be warned if these are
still laying around when doing upgrades? And what's the actual use
case for supporting that? There is something else that we could do
here: add a pre-run check to make pg_upgrade fail gracefully if we
find in-place tablespaces in the old cluster.

+# Test with in-place tablespace.
+$oldnode->append_conf('postgresql.conf', 'allow_in_place_tablespaces = on');
+$oldnode->reload;
+$oldnode->safe_psql('postgres', "CREATE TABLESPACE space_test LOCATION ''");

While on it, note that this breaks the case of cross-version upgrades
where the old cluster uses binaries where in-place tablespaces are not
supported. So this requires an extra version check.
--
Michael