From: | Justin Pryzby <pryzby(at)telsasoft(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, "Bossart, Nathan" <bossartn(at)amazon(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Stephen Frost <sfrost(at)snowman(dot)net>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, David Steele <david(at)pgmasters(dot)net>, pgsql-hackers(at)postgresql(dot)org, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de> |
Subject: | Re: pg_ls_tmpdir to show directories and shared filesets (and pg_ls_*) |
Date: | 2022-03-31 23:42:00 |
Message-ID: | 20220331234159.GS28503@telsasoft.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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(dot)GA29065(at)telsasoft(dot)com
> 20200313131232(dot)GO29065(at)telsasoft(dot)com
>
> I added the metadata function 2 years ago since it's silly to show metadata for
> tmpdir but not other, arbitrary directories.
> 20200310183037(dot)GA29065(at)telsasoft(dot)com
> 20200313131232(dot)GO29065(at)telsasoft(dot)com
> 20201223191710(dot)GR30237(at)telsasoft(dot)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(dot)GJ28974(at)telsasoft(dot)com
> 20191227195918(dot)GF12890(at)telsasoft(dot)com
> 20200116003924(dot)GJ26045(at)telsasoft(dot)com
> 20200908195126(dot)GB18552(at)telsasoft(dot)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
Attachment | Content-Type | Size |
---|---|---|
v35-0001-Document-historic-behavior-of-links-to-directori.patch | text/x-diff | 1.0 KB |
v35-0002-Add-tests-before-changing-pg_ls_.patch | text/x-diff | 3.4 KB |
v35-0003-Add-pg_ls_dir_metadata-to-list-a-dir-with-file-m.patch | text/x-diff | 17.8 KB |
v35-0004-pg_ls_tmpdir-to-show-directories-and-isdir-argum.patch | text/x-diff | 6.6 KB |
v35-0005-pg_ls_-dir-to-show-directories-and-isdir-column.patch | text/x-diff | 13.3 KB |
v35-0006-pg_ls_logdir-to-ignore-error-if-initial-top-dir-.patch | text/x-diff | 3.1 KB |
v35-0007-pg_ls_-dir-to-return-all-the-metadata-from-pg_st.patch | text/x-diff | 21.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Michail Nikolaev | 2022-03-31 23:57:44 | Re: [PATCH] Full support for index LP_DEAD hint bits on standby |
Previous Message | Bradley Ayers | 2022-03-31 23:39:06 | [WIP] ALTER COLUMN IF EXISTS |