Re: [HACKERS] Inconsistent syntax in GRANT

Updated patch applied. I decided Tom was right to just ignore invalid
sequence permission from pre-8.2 databases, rather than try to use GRANT
TABLE; there was no reason to do it and avoiding it made the code
cleaner and more robust.

The changes were:

Add GRANT ON SEQUENCE syntax to support sequence-only permissions.
Continue to support GRANT ON [TABLE] for sequences for backward
compatibility; issue warning for invalid sequence permissions.

[ Backward compatibility warning message.]

Add USAGE permission for sequences that allows only currval() and
nextval(), not setval().

Mention object name in grant/revoke warnings because of possible
multi-object operations.

---------------------------------------------------------------------------

Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > > Tom Lane wrote:
> > >> Just ignore the inapplicable permissions during pg_dump. I think you're
> > >> making this harder than it needs to be...
> >
> > > check all permission bits
> > > call object-type-specific routine
> > > loop over each object and set permission bits
> >
> > > so, to fix this, I would need to move the permission bit checks into
> > > object-type-specific routines so that I could check the permission bits
> > > for each object, rather than once in a single place.
> >
> > You'd have to allow the union of relation and sequence rights during the
> > conversion to bitmask form in ExecuteGrantStmt, and then check more
> > closely inside the per-object loop in ExecGrant_Relation, but that
> > doesn't seem like a showstopper to me. It certainly seems more pleasant
> > than exposing bizarre restrictions to users because we're sharing code
> > between the cases.
>
> Your idea of using a union of permission bits was very helpful. I was
> afraid I was going to have to loop over every permission bit again in
> the table/sequence grant permission code, but the union allowed for a
> very simple check in that code.
>
> It allows for better code checks and I think it behaves as expected:
>
> test=> CREATE TABLE tab(x INTEGER);
> CREATE TABLE
> test=> CREATE SEQUENCE seq;
> CREATE SEQUENCE
> test=> GRANT ALL ON seq, tab TO PUBLIC;
> GRANT
> test=> REVOKE USAGE ON seq, tab FROM PUBLIC;
> ERROR: invalid privilege type USAGE for table
> test=> REVOKE SELECT ON seq, tab FROM PUBLIC;
> REVOKE
> test=> REVOKE DELETE ON seq, tab FROM PUBLIC;
> WARNING: sequence "seq" only supports USAGE, SELECT, AND UPDATE
> WARNING: no privileges could be revoked for "seq"
> REVOKE
>
> and pg_dump has:
>
> GRANT USAGE,UPDATE ON SEQUENCE x TO PUBLIC;
>
> GRANT INSERT,RULE,UPDATE,REFERENCES,TRIGGER ON TABLE xx TO PUBLIC;
>
> Note I had to add the object name to the warning message so it is clear
> which object permission changes did succeed. I have updated the patch.
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073