Re: A micro-optimisation for walkdir()

Thomas Munro <thomas.munro@gmail.com> writes:
>> On Wed, Sep 2, 2020 at 4:35 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Hm.  If we do this, I can see wanting to apply the knowledge in more
>>> places than walkdir().

> Good idea.  Here's a new version that defines a new function
> get_dirent_type() in src/common/file_utils_febe.c and uses it for both
> frontend and backend walkdir().

Quick thoughts on this patch:

* The API spec for get_dirent_type() needs to say that errno is
meaningful when the return value is PGFILETYPE_ERROR.  That's
something that would not be hard to break, so not documenting
the point at all doesn't seem great.  More generally, I don't
especially like having the callers know that the errno is from
stat() rather than something else.

* I don't quite like the calling code you have that covers some
return values and then has a default: case without any comment.
It's not really obvious that the default: case is expected to be
hit in non-error situations, especially when there is a separate
switch path for errors.  I can't find fault with the code as such,
but I think it'd be good to have a comment there.  Maybe along
the lines of "Ignore file types other than regular files and
directories".

Both of these concerns would abate if we had get_dirent_type()
just throw an error itself when stat() fails, thereby removing the
PGFILETYPE_ERROR result code.  I'm not 100% sold either way on
that, but it's something to think about.  Is there ever going
to be a reason for the caller to ignore an error?

            regards, tom lane