Re: teach pg_upgrade to handle in-place tablespaces

On Tue, Jul 22, 2025 at 10:37:05AM +0900, Michael Paquier wrote:
> 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.

I'm not sure what you mean. allow_in_place_tablespaces was added in v15,
right?

> 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.

Seems reasonable, will add this.

> + /*
> + * 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?).

I'm not aware of any such discussion. I know there was a time before
version-specific tablespace subdirectories were a thing, but that is long
gone. And yeah, we've preserved tablespace OIDs since v15.

> + if (is_absolute_path(PQgetvalue(res, tblnum, i_spclocation)))
> + {
>
> This use of is_absolute_path() should document that it is for in-place
> tablespaces?

Yes.

> + 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.

I'll take a look and see what I can do.

--
nathan