Re: [HACKERS] replace GrantObjectType with ObjectType

Peter,

* Peter Eisentraut (peter(dot)eisentraut(at)2ndquadrant(dot)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