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 20110112171347.GH4933@tamriel.snowman.net
Whole thread Raw
In response to Re: Add support for logging the current role  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Add support for logging the current role  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Uh, did you actually stop to *think* about this patch?

Actually, I was worried about exactly that, but I didn't see anything at
the top of elog.c that indicated if it'd be a problem or not (and the
Syscache lookup issue was *exactly* what I was looking for). :(  There
was much discussion about recursion and memory commit and whatnot, but
nothing about SysCache lookups.

> What you have just committed puts a syscache lookup into the elog output
> path.  Quite aside from the likely performance hit, this will
> malfunction badly in any case where we're trying to log from an aborted
> transaction.

I had been looking into storing the current role inside the Proc struct
or in some new variable and then pulling it from there (updating it when
needed during a SET ROLE, of course), but it seemed a bit of overkill if
it wasn't necessary (which wasn't obvious to me). We could also just log
the role's OID (%o anyone..?), since that doesn't need a syscache lookup
to get at.  I'd much rather log the role name if we can tho.

I had looked through some of the other calls happening in log_line_prefix
and didn't see any explicit syscache lookups but it seemed like we were
doing quite a few other things that might have issues, so I had assumed
it'd be alright.  Sorry about that.
Thanks,
    Stephen

pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: libpq documentation cleanups (repost 3)
Next
From: Jeff Davis
Date:
Subject: Re: WIP: RangeTypes