On Tue, Dec 12, 2017 at 10:16 PM, Peter Eisentraut < peter.eisentr...@2ndquadrant.com> wrote:
> On 10/12/17 22:18, Stephen Frost wrote: > > I'm generally supportive of this, but I'm not entirely thrilled with how > > this ends up conflating TABLEs and RELATIONs. From the GRANT > > perspective, there's no distinction, and that was clear from the > > language used and how things were handled, but the OBJECT enum has that > > distinction. This change just makes VIEWs be OBJECT_TABLE even though > > they actually aren't tables and there even is an OBJECT_VIEW value. > > This commit may be able to grok that and manage it properly, but later > > hackers might miss that. > > > > I would also suggest that the naming be consistent with the other bits > > of the GRANT system (eg: ACL_ALL_RIGHTS_NAMESPACE would be changed to > > ACL_ALL_RIGHTS_SCHEMA, to match OBJECT_SCHEMA). > > OK, here is a bigger patch set that addresses these issues. I have > added OBJECT_RELATION to reflect the difference between TABLE and > RELATION. I have also renamed NAMESPACE to SCHEMA. And then I got rid > of AclObjectKind as well, because it's just another enum for the same > thing. > > This is now a bit bigger, so I'll put it in the commit fest for detailed > review. > > I quickly look the patch and I liked the v2-0001-Replace-GrantObjectType-with-ObjectType.patch, it's very clean and making things much better. I looked at another patch About v2-0002-Replace-AclObjectKind-with-ObjectType.patch: I noted that no_priv_msg and not_owner_msg array been removed and code fitted the code into aclcheck_error(). Actually that makes the code more complex then what it used to be. I would prefer the array rather then code been fitted into the function. > -- > Peter Eisentraut http://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services > -- Rushabh Lathia
