Re: Add support for logging the current role

On Thu, Feb 17, 2011 at 4:53 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> It seems there's at least one more thing to worry about here, which is
>> the overhead of this computation when CSV logging is in use.  If no
>> SET ROLE or SET SESSION AUTHORIZATION commands are in use, the code
>> will call show_role(), which will return "none".  We'll then strcmp()
>> that against "none" and decide to call show_session_authorization(),
>> which will call strtoul() to find the comma separator and then return
>> a pointer to the string that follows it.  Now, none of that is
>> enormously expensive, so maybe it's not worth worrying about, but
>> since logging can be a hotspot, I thought I'd mention it and solicit
>> an opinion on whether that's likely to be a problem in practice.
>
> Well, in the first place, going through two not-very-related APIs in
> order to reverse-engineer what miscinit.c already knows is pretty silly
> (not to mention full of possible bugs).  We ought to be looking at the
> GetUserId state directly.
>
> Now you will complain that elog.c mustn't try to map that OID back to
> string form, which is true.  But IIRC, we used to keep the current
> userid stored in both OID and string form.  The string form was removed
> as unnecessary overhead, but maybe it'd be a good idea to put that back.
>
> In short, add a bit of overhead at SetUserId time in order to make this
> cheap (and accurate) in elog.c.

As Stephen says, I think this is utterly impractical; those routines
can't ever throw any kind of error. I was mostly wondering whether we
ought to create a function show_explicitly_set_role() or somesuch that
would do basically the same thing as the proposed code, but try to
avoid the strcmp in the common case where no set role has been done.
I'm not very certain it's worth worrying about, though.
write_csvlog() is not a trivial function as it is.

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