* Tom Lane (t...@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 <sfr...@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 <sfr...@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.
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***************
*** 3504,3510 **** local0.* /var/log/postgresql
</row>
<row>
<entry><literal>%u</literal></entry>
! <entry>User name</entry>
<entry>yes</entry>
</row>
<row>
--- 3504,3523 ----
</row>
<row>
<entry><literal>%u</literal></entry>
! <entry>Session user name, typically the user name which was used
! to authenticate to <productname>PostgreSQL</productname> with,
! but can be changed by a superuser, see <command>SET SESSION
! AUTHORIZATION</></entry>
! <entry>yes</entry>
! </row>
! <row>
! <entry><literal>%U</literal></entry>
! <entry>Current role name, when set with <command>SET ROLE</>;
! the current role identifier is relevant for permission checking;
! Returns 'none' if the current role matches the session user.
! Note: Log messages from inside <literal>SECURITY DEFINER</>
! functions will show the calling role, not the effective role
! inside the <literal>SECURITY DEFINER</> function</entry>
<entry>yes</entry>
</row>
<row>
***************
*** 3731,3737 **** FROM pg_stat_activity;
(<acronym>CSV</>) format,
with these columns:
timestamp with milliseconds,
! user name,
database name,
process ID,
client host:port number,
--- 3744,3751 ----
(<acronym>CSV</>) format,
with these columns:
timestamp with milliseconds,
! session user name,
! current role name,
database name,
process ID,
client host:port number,
***************
*** 3755,3760 **** FROM pg_stat_activity;
--- 3769,3778 ----
location of the error in the PostgreSQL source code
(if <varname>log_error_verbosity</> is set to <literal>verbose</>),
and application name.
+
+ For additional details on the definition of the above columns, refer
+ to the documentation for log_line_prefix.
+
Here is a sample table definition for storing CSV-format log output:
<programlisting>
***************
*** 3762,3767 **** CREATE TABLE postgres_log
--- 3780,3786 ----
(
log_time timestamp(3) with time zone,
user_name text,
+ role_name text,
database_name text,
process_id integer,
connection_from text,
*** a/src/backend/commands/variable.c
--- b/src/backend/commands/variable.c
***************
*** 760,765 **** assign_session_authorization(const char *value, bool doit, GucSource source)
--- 760,770 ----
return result;
}
+ /*
+ * function to return the stored session username, needed because we
+ * can't do catalog lookups when possibly being called after an error,
+ * eg: from elog.c or part of GUC handling.
+ */
const char *
show_session_authorization(void)
{
***************
*** 885,890 **** assign_role(const char *value, bool doit, GucSource source)
--- 890,900 ----
return result;
}
+ /*
+ * function to return the stored role username, needed because we
+ * can't do catalog lookups when possibly being called after an error,
+ * eg: from elog.c or part of GUC handling.
+ */
const char *
show_role(void)
{
*** a/src/backend/utils/error/elog.c
--- b/src/backend/utils/error/elog.c
***************
*** 3,8 ****
--- 3,17 ----
* elog.c
* error logging and reporting
*
+ * A few comments about situations where error processing is called:
+ *
+ * We need to be cautious of both a performance hit when logging, since
+ * log messages can be generated at a huge rate if every command is being
+ * logged and we also need to watch out for what can happen when we are
+ * trying to log from an aborted transaction. Specifically, attempting to
+ * do SysCache lookups and possibly use other usually available backend
+ * systems will fail badly when logging from an aborted transaction.
+ *
* Some notes about recursion and errors during error processing:
*
* We need to be robust about recursive-error scenarios --- for example,
***************
*** 59,64 ****
--- 68,74 ----
#include "access/transam.h"
#include "access/xact.h"
+ #include "commands/variable.h"
#include "libpq/libpq.h"
#include "libpq/pqformat.h"
#include "mb/pg_wchar.h"
***************
*** 1817,1831 **** log_line_prefix(StringInfo buf, ErrorData *edata)
}
break;
case 'u':
- if (MyProcPort)
{
! const char *username = MyProcPort->user_name;
!
! if (username == NULL || *username == '\0')
! username = _("[unknown]");
! appendStringInfoString(buf, username);
}
break;
case 'd':
if (MyProcPort)
{
--- 1827,1850 ----
}
break;
case 'u':
{
! const char *session_auth = show_session_authorization();
! if (*session_auth != '\0')
! appendStringInfoString(buf, session_auth);
! else
! if (MyProcPort)
! {
! const char *username = MyProcPort->user_name;
!
! if (username == NULL || *username == '\0')
! username = _("[unknown]");
! appendStringInfoString(buf, username);
! }
}
break;
+ case 'U':
+ appendStringInfoString(buf, show_role());
+ break;
case 'd':
if (MyProcPort)
{
***************
*** 1952,1957 **** appendCSVLiteral(StringInfo buf, const char *data)
--- 1971,1978 ----
static void
write_csvlog(ErrorData *edata)
{
+ const char *session_auth = show_session_authorization();
+
StringInfoData buf;
bool print_stmt = false;
***************
*** 1989,1997 **** write_csvlog(ErrorData *edata)
appendStringInfoString(&buf, formatted_log_time);
appendStringInfoChar(&buf, ',');
! /* username */
! if (MyProcPort)
! appendCSVLiteral(&buf, MyProcPort->user_name);
appendStringInfoChar(&buf, ',');
/* database name */
--- 2010,2031 ----
appendStringInfoString(&buf, formatted_log_time);
appendStringInfoChar(&buf, ',');
! /* session username, as done for %u */
! if (*session_auth != '\0')
! appendCSVLiteral(&buf, session_auth);
! else
! if (MyProcPort)
! {
! const char *username = MyProcPort->user_name;
!
! if (username == NULL || *username == '\0')
! username = _("[unknown]");
! appendCSVLiteral(&buf, username);
! }
! appendStringInfoChar(&buf, ',');
!
! /* current role, same as %U */
! appendCSVLiteral(&buf, show_role());
appendStringInfoChar(&buf, ',');
/* database name */
signature.asc
Description: Digital signature
