On Tue, 18 Oct 2016 14:18:36 +0200
Gilles Darold <gilles.dar...@dalibo.com> wrote:

> Here is the v6 of the patch, here is the description of the
> pg_current_logfile() function, I have tried to keep thing as simple as
> possible:
> 
> pg_current_logfile( [ destination text ] )
> -----------------------------------------------------

Attached is a doc patch to apply on top of your v6 patch.

Changes are, roughly:

Put pg_log_file in alphabetical order in the file name listing
and section body.

Put pg_current_logfile in alphabetical order in the function
name listing and section body.

1 argument functions don't seem to have a parameter name
when listed in documentation tables, just a data type,
so I got rid of the parameter name for pg_current_logfile().

Cleaned up the wording and provided more detail.

Added hyperlink to log_destination config parameter.

Added markup to various system values.  The markup does
not stand out very well in the html docs, but that's a different
issue and should be fixed by changing the markup processing.
(I used the markup found in the log_destination()
config parameter docs.)

pg_current_logfile does not seem related to pg_listening_channels
or pg_notification_queue_usage so I moved the latter 2 index
entries closer to their text.


I also have a couple of preliminary comments.

It seems like the code is testing for whether the
logfile is a CSV file by looking for a '.csv' suffix
on the end of the file name.  This seems a little fragile.
Shouldn't it be looking at something set internally when the
log_destination config declaration is parsed?

Since pg_log_file may contain only one line, and that
line may be either the filename of the csv log file or
the file name of the stderr file name it's impossible
to tell whether that single file is in csv or stderr
format.  I suppose it might be possible based on file
name suffix, but Unix expressly ignores file name
extensions and it seems unwise to force dependence
on them.  Perhaps each line could begin with
the "type" of the file, either 'csv' or 'stderr'
followed by a space and the file name?  

In other words,
as long as you're making the content of pg_log_file
a data structure that contains more than just a single
file name you may as well make that data structure
something well-defined, easily parseable in shell, extensible,
and informative.

Hope to provide more feedback soon.

Regards,

Karl <k...@meme.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index d1a5b96..54e10af 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -15443,6 +15443,12 @@ SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS t(ls,n);
       </row>
 
       <row>
+       <entry><literal><function>pg_current_logfile(<optional><type>text</></optional>)</function></literal></entry>
+       <entry><type>text</type></entry>
+       <entry>log file in use by the logging collector</entry>
+      </row>
+
+      <row>
        <entry><literal><function>pg_my_temp_schema()</function></literal></entry>
        <entry><type>oid</type></entry>
        <entry>OID of session's temporary schema, or 0 if none</entry>
@@ -15480,12 +15486,6 @@ SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS t(ls,n);
       </row>
 
       <row>
-       <entry><literal><function>pg_current_logfile(<optional> <parameter>destination</> <type>text</></optional>)</function></literal></entry>
-       <entry><type>text</type></entry>
-       <entry>current log file used by the logging collector</entry>
-      </row>
-
-      <row>
        <entry><literal><function>session_user</function></literal></entry>
        <entry><type>name</type></entry>
        <entry>session user name</entry>
@@ -15667,6 +15667,27 @@ SET search_path TO <replaceable>schema</> <optional>, <replaceable>schema</>, ..
    </para>
 
    <indexterm>
+    <primary>pg_current_logfile</primary>
+   </indexterm>
+
+   <para>
+    <function>pg_current_logfile</function> returns, as <type>text</type>
+    the name of the log file currently in use by the logging collector.
+    Log collection must be active or the return value is undefined.  When
+    the <xref linkend="guc-log-destination"> configuration parameter
+    contains both <systemitem>csvlog</> and <systemitem>stderr</>
+    <function>pg_current_logfile</function> returns the stderr filename.
+    When <xref linkend="guc-log-destination"> does not
+    contain <systemitem>stderr</> the csv filename is returned.  When it
+    does not contains <systemitem>csvlog</> the stderr filename is
+    returned.  To request a specific file format supply either
+    <systemitem>csvlog</> or <systemitem>stderr</> as the value of the
+    optional, type <type>text</type>, parameter. The return value is
+    undefined when the log format requested is not a configured log
+    destination.
+   </para>
+
+   <indexterm>
     <primary>pg_my_temp_schema</primary>
    </indexterm>
 
@@ -15692,23 +15713,6 @@ SET search_path TO <replaceable>schema</> <optional>, <replaceable>schema</>, ..
     <primary>pg_notification_queue_usage</primary>
    </indexterm>
 
-   <indexterm>
-    <primary>pg_current_logfile</primary>
-   </indexterm>
-
-   <para>
-    <function>pg_current_logfile</function> returns the name of the
-    current log file used by the logging collector, as <type>text</type>.
-    Log collection must be active or the return value is undefined. When
-    csvlog is used as log destination, the csv filename is returned, when
-    it is set to stderr, the stderr filename is returned. When both are
-    used, it returns the stderr filename.
-    There is an optional parameter of type <type>text</type> to determines
-    the log filename to return following the log destination, values can
-    be 'csvlog' or 'stderr'. When the log format asked is not used as log
-    destination then the return value is undefined.
-   </para>
-
    <para>
     <function>pg_listening_channels</function> returns a set of names of
     asynchronous notification channels that the current session is listening
diff --git a/doc/src/sgml/storage.sgml b/doc/src/sgml/storage.sgml
index 6cecd81..6420597 100644
--- a/doc/src/sgml/storage.sgml
+++ b/doc/src/sgml/storage.sgml
@@ -83,6 +83,16 @@ Item
 </row>
 
 <row>
+ <entry><filename>pg_log_file</></entry>
+ <entry>A file recording the current log file(s) used by the syslogger, one
+  file name per line.  Only file names are present
+  in <filename>pg_log_file</>; the <xref linkend="guc-log-directory">
+  configuration parameter sets the location of the files.  This file is
+  present only when <xref linkend="guc-logging-collector"> is
+  activated.</entry>
+</row>
+
+<row>
  <entry><filename>pg_logical</></entry>
  <entry>Subdirectory containing status data for logical decoding</entry>
 </row>
@@ -170,13 +180,6 @@ last started with</entry>
   (this file is not present after server shutdown)</entry>
 </row>
 
-<row>
- <entry><filename>pg_log_file</></entry>
- <entry>A file recording the current log file(s) used by the syslogger
-  when log collection is active (this file is not present when logging_collector is not activated)</entry>
-</row>
-
-
 </tbody>
 </tgroup>
 </table>
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to