Re: Add support for logging the current role

On Mon, Feb 14, 2011 at 11:51, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>> It would be awfully nice if we could inject something into the csvlog
>> output format that would let client programs know which fields are
>> present.  This would be especially useful for people writing tools
>> that are intended to work on ANY PG installation, rather than just,
>> say, their own.  I'm not sure if there's a clean way to do that,
>> though.
>
> Added csvlog_header GUC to allow the user to ask for the header to be
> printed at the top of each log file.  If and when an option is added to
> PG's COPY to respect the header, this should resolve that issue.

We need to design csvlog_header more carefully. csvlog_header won't work
if log_filename is low-resolution, ex. log-per-day. It's still useful when
a DBA reads the file manually, but documentation would better.
Or, should we skip writing headers when the open log file is not
empty? It works unless when csvlog_fields is modified before restart,
and also on logger's crash and restart, though it's a rare case.

A few comments and trivial issues:

* It might be my misunderstanding, but there was a short description for %U
for in log_line_prefix in postgresql.conf, right? Did you remove it in the
latest version?

* The long csvlog_fields default is a problem also in postgresql.conf,
but we have no solution for the issue... The current code is the best for now.

* In assign_csvlog_fields(), we need to cleanup memory and memory context
before return on error.
+ /* check if no option matched, and if so, return error */
+ if (curr_option == MAX_CSVLOG_OPTS)
+ return NULL;

* An added needless "return" should be removed.
*** 2139,2144 **** write_csvlog(ErrorData *edata)
--- 2379,2386 ----
write_pipe_chunks(buf.data, buf.len, LOG_DESTINATION_CSVLOG);

pfree(buf.data);
+
+ return;
}

/*

--
Itagaki Takahiro