Re: Review of GetUserId() Usage

On Sat, Nov 29, 2014 at 3:00 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Stephen Frost (sfrost(at)snowman(dot)net) wrote:
>> * Stephen Frost (sfrost(at)snowman(dot)net) wrote:
>> > Attached is a patch to address the pg_cancel/terminate_backend and the
>> > statistics info as discussed previously. It sounds like we're coming to
>>
>> And I forgot the attachment, of course. Apologies.
>
> Updated patch attached which also changes the error messages (which
> hadn't been updated in the prior versions and really should be).
>
> Barring objections, I plan to move forward with this one and get this
> relatively minor change wrapped up. As mentioned in the commit, while
> this might be an arguably back-patchable change, the lack of field
> complaints and the fact that it changes existing behavior mean it should
> go only against master, imv.

This patch does a couple of different things:

1. It makes more of the crappy error message change that Andres and I
already objected to on the other thread. Whether you disagree with
those objections or not, don't make an end-run around them by putting
more of the same stuff into patches on other threads.

2. It changes the functions in pgstatfuncs.c so that you can see the
relevant information not only for your own role, but also for roles of
which you are a member. That seems fine, but do we need a
documentation change someplace?

3. It messes around with pg_signal_backend(). There are currently no
cases in which pg_signal_backend() throws an error, which is good,
because it lets you write queries against pg_stat_activity() that
don't fail halfway through, even if you are missing permissions on
some things. This patch introduces such a case, which is bad.

I think it's unfathomable that you would consider anything in this
patch a back-patchable bug fix. It's clearly a straight-up behavior
change... or more properly three different changes, only one of which
I agree with.

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