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
signature.asc
Description: PGP signature