Re: Add support for logging the current role - Mailing list pgsql-hackers

From Stephen Frost
Subject Re: Add support for logging the current role
Date
Msg-id 20110114164024.GT4933@tamriel.snowman.net
Whole thread Raw
In response to Re: Add support for logging the current role  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> I seem to recall that the assign hook for role stores the string form of
> the role name anyway.

Indeed it does, and it's already exposed through show_role() since it's
needed in guc.c.  Based on my review and understanding of the comments
and calls, it also doesn't do anything particularly complicated or any
syscache searches or anything.

> It wouldn't track the
> effects of RENAME ROLE against an actively-used role, but then again
> neither does %u.

Right, I didn't specifically point that out in the documentation
changes, but I can if people feel it's neceessary.

> What bothered me more was the cost of adding another output
> column to CSV log mode.  That's not something you're going to be able to
> finesse such that only people who care pay the cost.

I definitely feel this is something that we should be logging in the CSV
also, and you're right, there doesn't appear to be a way to do that
without just outright changing the format and causing people to have to
update anything/everything that uses it.  I have a hard time with the
idea that we'll commit to never changing that format though, so do we
want to provide a way for users to specify the format (ala
log_line_prefix), or just ask users to expect and deal with format
changes when they happen..?

I noticed Dimitri would like another change to the CSV log format (which
looked reasonable to me, asking to have something split out from the
query string itself), it'd certainly be better to change both in the
same release than split them across two (of course, we might come up
with something else in the future...).

I have to admit to being a bit suprised the CSV logging wasn't
implemented with a 'format' type option.  I'm not sure if I have the
cycles or even if we would want to try and add that now, but it
strikes me as something we should probably do.

Updated patch attached, included new comments for elog.c too.

    Thanks,

        Stephen

commit 4e27ab79ef9b0d0c3c9824d672e06160dd227cc2
Author: Stephen Frost <sfrost@snowman.net>
Date:   Wed Jan 12 12:22:16 2011 -0500

    Improve comments at the top of elog.c

    Add in some comments about how certain usually available backend
    systems may be unavailable or which won't function properly in
    elog.c due to the current transaction being in a failed state.

commit d3ca4063ba8e16930278947c32c336b5b80cdaba
Author: Stephen Frost <sfrost@snowman.net>
Date:   Fri Jan 14 11:19:45 2011 -0500

    Add %U option to log_line_prefix for current role

    This adds a new option to log_line_prefix (%U) to allow the current
    role to be logged, which is valuable information when an application
    or user is using SET ROLE and roles which are set 'noinherit'.

    This also changes the current definition of %u to be 'Session user',
    to avoid confusion when a superuser uses 'SET SESSION AUTHORIZATION'.
    Otherwise, a log might read 'login_user none' but actually be running
    as a different user due to SET SESSION AUTHORIZATION.  The 'username'
    field for CSV logging was also updated to be 'Session user'.  Note:
    SET SESSION AUTHORIZATION is only allowed for superusers, and the
    logged username will only change if SET SESSION AUTHORIZATION is
    called, so this is unlikely to have signifigant user impact.

    Last, but certainly not least, role_name was added as a new column to
    the CSV log output and corresponding example CSV table definition.
    This is a user-visible change which should be called out in the release
    notes.

Attachment

pgsql-hackers by date:

Previous
From: Andrew Dunstan
Date:
Subject: Re: Add support for logging the current role
Next
From: Tom Lane
Date:
Subject: Re: Add support for logging the current role