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
