On 3/16/22 13:33, Thomas Munro wrote:
>
>> 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 also wondering why this is only a warning.
>> 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.
I'm not going to press on this, but FWIW this solution sounds better to
me. Either way it is a behavioral change. Windows used to error in this
case and now it does not, Unix used to warn in this case and now it does
not.
For my 2C I'd rather this was a hard error because that makes life a lot
easier down the line. But, that's certainly not the responsibility of
this patch.
Regards,
-David
