On Fri, 2020-02-21 at 15:36 +0900, Michael Paquier wrote:
> We don't do that for the normal directory scan path, so it does not
> strike me as a good idea on consistency ground. As a whole, I don't
> see much point in having a separate routine which is just roughly a
> duplicate of scan_directory(), and I think that we had better just
> add
> the check looking for matches with TABLESPACE_VERSION_DIRECTORY
> directly when having a directory, if subdir is "pg_tblspc". That
> also makes the patch much shorter.
To be honest, i dislike both: The other doubles logic (note: i don't
see it necessarily as 100% code duplication, since the semantic of
scan_tablespaces() is different: it serves as a driver for
scan_directories() and just resolves entries in pg_tblspc directly).
The other makes scan_directories() complicater to read and special
cases just a single directory in an otherwise more or less generic
function. E.g. it makes me uncomfortable if we get a pg_tblspc
somewhere else than PGDATA (if someone managed to create such a
directory in a foreign tablespace location for example), so we should
maintain an additional check if we really operate on the pg_tblspc we
have to. That was the reason(s) i've moved it into a separate function.
That said, i'll provide an updated patch with your ideas.
Bernd
