On 12/20/17 22:01, Stephen Frost wrote: > There's some downsides to this approach though: we do an initial set of > checks in ExecGrantStmt, but we can't do all of them because we don't > know if it's a sequence or not, so we end up with some additional > special checks to see if the GRANT is valid down in ExecGrant_Relation > after we figure out what kind of relation it is.
I think that we allow a sequence to be treated like a table in GRANT (and other places) is a historical wart that we won't easily be able to get rid of. I don't think the object address system should be bent to accommodate that. I'd rather see the warts in the code explicitly. > Further, anything which handles an OBJECT_TABLE or OBJECT_VIEW or > OBJECT_SEQUENCE today might be expected to now be able to handle an > OBJECT_RELATION instead, though it doesn't look like this patch makes > any attempt to address that. To some extent 0002 does that. > In the end, I'd personally prefer refactoring ExecGrantStmt and friends > to move the GRANT-type checks down into the individual functions and, > ideally, avoid having to have OBJECT_RELATION at all, though that seems > like it'd be a good bit of work. I'm not sure I follow that, since it appears to be the opposite of what you said earlier, i.e., we should have OBJECT_RELATION so as to avoid using OBJECT_TABLE when we don't really know yet whether something is a table. > My second preference would be to at least add some commentary about > OBJECT_RELATION vs. OBJECT_(TABLE|VIEW|SEQUENCE), when which should be > used and why, and review functions that accept objects of those types > to see if they should be extended to also accept OBJECT_RELATION. I can look into that. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
