* Robert Haas (robertmh...@gmail.com) wrote: > Logging the OID seems to be of questionable value.
I certainly disagree about this, not being able to figure out what's
causing a 'permissions denied' error because you don't know which role
the log is coming from is *very* annoying. Having to go look up the
role from the OID in the log is also annoying, but less so, imv. :)
> I thought of the
> update-the-variable-when-it-changes approach too, but besides being a
> bit expensive if it's changing frequently, it's not necessarily safe
> to do the syscache lookup there either - see the comments for
> GetUserIdAndSecContext (which are really for SetUserIdAndSecContext,
> but they're in an odd place).
Alright, here's a patch which adds the ability to log the current role's
OID and which calls GetUserIdAndSecContext() directly and handles the
possibility that CurrentUserId isn't valid. Perhaps we should just grab
CurrentUserId directly rather than going through
GetUserIdAndSecContext()? I could certainly do that instead.
Also includes those additional comments in elog.c.
Thanks,
Stephen
commit d9a7acd5ea1f5214b44875b6d257c5c59590167c
Author: Stephen Frost <sfr...@snowman.net>
Date: Wed Jan 12 12:53:50 2011 -0500
Use GetUserIdAndSecContext to get role OID in elog
We can't be sure that GetUserId() will be called when current_user
is a valid Oid, per the comments in GetUserIdAndSecContext, when
called from elog.c, so instead call GetUserIdAndSecContext directly
and handle the possible invalid Oid ourselves.
commit 605497b062298ea195d8999f8cefca10968ae22f
Author: Stephen Frost <sfr...@snowman.net>
Date: Wed Jan 12 12:29:44 2011 -0500
Change to logging role's OID instead of name
Remove the SysCache lookup from elog.c/log_line_prefix by logging
the role's OID instead, this addresses a concern where a SysCache
lookup could malfunction badly due to logging from a failed
transaction. Note that using SysCache from the elog routines could
also be a performance hit, though this would only be the case if a
user chose to enable that logging.
*** 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,3517 ----
</row>
<row>
<entry><literal>%u</literal></entry>
! <entry>User name which was used to authenticate to <productname>PostgreSQL</productname> with</entry>
! <entry>yes</entry>
! </row>
! <row>
! <entry><literal>%o</literal></entry>
! <entry>Current role OID, set via <command>SET ROLE</>, the
! current role is relevant for permission checking, the mapping
! from OID to role can be found in the pg_authid catalog</entry>
<entry>yes</entry>
</row>
<row>
*** 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,
***************
*** 1826,1831 **** log_line_prefix(StringInfo buf, ErrorData *edata)
--- 1835,1852 ----
appendStringInfoString(buf, username);
}
break;
+ case 'o':
+ {
+ Oid curr_role;
+ int curr_sec_context;
+
+ GetUserIdAndSecContext(&curr_role,&curr_sec_context);
+ if (OidIsValid(curr_role))
+ appendStringInfo(buf, "%u", curr_role);
+ else
+ appendStringInfoString(buf, _("[unknown]"));
+ }
+ break;
case 'd':
if (MyProcPort)
{
signature.asc
Description: Digital signature
