On Wed, 6 Apr 2016 22:26:13 -0500 "Karl O. Pinc" <k...@meme.com> wrote: > On Wed, 23 Mar 2016 23:22:26 +0100 > Gilles Darold <gilles.dar...@dalibo.com> wrote: > > > Thanks for the reminder, here is the v3 of the patch after a deeper > > review and testing. It is now registered to the next commit fest > > under the System Administration topic.
> I've not yet even tried building it, Ok. I've built it (but not tested it). Some comments. The docs don't build. You are missing a "<row"> at line 15197 of func.sgml. (Patched against current HEAD.) Is there any rationale for the placement of your function in that documentation table? I don't see any organizing principle to the table so am wondering where it might best fit. (http://www.postgresql.org/docs/devel/static/functions-info.html) Perhaps you want to write?: <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. </para> (Removed "a" before "text", and added "or..." to the end.) Unless you want to define the return value to be NULL when log collection is not active. This might be cleaner. I say this because I'm tempted to add "This can be tested for with current_setting('logging_collector')='on'." to the end of the paragraph. But adding such text means that the "logging_collector" setting is documented in multiple places, in some sense, and such redundancy is best avoided when possible. I don't see a problem with defining the return value to be NULL -- so long as it's defined to be NULL when there is no current log file. This would be different from defining it to be NULL when log collection is off. Although not very different it should be clear that using pg_currrent_logfile() to test whether log collection is on is not a good practice. Perhaps some text like?: <para> <function>pg_current_logfile</function> returns the name of the current log file used by the logging collector, as <type>text</type>. <literal>NULL</literal> is returned and a <literal>NOTICE</literal> raised when no log file exists. </para> (I'm going to have to think some more about the raising of the notice, and of the other error handling in your code. I've not paid it any attention and error handling is always problematic.) Regards, Karl <k...@meme.com> Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
