On Tue, Aug 9, 2022 at 3:27 PM Tom Lane <t...@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.
