Peter, * Peter Eisentraut (peter.eisentr...@2ndquadrant.com) wrote: > 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.
I agree that we won't be able to stop allowing GRANT to accept various object types without an explicit type being declared. What might actually be nice is if we'd allow *more* things to be specified that way rather than trying to tighten it up- I can't count on all my fingers and toes the number of times I've done 'grant usage on myschema to joe;' and been most annoyed that it didn't work. Supporting that today would involve making a 'relation-or-schema' thing in the object system because we're forcing ourselves to call whatever is passed in to GRANT a certain kind of object before we've really figured out what kind of object it is, and that's what I'm objecting to here. I don't want to bend the object address system to handle that either, and so I agree that it'd be better to have the GRANT code deal with the ambiguity directly. > > 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. I didn't actually suggest having an OBJECT_RELATION- I complained that you were stamping 'OBJECT_TABLE' on things that definitely weren't tables and that you were conflating tables and relations. I'm afraid that I wasn't terribly clear on a path forward two months ago because I didn't really have any great ideas on how to fix that while avoiding having overlapping object types, which I do think is something we should try to avoid. > > 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. Yes, documenting it, at least, is necessary if we're going to go with this approach, though I wonder if that will end up making it difficult to remove it later if someone gets around to reworking the GRANT system to directly deal with this ambiguity without needing a special-case in the object address system for it. I guess one question coming out of this is- do you see another use for this OBJECT_RELATION object type..? If it's really only this one special case in GRANT that needs it then I think that makes it much less appealing to have. Thanks! Stephen
signature.asc
Description: Digital signature
