On Mon, Mar 14, 2022 at 09:37:25PM -0500, Justin Pryzby wrote:
> The original, minimal goal of this patch was to show shared tempdirs in
> pg_ls_tmpfile() - rather than hiding them misleadingly as currently happens.
> 20200310183037.GA29065@telsasoft.com
> 20200313131232.GO29065@telsasoft.com
>
> I added the metadata function 2 years ago since it's silly to show metadata for
> tmpdir but not other, arbitrary directories.
> 20200310183037.GA29065@telsasoft.com
> 20200313131232.GO29065@telsasoft.com
> 20201223191710.GR30237@telsasoft.com
I renamed the CF entry to make even more clear the original motive for the
patches (I'm not maintaining the patch to add the metadata function just to
avoid writing a lateral join).
> > In the whole set, improving the docs as of 0001 makes sense, but the
> > change is incomplete. Most of 0002 also makes sense and should be
> > stable enough. I am less enthusiastic about any of the other changes
> > proposed and what we can gain from these parts.
>
> It is frustrating to hear this feedback now, after the patch has gone through
> multiple rewrites over 2 years - based on other positive feedback and review.
> I went to the effort to ask, numerous times, whether to write the patch and how
> its interfaces should look. Now, I'm hearing that not only the implementation
> but its goals are wrong. What should I have done to avoid that ?
>
> 20200503024215.GJ28974@telsasoft.com
> 20191227195918.GF12890@telsasoft.com
> 20200116003924.GJ26045@telsasoft.com
> 20200908195126.GB18552@telsasoft.com
Michael said he's not enthusiastic about the patch. But I haven't heard a
suggestion about how else to address the issue that pg_ls_tmpdir() hides shared
filesets.
On Mon, Mar 28, 2022 at 09:13:52PM -0500, Justin Pryzby wrote:
> On Sat, Mar 26, 2022 at 08:23:54PM +0900, Michael Paquier wrote:
> > On Wed, Mar 23, 2022 at 03:17:35PM +0900, Michael Paquier wrote:
> > > FWIW, per my review the bit of the patch set that I found the most
> > > relevant is the addition of a note in the docs of pg_stat_file() about
> > > the case where "filename" is a link, because the code internally uses
> > > stat(). The function name makes that obvious, but that's not
> > > commonly known, I guess. Please see the attached, that I would be
> > > fine to apply.
> >
> > Hmm. I am having second thoughts on this one, as on Windows we rely
> > on GetFileInformationByHandle() for the emulation of stat() in
> > win32stat.c, and it looks like this returns some information about the
> > junction point and not the directory or file this is pointing to, it
> > seems.
>
> Where did you find that ? What metadata does it return about the junction
> point ? We only care about a handful of fields.
Pending your feedback, I didn't modify this beyond your original suggestion -
which seemed like a good one.
This also adds some comments you requested and fixes your coding style
complaints, and causes cfbot to test my proposed patch rather than your doc
patch.
--
Justin
