Re: [PATCHES] Roles - SET ROLE Updated
Stephen Frost [EMAIL PROTECTED] writes: Here's a much better version of the SET ROLE work. I'm reasonably happy with it. The only parts I don't like are that I had to do some ugly things in gram.y to avoid making NONE reserved, and I can't seem to see how to avoid having ROLE be reserved (I understand it was reserved in SQL99 but not in SQL2003...). Updated yet again, fixing a bug in the prior one that caused it to not work properly, and some additional things: I don't think this patch works; it certainly doesn't do what I'd expect to happen with SECURITY DEFINER functions. At the very least you'd need to make fmgr_security_definer save/restore the current role setting. But I doubt that this is even the direction we want to head in. After rereading SQL99 4.31, I don't think there is any need to distinguish CURRENT_USER from CURRENT_ROLE, mainly because our implementation does not distinguish users from roles at all. (Which I think is good.) So ISTM we should not change GetUserId() as you propose, but leave it alone and implement SetRole approximately like SetSessionUserId is implemented, ie, setting a background value that may sometimes get copied into CurrentUserId. The stack aspect only matters to the extent that SetRoleId has precedence over SetSessionUserId for determining the outside-a-transaction value of CurrentUserId. regards, tom lane ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Roles - SET ROLE Updated
Stephen Frost [EMAIL PROTECTED] writes: * Tom Lane ([EMAIL PROTECTED]) wrote: After rereading SQL99 4.31, I don't think there is any need to distinguish CURRENT_USER from CURRENT_ROLE, mainly because our implementation does not distinguish users from roles at all. CURRENT_USER and CURRENT_ROLE can have different values, as I understand SQL2003, and there are places where one is used instead of the other It's possible for CURRENT_ROLE to be null according to the spec; if you like we could implement that as returning what the current outer-level SET ROLE value is (which would then make it semantically more like SESSION_USER than CURRENT_USER). I don't think CURRENT_USER should ever be allowed to be null, or to be different from the active authorization identifier, first because it's silly and second because it will break existing applications that depend on CURRENT_USER for authorization checking. Given that we don't really distinguish users and roles, I would be inclined to make the same argument for CURRENT_ROLE too, leaving SHOW ROLE (and its function equivalent) as the only way to see what you SET ROLE to. But it's less likely to break existing apps if we don't. (such as with the 'grantor' in grants, according to SQL2003 the 'grantor' should be the CURRENT_USER, regardless of if CURRENT_ROLE is set or not). Exactly. CURRENT_USER has to be the active authorization identifier. Do you want me to rework the patch along these lines or are you already working on it? I'm working on it ... regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Roles - SET ROLE Updated
BTW, I realized we do not support granting roles to PUBLIC: regression=# create role r; CREATE ROLE regression=# grant r to public; ERROR: role public does not exist but as far as I can tell SQL99 expects this to work. regards, tom lane ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Roles - SET ROLE Updated
* Tom Lane ([EMAIL PROTECTED]) wrote: BTW, I realized we do not support granting roles to PUBLIC: regression=# create role r; CREATE ROLE regression=# grant r to public; ERROR: role public does not exist but as far as I can tell SQL99 expects this to work. Indeed, I believe you're correct, sorry about missing that. Stephen signature.asc Description: Digital signature
Re: [PATCHES] Roles - SET ROLE Updated
Stephen Frost [EMAIL PROTECTED] writes: * Tom Lane ([EMAIL PROTECTED]) wrote: BTW, I realized we do not support granting roles to PUBLIC: regression=# create role r; CREATE ROLE regression=# grant r to public; ERROR: role public does not exist but as far as I can tell SQL99 expects this to work. Indeed, I believe you're correct, sorry about missing that. However, on second thought I'm not sure that this is sensible anyway. Consider that every role is implicitly a member of PUBLIC --- so isn't the above a creation of a circular membership loop, which is (for good reason) forbidden by the spec? regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Roles - SET ROLE Updated
Another issue: I like the has_role() function and in fact think it needs to come in multiple variants just like has_table_privilege and friends: has_role(name, name) has_role(name, oid) has_role(oid, name) has_role(oid, oid) has_role(name) -- implicitly has_role(current_user, ...) has_role(oid) However I'm a bit dubious about whether has_role isn't an invasion of application namespace. pg_has_role would be better, but we have the (mis) precedent of has_table_privilege. What do you think about calling it has_role_privilege? regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Roles - SET ROLE Updated
* Tom Lane ([EMAIL PROTECTED]) wrote: Another issue: I like the has_role() function and in fact think it needs to come in multiple variants just like has_table_privilege and friends: has_role(name, name) has_role(name, oid) has_role(oid, name) has_role(oid, oid) has_role(name) -- implicitly has_role(current_user, ...) has_role(oid) However I'm a bit dubious about whether has_role isn't an invasion of application namespace. pg_has_role would be better, but we have the (mis) precedent of has_table_privilege. What do you think about calling it has_role_privilege? I thought about that originally. It seemed a bit long to me and I felt that having the 'privilege' of a role wasn't quite the same as having a 'role', but honestly I'm not terribly picky and on reflection a role *is* like other objects in the catalog (I originally hadn't considered it such), so, that's fine with me... has_role() was another reason I was thinking about having a seperate function for 'is_member_of_role' which didn't pollute the cache, just a side-note. Thanks, Stephen signature.asc Description: Digital signature
