On Sat, Jan 23, 2010 at 2:17 AM, KaiGai Kohei <kai...@kaigai.gr.jp> wrote: > However, it is unclear for me whether the revised ATSimplePermissions() > provide cleaner code than currently we have, because it also needs > a big switch ... case statement within. > > Am I misunderstanding something?
Well, not everyone is going to agree on what "cleaner code" means in every case, but the reason that I like my design better is because it moves all of the decision making out of ATPrepCmd() into ATSimplePermissions(). What you're proposing would mean that ATPrepCmd() would basically continue to know everything about which operations need which permissions checks, which I don't think is going to scale very well to alternative security providers, if we eventually decide to support such a thing. >> It may also be worth refactoring is ATAddCheckConstraint(), which >> currently does its own recursion only so that the constraint name at >> the top of the inheritance hierarchy propagates all the way down >> unchanged. I wonder if it would be cleaner/possible to work out the >> constraint name earlier and then just use the standard recursion >> mechanism. > > Isn't it possible to check whether the given constraint is CHECK() > or FOREIGN KEY in the ATPrepCmd() stage, and assign individual > cmd->subtype? If CONSTR_FOREIGN, it will never recursion. > > In this case, we can apply checks in ATPrepCmd() stage for CONSTR_CHECK. I don't understand what you're saying here. > It is a good idea to consolidate permission check into tablecmd.c from > various kind of core routines. > And, it also makes the basis clear. The permission check should be applied > after the code path gathered enough information to make its access control > decision as early as possible. And just as importantly, in a consistent place. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
