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