Re: Add support for logging the current role

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
>> I understand. But doing this right is going to take more than ten lines
>> of code, and more than a negligible performance penalty. We have to
>> consider whether it's worth it.

> It'd be ideal if the performance hit could only be felt by people who
> want to enable the option. On the flip side, I don't know that adding a
> bit of extra work to SET ROLE would be that bad. If it helps (and I
> don't know if it does, I'm still trying to wrap my head around
> GetUserIdAndSecContext/SetUserIdAndSecContext), I'd be fine with *not*
> trying to log the right role when inside Security Definter functions
> (after all, if those are getting called, the user could go look at the
> function definition to see which role it's being run as).

> I gather one issue is how we can pick up what the correct role name is
> when resetting the role due to a failed transaction..? Building a stack
> with all the role names pre-cached to deal with that wouldn't be likely
> to work and we'd need more than one level to deal with savepoints, I
> assume? We could reset it to an Invalid name on abort and then detect
> that it needs to be corrected at the start of the next transaction,
> perhaps?

I seem to recall that the assign hook for role stores the string form of
the role name anyway. So in principle you could arrange for that to get
dumped someplace where elog.c could look at it (think about just adding
a string parameter to SetUserIdAndSecContext). It wouldn't track the
effects of RENAME ROLE against an actively-used role, but then again
neither does %u.

I'm not actually concerned about adding a few extra cycles to SET ROLE
for this. 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.

regards, tom lane