2009/12/22 KaiGai Kohei <kai...@ak.jp.nec.com>: > (2009/12/21 12:53), Robert Haas wrote: >> On Thu, Dec 17, 2009 at 7:19 PM, Tom Lane<t...@sss.pgh.pa.us> wrote: >>> KaiGai Kohei<kai...@ak.jp.nec.com> writes: >>>> [ patch to remove EnableDisableRule's permissions check ] >>> >>> I don't particularly like this patch, mainly because I disagree with >>> randomly removing permissions checks without any sort of plan about >>> where they ought to go. >> >> So where ought they to go? >> >>> If we're going to start moving these checks around we need a very >>> well-defined notion of where permissions checks should be made, so that >>> everyone knows what to expect. I have not seen any plan for that. >> >> This seems to me to get right the heart of the matter. When I >> submitted my machine-readable explain patch, you critiqued it for >> implementing half of an abstraction layer, and it seems to me that our >> current permissions-checking logic has precisely the same issue. We >> consistently write code that starts by checking permissions and then >> moves right along to implementing the action. Those two things need >> to be severed. I see two ways to do this. Given a function that (A) >> does some prep work, (B) checks permissions, and (C) performs the >> action, we could either: >> >> 1. Make the existing function do (A) and (B) and then call another >> function to do (C), or >> 2. Make the existing function do (A), call another function to do (B), >> and then do (C) itself. >> >> I'm not sure which will work better, but I think making a decision >> about which way to do it and how to name the functions would be a big >> step towards having a coherent plan for this project. > > We have mixed policy in the current implementation. > > The point is what database object shall be handled in this function. > > If we consider a rewrite rule as a database object, not a property of > the relation, it seems to me a correct manner to apply permission checks > in the EnableDisableRule(), because it handles a given rewrite rule. > > If we consider a rewrite rule as a property of a relation, not an independent > database object, it seems to me we should apply permission checks in > ATPrepCmd() > which handles a relation, rather than EnableDisableRule(). > > My patch stands on the later perspective, because pg_rewrite entries don't > have its own ownership and access privileges, and it is always owned by > a certain relation.
That's somewhat separate from the point I was making, but it's a good point all the same. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
