On Tue, Aug 9, 2022 at 3:27 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Actually, having now read the patch, I don't think there is any
> part of 0002 that is a good idea. It's blithely removing the
> comments that explain why the existing coding is the way it is,
> and not providing a shred of justification for making checkpoints
> more brittle.
0002 also contradicts the original $SUBJECT and goal of this thread,
which is possibly why it was kept separate. I was only thinking of
committing 0001 myself, which is the one I'd reviewed an earlier
version of.
> I have not tried to analyze the error-handling properties of 0001,
> but if it's being equally cavalier then it shouldn't be committed
> either. Most of this behavior is the result of decades of hard-won
> experience; discarding it because it doesn't fit conveniently
> into some refactoring plan isn't smart.
0001 does introduce new errors, as mentioned in the commit message, in
the form of elevel ERROR passed into get_dirent_type(), which might be
thrown if your OS has no d_type and lstat() fails (also if you asked
to follow symlinks, but in those cases errors were already thrown).
But in those cases, it seems at least a little fishy that we ignored
errors from the existing lstat(). I wondered if that was because they
expected that any failure meant ENOENT and they wanted to tolerate
that, but that does not seem to be the case, so I considered the error
to be an improvement.
