From: Justin Pryzby <pryzby@telsasoft.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org, Fabien COELHO <coelho@cri.ensmp.fr>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: David Steele <david@pgmasters.net>
Cc: Bossart, Nathan <bossartn@amazon.com>
Cc: Thomas Munro <thomas.munro@gmail.com>
Subject: Re: pg11+: pg_ls_*dir LIMIT 1: temporary files .. not closed at end-of-transaction
Date: Sun, 29 Mar 2020 12:14:15 -0500
Message-ID: <20200329171415.GL20103@telsasoft.com> (raw)
In-Reply-To: <27064.1585499825@sss.pgh.pa.us>
References: <20200311111921.GQ29065@telsasoft.com>
<21724.1583955158@sss.pgh.pa.us>
<20200312121156.GB29065@telsasoft.com>
<20200316155306.GM26184@telsasoft.com>
<3061.1584409130@sss.pgh.pa.us>
<20200317020017.GT26184@telsasoft.com>
<24244.1585415634@sss.pgh.pa.us>
<20200328183904.GI20103@telsasoft.com>
<28906.1585422475@sss.pgh.pa.us>
<27064.1585499825@sss.pgh.pa.us>
On Sun, Mar 29, 2020 at 12:37:05PM -0400, Tom Lane wrote:
> I wrote:
> > Justin Pryzby <pryzby@telsasoft.com> writes:
> >> Maybe we should lstat() the file to determine if it's a dangling link; if
> >> lstat() fails, then skip it. Currently, we use stat(), which shows metdata of
> >> a link's *target*. Maybe we'd change that.
>
> > Hm, good point that ENOENT could refer to a symlink's target. Still,
> > I'm not sure it's worth going out of our way to disambiguate that,
> > given that these directories aren't really supposed to contain symlinks.
> > (And on the third hand, if they aren't supposed to, then maybe these
> > functions needn't look through any symlinks? In which case just
> > substituting lstat for stat would resolve the ambiguity.)
>
> After looking at the callers of pg_ls_dir_files, and noticing that
> it's already defined to ignore anything that's not a regular file,
> I think switching to lstat makes sense.
Yea, only pg_ls_dir() shows special file types (and currently the others even
hide dirs).
The essence of your patch is to ignore ENOENT, but you also changed to use
lstat(), which seems unrelated. That means we'll now hide (non-broken)
symlinks. Is that intentional/needed ? I guess maybe you're trying to fix the
bug (?) that symlinks aren't skipped? If so, I guess it should be a separate
commit, or the commit message should say so. I think the doc update is already
handled by: 8b6d94cf6c8319bfd6bebf8b863a5db586c19c3b (we didn't used to say we
skipped specials, and now we say we do, and we'll to follow through RSN and
actually do it, too).
> diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
> index 01185f2..8429a12 100644
> --- a/src/backend/utils/adt/genfile.c
> +++ b/src/backend/utils/adt/genfile.c
> @@ -596,10 +596,15 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok)
>
> /* Get the file info */
> snprintf(path, sizeof(path), "%s/%s", dir, de->d_name);
> - if (stat(path, &attrib) < 0)
> + if (lstat(path, &attrib) < 0)
> + {
> + /* Ignore concurrently-deleted files, else complain */
> + if (errno == ENOENT)
> + continue;
> ereport(ERROR,
> (errcode_for_file_access(),
> errmsg("could not stat file \"%s\": %m", path)));
> + }
>
> /* Ignore anything but regular files */
> if (!S_ISREG(attrib.st_mode))
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pryzby@telsasoft.com, tgl@sss.pgh.pa.us, coelho@cri.ensmp.fr, alvherre@2ndquadrant.com, david@pgmasters.net, bossartn@amazon.com, thomas.munro@gmail.com
Subject: Re: pg11+: pg_ls_*dir LIMIT 1: temporary files .. not closed at end-of-transaction
In-Reply-To: <20200329171415.GL20103@telsasoft.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox