On Thu, Mar 17, 2022 at 3:29 AM David Steele <david@pgmasters.net> wrote:
> Sure, but there is a behavioral change whether the GUC is enabled or
> not. Before, if there was clutter in pg_tblspc there would at least be a
> warning in the log. Now that logging does not happen.
If there's clutter that doesn't look like an OID, we already ignore it
silently here:
/* Skip anything that doesn't look like a tablespace */
if (strspn(de->d_name, "0123456789") != strlen(de->d_name))
continue;
> It seems that the warning at line 8313 is essentially dead code now. I
> don't expect test code to have an impact on production systems, even if
> the effect is minor.
It's not dead, it's how we'd report something like EACCES or EIO. Why
we only warn and press on, I'm not sure. (I'm also looking into why
we ignore OS errors for pgwin32_is_junction everywhere.)
> I'm less worried about what happens when the flag is flipped on then off
> because that's not likely to happen in production.
So, concretely, if there is a non-symlink with a name that looks like
an OID under pg_tblspc, previously we'd barf (or apparently press on
with an empty pathname on Windows, which might indicate a lack of
error checking somewhere). Given the policy of quietly ignoring
anything else in the directory, is it really this code's job to sanity
check the cluster layout? Hmm, I guess we could fix the problem on
Windows in a different way so that it behaves like Unix, and then
revert this. You'd get an ignorable not-a-symlink warning as before
(and now it'd work on Windows) when backing up in-place tablespaces,
but I'm not sure it's really an improvement.