On Sun, Dec 20, 2009 at 10:53 PM, I 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?
I have looked this over a little bit and I guess I don't see why the lack of a grand plan for how to organize all of our permissions checks ought to keep us from removing this one on the grounds of redundancy. We have to attack this problem in small pieces if we're going to make any progress, and the pieces aren't going to get any smaller than this. Per Tom's suggestion, I did a quick grep of the Slony source code for EnableDisableRule and got no hits. I think the appropriate level of concern about the possibility that third-party code might be calling this function is to (1) add a comment to the function noting that it is the caller's responsibility to check permissions, and (2) if we're *really* concerned that someone might miss that, release-note it. It is very unclear that it's worth even that much effort, though, since we have zero evidence that there is any third-party code using this function directly. A quick Google search on EnableDisableRule hits mostly this thread. With respect to cleaning up permissions more generally, it seems to me that Stephen Frost hit the nail on the upthread when he remarked that the current coding, where we're expecting certain functions in tablecmds.c to be called with PART of the permissions checking done, is fairly counterintuitive. I think it would be interesting to see if someone could propose a refactoring that eliminates this and puts all the permissions checking in a single, appropriate location. But ISTM that this patch would be a subset of any such more comprehensive patch, so that doesn't seem like a reason to hold off on applying this either. So I am inclined to go ahead and apply this with the addition of the comment discussed above. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
