Horton

Re: Add support for logging the current role

Itagaki Takahiro <itagaki.takahiro@gmail.com>

View thread
From: Itagaki Takahiro <itagaki.takahiro@gmail.com>
To: Stephen Frost <sfrost@snowman.net>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Robert Haas <robertmhaas@gmail.com>, Andrew Dunstan <andrew@dunslane.net>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2011-01-24T10:15:26Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add support for an application_name parameter, which is displayed in

  2. Make CSV column ordering a bit more logical.

  3. Extend the format of CSV logs to include the additional information supplied

  4. Add virtual transaction IDs to CSVLOG output, so that messages coming from

  5. Provide for logfiles in machine readable CSV format. In consequence, rename

On Wed, Jan 19, 2011 at 14:36, Stephen Frost <sfrost@snowman.net> wrote:
> Alright, here's the latest on this patch.  I've added a log_csv_fields
> GUC along with the associated magic to make it work (at least for me).
> Also added 'role_name' and '%U' options.  Requires postmaster restart to
> change, didn't include any 'short-cut' field options, though I don't
> think it'd be hard to do if we can decide on it.  Default remains the
> same as what was in 9.0.

Hi, I reviewed log_csv_options.patch. It roughly looks good,
but I'd like to discuss about the design in some points.

==== Features ====
The patch adds "log_csv_fields" GUC variable. It allows to customize
columns in csvlog. The default setting is what we were writing 9.0 or
earlier versions.

It also add "role_name" for log_csv_fields and "%U" for log_line_prefix
to record role names. They are the name set by SET ROLE. OTOH, user_name
and %u shows authorization user names.

==== Discussions ====
* How about "csvlog_fields" rather than "log_csv_fields"?
  Since we have variables with syslog_ prefix, csvlog_ prefix
  seems to be better.

* We use %<what> syntax to specify fields in logs for log_line_prefix,
  but will use long field names for log_csv_fields. Do you have any
  better idea to share the names in both options?  At least I want to
  share the short description for them in postgresql.conf.

* log_csv_fields's GUC context is PGC_POSTMASTER. Is it by design?
  PGC_SIGHUP would be more consistent compared with log_line_prefix.
  However, the csv format will not be valid because the column
  definitions will be changed in the middle of file.

* "none" is not so useful for the initial "role_name" field.
  Should we use user_name instead of the empty role_name?

==== Comments for codes ====
Some of the items are trivial, though.

* What objects do you want to allocate in TopMemoryContext in
  assign_log_csv_fields() ? AFAICS, we don't have to keep rawstring
  and column_list in long-term context. Or, if you need TopMemoryContext,
  those variables should be pfree'ed at the end of function.

* appendStringInfoChar() calls in write_csvlog() seem to be wrong
  when the last field is not application_name.

* Docs need more cross-reference hyper-links for "see also" items.

* Docs need some tags for itemized elements or pre-formatted codes.
  They looks itemized in the sgml files, but will be flattened in
  complied HTML files.

* A declaration of assign_log_csv_fields() at the top of elog.c
  needs "extern".
* There is a duplicated declaration for build_default_csvlog_list().
* list_free() is NIL-safe. You don't have to check whether the list
  is NIL before call the function.

-- 
Itagaki Takahiro