On Wed, 6 Apr 2016 22:26:13 -0500
"Karl O. Pinc" <kop@meme.com> wrote:
> On Wed, 23 Mar 2016 23:22:26 +0100
> Gilles Darold <gilles.darold@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
logfile 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 <kop@meme.com>
Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
