Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

2010-07-11 Thread KaiGai Kohei
(2010/07/10 2:12), Simon Riggs wrote: > On Fri, 2010-07-09 at 11:09 -0400, Stephen Frost wrote: >>> Strangely, I was looking into removing the ExecCheckRTPerms check >>> altogether by forcing plan invalidation when permissions are >> updated. >>> That would be a performance tweak that would render

Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

2010-07-11 Thread Robert Haas
On Jul 11, 2010, at 10:44 AM, Tom Lane wrote: > Simon Riggs writes: >> On Fri, 2010-07-09 at 17:21 -0400, Tom Lane wrote: >>> I'd still want to see some evidence showing that it's worth >>> troubling over though. Premature optimization being the root of all >>> evil, and all that. (In this case

Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

2010-07-11 Thread Tom Lane
Simon Riggs writes: > On Fri, 2010-07-09 at 17:21 -0400, Tom Lane wrote: >> I'd still want to see some evidence showing that it's worth >> troubling over though. Premature optimization being the root of all >> evil, and all that. (In this case, the hazard we expose ourselves to >> seems to be se

Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

2010-07-11 Thread Simon Riggs
On Fri, 2010-07-09 at 17:21 -0400, Tom Lane wrote: > Simon Riggs writes: > > On Fri, 2010-07-09 at 14:01 -0400, Tom Lane wrote: > >> Consider PREPARE followed only later by EXECUTE. Your proposal would > >> make the PREPARE fail outright, when it currently does not. > > > Just to avoid wasted in

Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

2010-07-09 Thread Tom Lane
Simon Riggs writes: > On Fri, 2010-07-09 at 14:01 -0400, Tom Lane wrote: >> Consider PREPARE followed only later by EXECUTE. Your proposal would >> make the PREPARE fail outright, when it currently does not. > Just to avoid wasted investigation: are you saying that is important > behaviour that

Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

2010-07-09 Thread Simon Riggs
On Fri, 2010-07-09 at 14:01 -0400, Tom Lane wrote: > Simon Riggs writes: > > On Fri, 2010-07-09 at 13:21 -0400, Tom Lane wrote: > >> Except that it *is* a change in behavior: the first check will occur > >> too soon. > > > Sooner matters why? > > Consider PREPARE followed only later by EXECUTE.

Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

2010-07-09 Thread Tom Lane
Simon Riggs writes: > On Fri, 2010-07-09 at 13:21 -0400, Tom Lane wrote: >> Except that it *is* a change in behavior: the first check will occur >> too soon. > Sooner matters why? Consider PREPARE followed only later by EXECUTE. Your proposal would make the PREPARE fail outright, when it curren

Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

2010-07-09 Thread Stephen Frost
Robert, * Robert Haas (robertmh...@gmail.com) wrote: > This is certainly true, but I also wonder what SE-PostgreSQL plans to > do about this. Taking this to its logical exteme, the system security > policy could change in mid-query - and while you'd like to think that > the system would stop emit

Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

2010-07-09 Thread Simon Riggs
On Fri, 2010-07-09 at 13:21 -0400, Tom Lane wrote: > > Agreed that permission checks should logically be applied at > execution > > time. I am proposing a performance optimisation, not a change in > > behaviour. > > Except that it *is* a change in behavior: the first check will occur > too > soon.

Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

2010-07-09 Thread Robert Haas
On Fri, Jul 9, 2010 at 1:21 PM, Tom Lane wrote: > Simon Riggs writes: >> On Fri, 2010-07-09 at 11:09 -0400, Tom Lane wrote: >>> Simon Riggs writes: Strangely, I was looking into removing the ExecCheckRTPerms check altogether by forcing plan invalidation when permissions are updated. >>

Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

2010-07-09 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote: > Simon Riggs writes: > > Agreed that permission checks should logically be applied at execution > > time. I am proposing a performance optimisation, not a change in > > behaviour. > > Except that it *is* a change in behavior: the first check will occur too

Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

2010-07-09 Thread Tom Lane
Simon Riggs writes: > On Fri, 2010-07-09 at 11:09 -0400, Tom Lane wrote: >> Simon Riggs writes: >>> Strangely, I was looking into removing the ExecCheckRTPerms check >>> altogether by forcing plan invalidation when permissions are updated. >>> That would be a performance tweak that would render t

Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

2010-07-09 Thread Stephen Frost
Simon, * Simon Riggs (si...@2ndquadrant.com) wrote: > With respect, there doesn't seem to be much use case anyway. I'm sorry > to be expressing that opinion now; been away for a while. I am somewhat > amazed that Tom isn't dancing on your head for proposing it though. I believe it's because we've

Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

2010-07-09 Thread Simon Riggs
On Fri, 2010-07-09 at 11:09 -0400, Stephen Frost wrote: > > Strangely, I was looking into removing the ExecCheckRTPerms check > > altogether by forcing plan invalidation when permissions are > updated. > > That would be a performance tweak that would render this change > useless. > > I don't see h

Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

2010-07-09 Thread Simon Riggs
On Fri, 2010-07-09 at 11:07 -0400, Robert Haas wrote: > On Fri, Jul 9, 2010 at 10:51 AM, Simon Riggs > wrote: > > The loadable module doesn't "gain control" here it simplify kicks-in > > after, and in addition to, normal checking. That just means you have > the > > option of failing for additional

Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

2010-07-09 Thread Simon Riggs
On Fri, 2010-07-09 at 11:09 -0400, Tom Lane wrote: > Simon Riggs writes: > > Strangely, I was looking into removing the ExecCheckRTPerms check > > altogether by forcing plan invalidation when permissions are updated. > > That would be a performance tweak that would render this change useless. > >

Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

2010-07-09 Thread Simon Riggs
On Fri, 2010-07-09 at 11:07 -0400, Robert Haas wrote: > > Strangely, I was looking into removing the ExecCheckRTPerms check > > altogether by forcing plan invalidation when permissions are updated. > > That would be a performance tweak that would render this change useless. > > Huh. Obviously, I

Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

2010-07-09 Thread Tom Lane
Simon Riggs writes: > Strangely, I was looking into removing the ExecCheckRTPerms check > altogether by forcing plan invalidation when permissions are updated. > That would be a performance tweak that would render this change useless. That seems both pointless and wrong. Permissions checks shoul

Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

2010-07-09 Thread Stephen Frost
* Simon Riggs (si...@2ndquadrant.com) wrote: > The loadable module doesn't "gain control" here it simplify kicks-in > after, and in addition to, normal checking. That just means you have the > option of failing for additional reasons. Right, additional checks (such as the label) can be done. > We

Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

2010-07-09 Thread Robert Haas
On Fri, Jul 9, 2010 at 10:51 AM, Simon Riggs wrote: > The loadable module doesn't "gain control" here it simplify kicks-in > after, and in addition to, normal checking. That just means you have the > option of failing for additional reasons. True. We could change it so that the normal checking i

Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

2010-07-09 Thread Simon Riggs
On Fri, 2010-07-09 at 14:06 +, Robert Haas wrote: > Log Message: > --- > Add a hook in ExecCheckRTPerms(). > > This hook allows a loadable module to gain control when table permissions > are checked. It is expected to be used by an eventual SE-PostgreSQL > implementation, but there ar