Jeevan,
* Jeevan Chalke (jeevan.cha...@gmail.com) wrote:
> The following review has been posted through the commitfest application:
> make installcheck-world: tested, passed
> Implements feature: tested, passed
> Spec compliant: tested, passed
> Documentation:tested, p
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation:tested, passed
I have reviewed the patch.
Patch is excellent in shape and do
On Fri, Dec 5, 2014 at 11:28 PM, Stephen Frost wrote:
> * Stephen Frost (sfr...@snowman.net) wrote:
> > * Stephen Frost (sfr...@snowman.net) wrote:
> > > > 3. It messes around with pg_signal_backend(). There are currently no
> > > > cases in which pg_signal_backend() throws an error, which is go
* Andres Freund (and...@2ndquadrant.com) wrote:
> Is the 'Only allow superusers to signal superuser-owned backends' check
> actually safe that way? I personally try to never use a superuser role
> as the login user, but grant my account a superuser role that doesn't
> inherit. But IIRC PGPROC->role
On 2014-12-05 09:28:13 -0500, Stephen Frost wrote:
> static int
> pg_signal_backend(int pid, int sig)
> {
> @@ -113,7 +117,12 @@ pg_signal_backend(int pid, int sig)
> return SIGNAL_BACKEND_ERROR;
> }
>
> - if (!(superuser() || proc->roleId == GetUserId()))
> + /* On
* Stephen Frost (sfr...@snowman.net) wrote:
> * Stephen Frost (sfr...@snowman.net) wrote:
> > > 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_acti
On Tue, Dec 2, 2014 at 2:50 PM, Stephen Frost wrote:
>> 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 in
* Stephen Frost (sfr...@snowman.net) wrote:
> > 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
Robert,
* Robert Haas (robertmh...@gmail.com) wrote:
> On Sat, Nov 29, 2014 at 3:00 PM, Stephen Frost wrote:
> > * Stephen Frost (sfr...@snowman.net) wrote:
> > Updated patch attached which also changes the error messages (which
> > hadn't been updated in the prior versions and really should be).
On Sat, Nov 29, 2014 at 3:00 PM, Stephen Frost wrote:
> * Stephen Frost (sfr...@snowman.net) wrote:
>> * Stephen Frost (sfr...@snowman.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
All,
* Stephen Frost (sfr...@snowman.net) wrote:
> * Stephen Frost (sfr...@snowman.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. Apolo
All,
* Peter Eisentraut (pete...@gmx.net) wrote:
> It would be weird if it were inconsistent: some things require role
> membership, some things require SET ROLE. Try explaining that.
Attached is a patch to address the pg_cancel/terminate_backend and the
statistics info as discussed previously.
* Stephen Frost (sfr...@snowman.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.
Thanks,
Stephen
diff --git a
All,
> I'll break them into three pieces- superuser() cleanup, GetUserId() ->
> has_privs_of_role(), and the additional-role-attributes patch will just
> depend on the others.
>
Attached is a patch for the GetUserId() -> has_privs_of_role() cleanup for
review.
-Adam
--
Adam Brightwell - adam.
>
> I'll break them into three pieces- superuser() cleanup, GetUserId() ->
> has_privs_of_role(), and the additional-role-attributes patch will just
> depend on the others.
>
The superuser() cleanup has already been submitted to the current
commitfest.
https://commitfest.postgresql.org/action/pat
* Robert Haas (robertmh...@gmail.com) wrote:
> I'm not sure what your point is. Whether keeping changes separate is
> easy or hard, and whether things overlap with multiple other things or
> just one, we need to make the effort to do it.
What I was getting at is that the role attributes patch wou
On Thu, Oct 16, 2014 at 11:28 AM, Stephen Frost wrote:
> On Thursday, October 16, 2014, Robert Haas wrote:
>>
>> On Thu, Oct 16, 2014 at 9:49 AM, Stephen Frost wrote:
>> > As a side-note, this change is included in the 'role attributes' patch.
>>
>> It's really important that we keep separate ch
Robert,
On Thursday, October 16, 2014, Robert Haas wrote:
> On Thu, Oct 16, 2014 at 9:49 AM, Stephen Frost > wrote:
> > As a side-note, this change is included in the 'role attributes' patch.
>
> It's really important that we keep separate changes in separate
> patches that are committed in sep
On Thu, Oct 16, 2014 at 9:49 AM, Stephen Frost wrote:
> As a side-note, this change is included in the 'role attributes' patch.
It's really important that we keep separate changes in separate
patches that are committed in separate commits. Otherwise, it gets
really confusing.
--
Robert Haas
En
* Peter Eisentraut (pete...@gmx.net) wrote:
> On 9/24/14 4:58 PM, Stephen Frost wrote:
> > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> >> I think the case for pgstat_get_backend_current_activity() and
> >> pg_stat_get_activity and the other pgstatfuncs.c callers is easy to make
> >> and se
* Peter Eisentraut (pete...@gmx.net) wrote:
> On 9/24/14 4:58 PM, Stephen Frost wrote:
> > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> >> I think the case for pgstat_get_backend_current_activity() and
> >> pg_stat_get_activity and the other pgstatfuncs.c callers is easy to make
> >> and se
On 9/24/14 4:58 PM, Stephen Frost wrote:
> Alvaro,
>
> * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
>> I think the case for pgstat_get_backend_current_activity() and
>> pg_stat_get_activity and the other pgstatfuncs.c callers is easy to make
>> and seems acceptable to me; but I would leave p
Alvaro,
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> I think the case for pgstat_get_backend_current_activity() and
> pg_stat_get_activity and the other pgstatfuncs.c callers is easy to make
> and seems acceptable to me; but I would leave pg_signal_backend out of
> that discussion, because
Stephen Frost wrote:
> pgstat_get_backend_current_activity()
> Used to decide if the current activity string should be returned or
> not. In my view, this is a clear case which should be addressed
> through has_privs_of_role() instead of requiring the user to
> SET ROLE to each
Greetings,
While looking through the GetUserId() usage in the backend, I couldn't
help but notice a few rather questionable cases that, in my view,
should be cleaned up.
As a reminder, GetUserId() returns the OID of the user we are
generally operating as (eg: whatever the 'role' is, as
25 matches
Mail list logo
