Re: add support for logging current role (what to review?) - Mailing list pgsql-hackers

On Mon, Jun 27, 2011 at 6:25 PM, Alex Hunsaker <badalex@gmail.com> wrote:
> Ive been holding off because its marked as Waiting on Author, am now
> thinking thats a mistake. =)
>
> It links to this patch:
> http://archives.postgresql.org/message-id/20110215135131.GX4116@tamriel.snowman.net
>
> Which is older than the latest patch in that thread posted by Robert:
> http://archives.postgresql.org/message-id/AANLkTikMadttguOWTkKLtgfe90kxR=U9njk9zEbRwb-+@mail.gmail.com
>
> (There are also various other patches and versions in that thread...)
>
> The main difference between the first and the last patch is the first
> one has support for changing what csv columns we output, while the
> latter just tacks on an additional column.
>
> The thread was very long and left me a bit confused as to what I
> should actually be looking at. Or perhaps thats the point-- we need to
> decide if a csvlog_fields GUC is worth it.

This is pretty much a classic example of the tailing wagging the dog.

Adding a new field to the output would be pretty easy if it weren't
for the fact that we want the CSV log output to contain all the same
fields that are available in the regular log.  Of course, adding a
field to the CSV format is no big deal either.  But now you have more
problems: (1) it breaks backward compatibility, (2) it makes the log
files bigger, and (3) if the new field is more expensive to compute
than the existing ones, then you're forcing people who want to use the
CSV log format to incur an overhead for a feature they don't care
about.  We could surmount these problems by making the CSV log format
more flexible, but (a) that's a lot of work, (b) it imposes a burden
on tool-builders, and (c) it might produce a really long icky-looking
configuration line in postgresql.conf to show the default value of the
GUC controlling the log format.  (You may think I'm joking about this
last one, but the point was raised... several times.)

The upshot, presumably, is that if Stephen would like us to add this
new field, he needs to be willing to rewrite our entire logging
infrastructure first... and then maybe we'll bounce the rewrite
because it adds too much complexity, but who was it that asked for all
that complexity again?  Oh, that's right: we did.  Now, I admit that
I've been guilty of engaging in this kind of scope creep from time to
time myself, so everyone feel free to pitch your favorite loose object
in my direction.

Anyway, if we are going to insist on rewriting substantial chunks of
the logging infrastructure before doing this, we at least need to
reach some agreement on what would be an acceptable outcome - and then
let Stephen code that up as a separate patch, submit it, get it
committed, and then do this on top of that.  Or we can just decide
that adding one relatively minor field to the text format output is
not worth knocking over the applecart for.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company