public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v16 07/10] Add pg_ls_dir_recurse to show dir recursively..
5+ messages / 4 participants
[nested] [flat]

* [PATCH v16 07/10] Add pg_ls_dir_recurse to show dir recursively..
@ 2020-03-09 03:52  Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 5+ messages in thread

From: Justin Pryzby @ 2020-03-09 03:52 UTC (permalink / raw)

..possibly there's a better place to put this, like maybe a doc-only example ?

Need catversion bumped ?
---
 doc/src/sgml/func.sgml                       | 27 ++++++++++++++++++++
 src/backend/catalog/system_views.sql         |  1 +
 src/include/catalog/pg_proc.dat              |  6 +++++
 src/test/regress/expected/misc_functions.out | 14 ++++++++++
 src/test/regress/sql/misc_functions.sql      |  6 +++++
 5 files changed, 54 insertions(+)

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index c3a6b3277f..3a2ad3eb50 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -25311,6 +25311,18 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
         EXECUTE to run the function.
        </entry>
       </row>
+      <row>
+       <entry>
+        <literal><function>pg_ls_dir_recurse(<parameter>dirname</parameter> <type>text</type>)</function></literal>
+       </entry>
+       <entry><type>setof text</type></entry>
+       <entry>
+        Call pg_ls_dir_metadata to recursively list the files in the specified
+        directory, along with each file's metadata.
+        Restricted to superusers by default, but other users can be granted
+        EXECUTE to run the function.
+       </entry>
+      </row>
       <row>
        <entry>
         <literal><function>pg_ls_logdir()</function></literal>
@@ -25422,6 +25434,21 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
     directory along with the file's metadata.
    </para>
 
+   <indexterm>
+    <primary>pg_ls_dir_recurse</primary>
+   </indexterm>
+   <para>
+    <function>pg_ls_dir_recurse</function> recursively lists the files 
+    in the specified directory.
+    To recursively list temporary directories in all tablespaces:
+<programlisting>
+SELECT * FROM (SELECT DISTINCT COALESCE(NULLIF(pg_tablespace_location(b.oid),'')||suffix, 'base/pgsql_tmp') AS dir
+FROM pg_tablespace b, pg_control_system() pcs,
+LATERAL format('/PG_%s_%s', left(current_setting('server_version_num'), 2), pcs.catalog_version_no) AS suffix) AS dir,
+LATERAL pg_ls_dir_recurse(dir) AS a;
+</programlisting>
+   </para>
+
    <indexterm>
     <primary>pg_ls_logdir</primary>
    </indexterm>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 1c77430f0c..215fba3b7d 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1468,6 +1468,7 @@ REVOKE EXECUTE ON FUNCTION pg_stat_file(text,boolean) FROM public;
 REVOKE EXECUTE ON FUNCTION pg_ls_dir(text) FROM public;
 REVOKE EXECUTE ON FUNCTION pg_ls_dir(text,boolean,boolean) FROM public;
 REVOKE EXECUTE ON FUNCTION pg_ls_dir_metadata(text,boolean,boolean) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_ls_dir_recurse(text) FROM public;
 
 --
 -- We also set up some things as accessible to standard roles.
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index adfce45d1a..f94f403475 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10916,6 +10916,12 @@
   proallargtypes => '{text,text,int8,timestamptz,bool}', proargmodes => '{i,o,o,o,o}',
   proargnames => '{dirname,name,size,modification,isdir}',
   prosrc => 'pg_ls_dir_metadata_1arg' },
+{ oid => '5034', descr => 'list all files in a directory recursively',
+  proname => 'pg_ls_dir_recurse', prorows => '10000', proretset => 't',
+  provolatile => 'v', prorettype => 'record', proargtypes => 'text',
+  proallargtypes => '{text,text,int8,timestamptz,bool}',
+  proargnames => '{dirname,name,size,modification,isdir}', proargmodes => '{i,o,o,o,o}',
+  prolang => 'sql', prosrc => "with recursive ls as (select * from pg_ls_dir_metadata(dirname, true, false) union all select ls.name||'/'||a.name, a.size, a.modification, a.isdir from ls, lateral pg_ls_dir_metadata(dirname||'/'||ls.name, false, false)a where ls.isdir) select * from ls" },
 
 # hash partitioning constraint function
 { oid => '5028', descr => 'hash partition CHECK constraint',
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
index 64b1417fb8..4188d684f0 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -243,6 +243,20 @@ select * from pg_ls_dir_metadata('.') limit 0;
 ------+------+--------------+-------
 (0 rows)
 
+-- Check that we at least succeed in recursing once, and that we don't show the leading dir prefix
+SELECT name, isdir FROM pg_ls_dir_recurse('.') WHERE isdir AND name~'^pg_wal';
+         name          | isdir 
+-----------------------+-------
+ pg_wal                | t
+ pg_wal/archive_status | t
+(2 rows)
+
+-- Check that expected columns are present
+SELECT * FROM pg_ls_dir_recurse('.') LIMIT 0;
+ name | size | modification | isdir 
+------+------+--------------+-------
+(0 rows)
+
 --
 -- Test adding a support function to a subject function
 --
diff --git a/src/test/regress/sql/misc_functions.sql b/src/test/regress/sql/misc_functions.sql
index 372345720d..6041c4f3dc 100644
--- a/src/test/regress/sql/misc_functions.sql
+++ b/src/test/regress/sql/misc_functions.sql
@@ -76,6 +76,12 @@ select name, isdir from pg_ls_dir_metadata('.', false, false) where name='.'; --
 -- Check that expected columns are present
 select * from pg_ls_dir_metadata('.') limit 0;
 
+-- Check that we at least succeed in recursing once, and that we don't show the leading dir prefix
+SELECT name, isdir FROM pg_ls_dir_recurse('.') WHERE isdir AND name~'^pg_wal';
+
+-- Check that expected columns are present
+SELECT * FROM pg_ls_dir_recurse('.') LIMIT 0;
+
 --
 -- Test adding a support function to a subject function
 --
-- 
2.17.0


--2FkSFaIQeDFoAt0B
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v16-0008-pg_ls_logdir-to-ignore-error-if-initial-top-dir-.patch"



^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: DOCS - pg_replication_slot . Fix the 'inactive_since' description
@ 2024-09-02 03:43  shveta malik <[email protected]>
  0 siblings, 1 reply; 5+ messages in thread

From: shveta malik @ 2024-09-02 03:43 UTC (permalink / raw)
  To: Peter Smith <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; shveta malik <[email protected]>

On Mon, Sep 2, 2024 at 5:47 AM Peter Smith <[email protected]> wrote:
>
> Hi hackers. While reviewing another thread I had cause to look at the
> docs for the pg_replication_slot 'inactive_since' field [1] for the
> first time.
>
> I was confused by the description, which is as follows:
> ----
> inactive_since timestamptz
> The time since the slot has become inactive. NULL if the slot is
> currently being used.
> ----
>
> Then I found the github history for the patch [2], and the
> accompanying long thread discussion [3] about the renaming of that
> field. I have no intention to re-open that can-of-worms, but OTOH I
> feel the first sentence of the field description is wrong and needs
> fixing.
>
> Specifically, IMO describing something as "The time since..." means
> some amount of elapsed time since some occurrence, but that is not the
> correct description for this timestamp field.
>
> This is not just a case of me being pedantic. For example, here is
> what Chat-GPT had to say:
> ----
> I asked:
> What does "The time since the slot has become inactive." mean?
>
> ChatGPT said:
> "The time since the slot has become inactive" refers to the duration
> that has passed from the moment a specific slot (likely a database
> replication slot or a similar entity) stopped being active. In other
> words, it measures how much time has elapsed since the slot
> transitioned from an active state to an inactive state.
>
> For example, if a slot became inactive 2 hours ago, "the time since
> the slot has become inactive" would be 2 hours.
> ----
>
> To summarize, the current description wrongly describes the field as a
> time duration:
> "The time since the slot has become inactive."
>
> I suggest replacing it with:
> "The slot has been inactive since this time."
>

+1 for the change. If I had read the document without knowing about
the patch, I too would have interpreted it as a duration.

thanks
Shveta






^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: DOCS - pg_replication_slot . Fix the 'inactive_since' description
@ 2024-09-03 05:13  Amit Kapila <[email protected]>
  parent: shveta malik <[email protected]>
  0 siblings, 2 replies; 5+ messages in thread

From: Amit Kapila @ 2024-09-03 05:13 UTC (permalink / raw)
  To: shveta malik <[email protected]>; +Cc: Peter Smith <[email protected]>; PostgreSQL Hackers <[email protected]>

On Mon, Sep 2, 2024 at 9:14 AM shveta malik <[email protected]> wrote:
>
> On Mon, Sep 2, 2024 at 5:47 AM Peter Smith <[email protected]> wrote:
> > ----
> >
> > To summarize, the current description wrongly describes the field as a
> > time duration:
> > "The time since the slot has become inactive."
> >
> > I suggest replacing it with:
> > "The slot has been inactive since this time."
> >
>
> +1 for the change. If I had read the document without knowing about
> the patch, I too would have interpreted it as a duration.
>

The suggested change looks good to me as well. I'll wait for a day or
two before pushing to see if anyone thinks otherwise.

-- 
With Regards,
Amit Kapila.






^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: DOCS - pg_replication_slot . Fix the 'inactive_since' description
@ 2024-09-03 06:12  shveta malik <[email protected]>
  parent: Amit Kapila <[email protected]>
  1 sibling, 0 replies; 5+ messages in thread

From: shveta malik @ 2024-09-03 06:12 UTC (permalink / raw)
  To: Amit Kapila <[email protected]>; +Cc: Peter Smith <[email protected]>; PostgreSQL Hackers <[email protected]>; shveta malik <[email protected]>

On Tue, Sep 3, 2024 at 10:43 AM Amit Kapila <[email protected]> wrote:
>
> > >
> > > To summarize, the current description wrongly describes the field as a
> > > time duration:
> > > "The time since the slot has become inactive."
> > >
> > > I suggest replacing it with:
> > > "The slot has been inactive since this time."
> > >
> >
> > +1 for the change. If I had read the document without knowing about
> > the patch, I too would have interpreted it as a duration.
> >
>
> The suggested change looks good to me as well. I'll wait for a day or
> two before pushing to see if anyone thinks otherwise.

Shall we make the change in code-comment as well:

typedef struct ReplicationSlot
{
...
        /* The time since the slot has become inactive */
        TimestampTz inactive_since;
}

thanks
Shveta






^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: DOCS - pg_replication_slot . Fix the 'inactive_since' description
@ 2024-09-04 05:32  Kyotaro Horiguchi <[email protected]>
  parent: Amit Kapila <[email protected]>
  1 sibling, 0 replies; 5+ messages in thread

From: Kyotaro Horiguchi @ 2024-09-04 05:32 UTC (permalink / raw)
  To: [email protected]; +Cc: [email protected]; [email protected]; [email protected]

At Tue, 3 Sep 2024 10:43:14 +0530, Amit Kapila <[email protected]> wrote in 
> On Mon, Sep 2, 2024 at 9:14 AM shveta malik <[email protected]> wrote:
> >
> > On Mon, Sep 2, 2024 at 5:47 AM Peter Smith <[email protected]> wrote:
> > > ----
> > >
> > > To summarize, the current description wrongly describes the field as a
> > > time duration:
> > > "The time since the slot has become inactive."
> > >
> > > I suggest replacing it with:
> > > "The slot has been inactive since this time."
> > >
> >
> > +1 for the change. If I had read the document without knowing about
> > the patch, I too would have interpreted it as a duration.
> >
> 
> The suggested change looks good to me as well. I'll wait for a day or
> two before pushing to see if anyone thinks otherwise.

If possible, I'd prefer to use "the time" as the subject. For example,
would "The time this slot was inactivated" be acceptable? However,
this loses the sense of continuation up to that point, so if that's
crucial, the current proposal might be better.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center


^ permalink  raw  reply  [nested|flat] 5+ messages in thread


end of thread, other threads:[~2024-09-04 05:32 UTC | newest]

Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-03-09 03:52 [PATCH v16 07/10] Add pg_ls_dir_recurse to show dir recursively.. Justin Pryzby <[email protected]>
2024-09-02 03:43 Re: DOCS - pg_replication_slot . Fix the 'inactive_since' description shveta malik <[email protected]>
2024-09-03 05:13 ` Re: DOCS - pg_replication_slot . Fix the 'inactive_since' description Amit Kapila <[email protected]>
2024-09-03 06:12   ` Re: DOCS - pg_replication_slot . Fix the 'inactive_since' description shveta malik <[email protected]>
2024-09-04 05:32   ` Re: DOCS - pg_replication_slot . Fix the 'inactive_since' description Kyotaro Horiguchi <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox