Thread: Re: [HACKERS] Inconsistent syntax in GRANT
Josh Berkus wrote: > Folks, > > Just got tripped up by this: > > GRANT SELECT ON table1 TO someuser; > GRANT SELECT ON table1_id_seq TO someuser; > .... both work > > However, > GRANT SELECT ON TABLE table1 TO someuser; > ... works, while .... > GRANT SELECT ON SEQUENCE table1_id_seq TO someuser; > ... raises an error. > > This is inconsistent. Do people agree with me that the parser should > accept "SEQUENCE" there, since the optional object name works for all > other objects? Is there some technical reason this is difficult to do? The following patch allows VIEW and SEQUENCE for GRANT. I didn't add checks for relkind, figuring it wasn't worth it, right? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073 Index: doc/src/sgml/ref/grant.sgml =================================================================== RCS file: /cvsroot/pgsql/doc/src/sgml/ref/grant.sgml,v retrieving revision 1.50 diff -c -c -r1.50 grant.sgml *** doc/src/sgml/ref/grant.sgml 20 Oct 2005 19:18:01 -0000 1.50 --- doc/src/sgml/ref/grant.sgml 5 Jan 2006 18:18:37 -0000 *************** *** 22,28 **** <synopsis> GRANT { { SELECT | INSERT | UPDATE | DELETE | RULE | REFERENCES | TRIGGER } [,...] | ALL [ PRIVILEGES ] } ! ON [ TABLE ] <replaceable class="PARAMETER">tablename</replaceable> [, ...] TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ] GRANT { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] } --- 22,28 ---- <synopsis> GRANT { { SELECT | INSERT | UPDATE | DELETE | RULE | REFERENCES | TRIGGER } [,...] | ALL [ PRIVILEGES ] } ! ON [ TABLE | VIEW | SEQUENCE ] <replaceable class="PARAMETER">tablename</replaceable> [, ...] TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ] GRANT { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] } Index: src/backend/parser/gram.y =================================================================== RCS file: /cvsroot/pgsql/src/backend/parser/gram.y,v retrieving revision 2.521 diff -c -c -r2.521 gram.y *** src/backend/parser/gram.y 29 Dec 2005 04:53:18 -0000 2.521 --- src/backend/parser/gram.y 5 Jan 2006 18:18:41 -0000 *************** *** 3315,3320 **** --- 3315,3321 ---- n->objs = $1; $$ = n; } + /* The next three are processed identically. */ | TABLE qualified_name_list { PrivTarget *n = makeNode(PrivTarget); *************** *** 3322,3327 **** --- 3323,3342 ---- n->objs = $2; $$ = n; } + | VIEW qualified_name_list + { + PrivTarget *n = makeNode(PrivTarget); + n->objtype = ACL_OBJECT_RELATION; + n->objs = $2; + $$ = n; + } + | SEQUENCE qualified_name_list + { + PrivTarget *n = makeNode(PrivTarget); + n->objtype = ACL_OBJECT_RELATION; + n->objs = $2; + $$ = n; + } | FUNCTION function_with_argtypes_list { PrivTarget *n = makeNode(PrivTarget);
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> The following patch allows VIEW and SEQUENCE for GRANT. I didn't add
> checks for relkind, figuring it wasn't worth it, right?
The permissions for a sequence aren't the same as they are for a table.
We've sort of ignored the point to date, but if we're going to add
special syntax for granting on a sequence, I don't think we should
continue to ignore it.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > The following patch allows VIEW and SEQUENCE for GRANT. I didn't add > > checks for relkind, figuring it wasn't worth it, right? > > The permissions for a sequence aren't the same as they are for a table. > We've sort of ignored the point to date, but if we're going to add > special syntax for granting on a sequence, I don't think we should > continue to ignore it. Uh, how are they different? You mean just UPDATE and none of the others do anything? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce, Tom, > > The permissions for a sequence aren't the same as they are for a > > table. We've sort of ignored the point to date, but if we're going to > > add special syntax for granting on a sequence, I don't think we should > > continue to ignore it. > > Uh, how are they different? You mean just UPDATE and none of the > others do anything? Yes, it would be nice to have real permissions for sequences, specifically USE (which allows nextval() and currval()) and UPDATE (which would allow setval() ). However, I don't know that the added functionality would justify breaking backwards-compatibility. Oh, and Bruce, I can't imagine needing specific relkind so I think that part's fine. -- --Josh Josh Berkus Aglio Database Solutions San Francisco
Bruce Momjian wrote: > The following patch allows VIEW and SEQUENCE for GRANT. I didn't add > checks for relkind, figuring it wasn't worth it, right? I think checking the relkind is pretty reasonable, and should require only a few lines of code -- why not do it? -Neil
Josh Berkus <josh@agliodbs.com> writes:
>> Uh, how are they different? You mean just UPDATE and none of the
>> others do anything?
> Yes, it would be nice to have real permissions for sequences, specifically
> USE (which allows nextval() and currval()) and UPDATE (which would allow
> setval() ). However, I don't know that the added functionality would
> justify breaking backwards-compatibility.
We could maintain backwards compatibility by continuing to accept the
old equivalences when you say GRANT ON TABLE. But when you say GRANT ON
SEQUENCE, I think it should use sequence-specific privilege keywords,
and not allow the privileges that don't mean anything for sequences,
like DELETE.
I'm not sure offhand what keywords we'd want to use, but now is the time
to look at it, *before* it becomes set in stone that GRANT ON SEQUENCE
is just another spelling of GRANT ON TABLE.
(The subtext of this is that I don't have a lot of use for allowing
variant syntaxes that don't actually do anything different ...)
regards, tom lane
On Thu, Jan 05, 2006 at 11:44:24 -0800, Josh Berkus <josh@agliodbs.com> wrote: > Bruce, Tom, > > > > The permissions for a sequence aren't the same as they are for a > > > table. We've sort of ignored the point to date, but if we're going to > > > add special syntax for granting on a sequence, I don't think we should > > > continue to ignore it. > > > > Uh, how are they different? You mean just UPDATE and none of the > > others do anything? > > Yes, it would be nice to have real permissions for sequences, specifically > USE (which allows nextval() and currval()) and UPDATE (which would allow > setval() ). However, I don't know that the added functionality would > justify breaking backwards-compatibility. It might be nice to split nextval and currval access as well. nextval access corresponds to INSERT and currval access to SELECT.
Bruno Wolff III wrote: > On Thu, Jan 05, 2006 at 11:44:24 -0800, > Josh Berkus <josh@agliodbs.com> wrote: > > Bruce, Tom, > > > > > > The permissions for a sequence aren't the same as they are for a > > > > table. We've sort of ignored the point to date, but if we're going to > > > > add special syntax for granting on a sequence, I don't think we should > > > > continue to ignore it. > > > > > > Uh, how are they different? You mean just UPDATE and none of the > > > others do anything? > > > > Yes, it would be nice to have real permissions for sequences, specifically > > USE (which allows nextval() and currval()) and UPDATE (which would allow > > setval() ). However, I don't know that the added functionality would > > justify breaking backwards-compatibility. > > It might be nice to split nextval and currval access as well. nextval access > corresponds to INSERT and currval access to SELECT. Uh, that is already in the code. nextval()/setval() is UPDATE, and currval() is SELECT. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
On 1/6/06, Bruce Momjian <pgman@candle.pha.pa.us> wrote: > Bruno Wolff III wrote: > > It might be nice to split nextval and currval access as well. nextval access > > corresponds to INSERT and currval access to SELECT. > > Uh, that is already in the code. nextval()/setval() is UPDATE, and > currval() is SELECT. This seems weird. Shouldn't nextval/currval go together and setval separately? Considering there's no currval() without nextval(), what point is disallowing currval() when user is able to call nextval()? I rather want to allow nextval/currval and disable setval as it allows regular user to DoS the database. -- marko [removing Tom from CC as he bounces gmail]
Tom Lane wrote: > Josh Berkus <josh@agliodbs.com> writes: > >> Uh, how are they different? You mean just UPDATE and none of the > >> others do anything? > > > Yes, it would be nice to have real permissions for sequences, specifically > > USE (which allows nextval() and currval()) and UPDATE (which would allow > > setval() ). However, I don't know that the added functionality would > > justify breaking backwards-compatibility. > > We could maintain backwards compatibility by continuing to accept the > old equivalences when you say GRANT ON TABLE. But when you say GRANT ON > SEQUENCE, I think it should use sequence-specific privilege keywords, > and not allow the privileges that don't mean anything for sequences, > like DELETE. OK. > I'm not sure offhand what keywords we'd want to use, but now is the time > to look at it, *before* it becomes set in stone that GRANT ON SEQUENCE > is just another spelling of GRANT ON TABLE. Sequences do not support INSERT, UPDATE, or DELETE, but we overload UPDATE to control nextval()/setval(), so I just allowed SELECT and UPDATE. I am not sure it makes any sense to allow rules, references, and triggers on sequences. However, using ALL or TABLE keywords you can define those permissions to a sequence. > (The subtext of this is that I don't have a lot of use for allowing > variant syntaxes that don't actually do anything different ...) FYI, SQL03 defines GRANT SEQUENCE. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073 Index: doc/src/sgml/ref/grant.sgml =================================================================== RCS file: /cvsroot/pgsql/doc/src/sgml/ref/grant.sgml,v retrieving revision 1.50 diff -c -c -r1.50 grant.sgml *** doc/src/sgml/ref/grant.sgml 20 Oct 2005 19:18:01 -0000 1.50 --- doc/src/sgml/ref/grant.sgml 6 Jan 2006 15:23:16 -0000 *************** *** 25,30 **** --- 25,35 ---- ON [ TABLE ] <replaceable class="PARAMETER">tablename</replaceable> [, ...] TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ] + GRANT { { SELECT | UPDATE } + [,...] | ALL [ PRIVILEGES ] } + ON SEQUENCE <replaceable class="PARAMETER">tablename</replaceable> [, ...] + TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ] + GRANT { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] } ON DATABASE <replaceable>dbname</replaceable> [, ...] TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ] *************** *** 511,517 **** <para> The <literal>RULE</literal> privilege, and privileges on ! databases, tablespaces, schemas, languages, and sequences are <productname>PostgreSQL</productname> extensions. </para> </refsect1> --- 516,522 ---- <para> The <literal>RULE</literal> privilege, and privileges on ! databases, tablespaces, schemas, and languages are <productname>PostgreSQL</productname> extensions. </para> </refsect1> Index: doc/src/sgml/ref/revoke.sgml =================================================================== RCS file: /cvsroot/pgsql/doc/src/sgml/ref/revoke.sgml,v retrieving revision 1.35 diff -c -c -r1.35 revoke.sgml *** doc/src/sgml/ref/revoke.sgml 20 Oct 2005 19:18:01 -0000 1.35 --- doc/src/sgml/ref/revoke.sgml 6 Jan 2006 15:23:16 -0000 *************** *** 28,33 **** --- 28,40 ---- [ CASCADE | RESTRICT ] REVOKE [ GRANT OPTION FOR ] + { { SELECT | UPDATE } + [,...] | ALL [ PRIVILEGES ] } + ON SEQUENCE <replaceable class="PARAMETER">tablename</replaceable> [, ...] + FROM { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] + [ CASCADE | RESTRICT ] + + REVOKE [ GRANT OPTION FOR ] { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] } ON DATABASE <replaceable>dbname</replaceable> [, ...] FROM { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] Index: src/backend/catalog/aclchk.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/catalog/aclchk.c,v retrieving revision 1.123 diff -c -c -r1.123 aclchk.c *** src/backend/catalog/aclchk.c 1 Dec 2005 02:03:00 -0000 1.123 --- src/backend/catalog/aclchk.c 6 Jan 2006 15:23:17 -0000 *************** *** 283,288 **** --- 283,289 ---- switch (stmt->objtype) { case ACL_OBJECT_RELATION: + case ACL_OBJECT_SEQUENCE: all_privileges = ACL_ALL_RIGHTS_RELATION; errormsg = _("invalid privilege type %s for table"); break; *************** *** 356,361 **** --- 357,363 ---- switch (istmt->objtype) { case ACL_OBJECT_RELATION: + case ACL_OBJECT_SEQUENCE: ExecGrant_Relation(istmt); break; case ACL_OBJECT_DATABASE: *************** *** 395,400 **** --- 397,403 ---- switch (objtype) { case ACL_OBJECT_RELATION: + case ACL_OBJECT_SEQUENCE: foreach(cell, objnames) { Oid relOid; *************** *** 577,582 **** --- 580,599 ---- errmsg("\"%s\" is a composite type", NameStr(pg_class_tuple->relname)))); + if (istmt->objtype == ACL_OBJECT_SEQUENCE) + { + if (pg_class_tuple->relkind != RELKIND_SEQUENCE) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("\"%s\" is not a sequence", + NameStr(pg_class_tuple->relname)))); + if (istmt->privileges != ACL_ALL_RIGHTS_RELATION && + istmt->privileges & ~(ACL_SELECT | ACL_UPDATE)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_GRANT_OPERATION), + errmsg("sequences only support SELECT and UPDATE privileges"))); + } + /* * Get owner ID and working copy of existing ACL. If there's no ACL, * substitute the proper default. Index: src/backend/catalog/pg_shdepend.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/catalog/pg_shdepend.c,v retrieving revision 1.6 diff -c -c -r1.6 pg_shdepend.c *** src/backend/catalog/pg_shdepend.c 1 Dec 2005 02:03:00 -0000 1.6 --- src/backend/catalog/pg_shdepend.c 6 Jan 2006 15:23:17 -0000 *************** *** 1133,1138 **** --- 1133,1139 ---- switch (sdepForm->classid) { case RelationRelationId: + /* could be a sequence */ istmt.objtype = ACL_OBJECT_RELATION; break; case DatabaseRelationId: Index: src/backend/parser/gram.y =================================================================== RCS file: /cvsroot/pgsql/src/backend/parser/gram.y,v retrieving revision 2.521 diff -c -c -r2.521 gram.y *** src/backend/parser/gram.y 29 Dec 2005 04:53:18 -0000 2.521 --- src/backend/parser/gram.y 6 Jan 2006 15:23:20 -0000 *************** *** 3322,3327 **** --- 3322,3334 ---- n->objs = $2; $$ = n; } + | SEQUENCE qualified_name_list + { + PrivTarget *n = makeNode(PrivTarget); + n->objtype = ACL_OBJECT_SEQUENCE; + n->objs = $2; + $$ = n; + } | FUNCTION function_with_argtypes_list { PrivTarget *n = makeNode(PrivTarget); Index: src/backend/utils/adt/acl.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/utils/adt/acl.c,v retrieving revision 1.129 diff -c -c -r1.129 acl.c *** src/backend/utils/adt/acl.c 18 Nov 2005 02:38:23 -0000 1.129 --- src/backend/utils/adt/acl.c 6 Jan 2006 15:23:23 -0000 *************** *** 542,547 **** --- 542,548 ---- switch (objtype) { case ACL_OBJECT_RELATION: + case ACL_OBJECT_SEQUENCE: world_default = ACL_NO_RIGHTS; owner_default = ACL_ALL_RIGHTS_RELATION; break; Index: src/include/nodes/parsenodes.h =================================================================== RCS file: /cvsroot/pgsql/src/include/nodes/parsenodes.h,v retrieving revision 1.298 diff -c -c -r1.298 parsenodes.h *** src/include/nodes/parsenodes.h 7 Dec 2005 15:20:55 -0000 1.298 --- src/include/nodes/parsenodes.h 6 Jan 2006 15:23:26 -0000 *************** *** 884,890 **** */ typedef enum GrantObjectType { ! ACL_OBJECT_RELATION, /* table, view, sequence */ ACL_OBJECT_DATABASE, /* database */ ACL_OBJECT_FUNCTION, /* function */ ACL_OBJECT_LANGUAGE, /* procedural language */ --- 884,891 ---- */ typedef enum GrantObjectType { ! ACL_OBJECT_RELATION, /* table, view */ ! ACL_OBJECT_SEQUENCE, /* sequence */ ACL_OBJECT_DATABASE, /* database */ ACL_OBJECT_FUNCTION, /* function */ ACL_OBJECT_LANGUAGE, /* procedural language */
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> FYI, SQL03 defines GRANT SEQUENCE.
Oh. Well, then that gives us precedent to go by. What do they specify
as the privileges for sequences?
regards, tom lane
Marko Kreen wrote: > On 1/6/06, Bruce Momjian <pgman@candle.pha.pa.us> wrote: > > Bruno Wolff III wrote: > > > It might be nice to split nextval and currval access as well. nextval access > > > corresponds to INSERT and currval access to SELECT. > > > > Uh, that is already in the code. nextval()/setval() is UPDATE, and > > currval() is SELECT. > > This seems weird. Shouldn't nextval/currval go together and setval > separately? Uh, logically, yes, but practially currval just reads/SELECTs, while nextval modifies/UPDATEs. > Considering there's no currval() without nextval(), what point > is disallowing currval() when user is able to call nextval()? Not sure. I think SET SESSION AUTHORIZATION would make it possible. > I rather want to allow nextval/currval and disable setval as it > allows regular user to DoS the database. Oh, interesting. We could easily have INSERT control that if we wanted, but I think you have to make a clear use case to override the risk of breaking applications. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > FYI, SQL03 defines GRANT SEQUENCE. > > Oh. Well, then that gives us precedent to go by. What do they specify > as the privileges for sequences? They don't seem to specify which actions go with which objects in the GRANT statement, nor do they specify what permissions should control the nextval-style statements. Seems like something they should have specified. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
On 1/6/06, Bruce Momjian <pgman@candle.pha.pa.us> wrote: > Marko Kreen wrote: > > On 1/6/06, Bruce Momjian <pgman@candle.pha.pa.us> wrote: > > > Bruno Wolff III wrote: > > > > It might be nice to split nextval and currval access as well. nextval access > > > > corresponds to INSERT and currval access to SELECT. > > > > > > Uh, that is already in the code. nextval()/setval() is UPDATE, and > > > currval() is SELECT. > > > > This seems weird. Shouldn't nextval/currval go together and setval > > separately? > > Uh, logically, yes, but practially currval just reads/SELECTs, while > nextval modifies/UPDATEs. Yeah, thats the mechanics behind it, but the currval() only works if the user was already able to call nextval(), so I see no point in separating them. In other words: there is nothing to do with only access to currval(), and with access to nextval() but not to currval() user loses only in convinience. > > Considering there's no currval() without nextval(), what point > > is disallowing currval() when user is able to call nextval()? > > Not sure. I think SET SESSION AUTHORIZATION would make it possible. /me confused, looks at docs... Huh? I really hope you are mistaken. This would mean the sequence state for currval() is kept per-user not per-backend. This would make impossible to make several connections as same user. Is Postgres really that broken? > > I rather want to allow nextval/currval and disable setval as it > > allows regular user to DoS the database. > > Oh, interesting. We could easily have INSERT control that if we wanted, > but I think you have to make a clear use case to override the risk of > breaking applications. I'd turn it around: is there any use-case for setval() for regular user? IMHO it's a admin-level operation, dangerous, and not needed for regular work. -- marko
Marko Kreen <markokr@gmail.com> writes:
> On 1/6/06, Bruce Momjian <pgman@candle.pha.pa.us> wrote:
>> Uh, logically, yes, but practially currval just reads/SELECTs, while
>> nextval modifies/UPDATEs.
> Yeah, thats the mechanics behind it, but the currval() only
> works if the user was already able to call nextval(), so I see
> no point in separating them.
You are completely wrong on this, because not all the code in a session
necessarily executes at the same privilege level. For instance, the
nextval() might be executed inside a SECURITY DEFINER function. It
might be reasonable to give code outside that function the right to see
what had been assigned (by executing currval()) without also saying that
it could do further nextvals().
I do agree that it would be a good idea to support a privilege
distinction between nextval() and setval().
>> Oh, interesting. We could easily have INSERT control that if we wanted,
>> but I think you have to make a clear use case to override the risk of
>> breaking applications.
There is no backwards-compatibility risk, because we'd still have the
old GRANT ON TABLE syntax grant both underlying rights. You'd have to
use the new syntax to get to a state where you had nextval but not
setval privilege or vice versa.
regards, tom lane
On Fri, Jan 06, 2006 at 19:11:27 +0200, Marko Kreen <markokr@gmail.com> wrote: > On 1/6/06, Bruce Momjian <pgman@candle.pha.pa.us> wrote: > > Considering there's no currval() without nextval(), what point > is disallowing currval() when user is able to call nextval()? > > I rather want to allow nextval/currval and disable setval as it > allows regular user to DoS the database. What I was thinking with this, is that you might allow someone the ability to insert records into a table which would make use of nextval, but not allow them to run nextval directly. But after inserting a record allow them to use currval to see what value was assigned. People could still mess with things by doing INSERTs and aborting the transaction, so this may not be the best example for why you would want this.
Bruno Wolff III wrote: > On Fri, Jan 06, 2006 at 19:11:27 +0200, > Marko Kreen <markokr@gmail.com> wrote: > > On 1/6/06, Bruce Momjian <pgman@candle.pha.pa.us> wrote: > > > > Considering there's no currval() without nextval(), what point > > is disallowing currval() when user is able to call nextval()? > > > > I rather want to allow nextval/currval and disable setval as it > > allows regular user to DoS the database. > > What I was thinking with this, is that you might allow someone the ability > to insert records into a table which would make use of nextval, but not > allow them to run nextval directly. But after inserting a record allow them > to use currval to see what value was assigned. > People could still mess with things by doing INSERTs and aborting the > transaction, so this may not be the best example for why you would want this. That seems too confusing to support based on usefulness of the new capability. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
On 1/6/06, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Marko Kreen <markokr@gmail.com> writes: > > On 1/6/06, Bruce Momjian <pgman@candle.pha.pa.us> wrote: > >> Uh, logically, yes, but practially currval just reads/SELECTs, while > >> nextval modifies/UPDATEs. > > > Yeah, thats the mechanics behind it, but the currval() only > > works if the user was already able to call nextval(), so I see > > no point in separating them. > > You are completely wrong on this, because not all the code in a session > necessarily executes at the same privilege level. For instance, the > nextval() might be executed inside a SECURITY DEFINER function. It > might be reasonable to give code outside that function the right to see > what had been assigned (by executing currval()) without also saying that > it could do further nextvals(). Ah, I did not think of this. Indeed, it's useful to keep them separate. I just wanted to point out that I see much more use to keep setval() separate from nextval/currval. (that is - always) > I do agree that it would be a good idea to support a privilege > distinction between nextval() and setval(). I tried to imagine a usage scenario for setval() but only single-user bulk data load comes in mind. Is there any actual scenario where it could be useful in multi-user setting? > >> Oh, interesting. We could easily have INSERT control that if we wanted, > >> but I think you have to make a clear use case to override the risk of > >> breaking applications. > > There is no backwards-compatibility risk, because we'd still have the > old GRANT ON TABLE syntax grant both underlying rights. You'd have to > use the new syntax to get to a state where you had nextval but not > setval privilege or vice versa. Good idea. -- marko
On 1/6/06, Bruno Wolff III <bruno@wolff.to> wrote: > On Fri, Jan 06, 2006 at 19:11:27 +0200, > Marko Kreen <markokr@gmail.com> wrote: > > On 1/6/06, Bruce Momjian <pgman@candle.pha.pa.us> wrote: > > > > Considering there's no currval() without nextval(), what point > > is disallowing currval() when user is able to call nextval()? > > > > I rather want to allow nextval/currval and disable setval as it > > allows regular user to DoS the database. > > What I was thinking with this, is that you might allow someone the ability > to insert records into a table which would make use of nextval, but not > allow them to run nextval directly. But after inserting a record allow them > to use currval to see what value was assigned. > People could still mess with things by doing INSERTs and aborting the > transaction, so this may not be the best example for why you would want this. This is similar to Tom's scenario. I'm not against keeping them separate. But my question is rather - is there any scenario where setval() should go with nextval()? It seems that their pairing is an accident and should be fixed. -- marko
Marko Kreen <markokr@gmail.com> writes:
> But my question is rather - is there any scenario where setval() should
> go with nextval()?
> It seems that their pairing is an accident and should be fixed.
I think the original argument for the current design was that with
enough nextval's you can duplicate the effect of a setval. This is only
strictly true if the sequence is CYCLE mode, and even then it'd take a
whole lot of patience to wrap an int8 sequence around ... but the
distinction between them is not so large as you make it out to be.
In any case I think we are wasting our time discussing it, and instead
should be looking through the SQL2003 spec to see what it requires.
Bruce couldn't find anything in it about this but I can't believe the
info isn't there somewhere.
regards, tom lane
Bruce Momjian wrote: > > I'm not sure offhand what keywords we'd want to use, but now is the time > > to look at it, *before* it becomes set in stone that GRANT ON SEQUENCE > > is just another spelling of GRANT ON TABLE. > > Sequences do not support INSERT, UPDATE, or DELETE, but we overload > UPDATE to control nextval()/setval(), so I just allowed SELECT and > UPDATE. I am not sure it makes any sense to allow rules, references, > and triggers on sequences. However, using ALL or TABLE keywords you can > define those permissions to a sequence. Here is an updated patch. The standard doesn't have GRANT VIEW so I didn't implement that. One tricky issue I realized is that we should dump out GRANT SEQUENCE, if possible. I have added code to check in pg_dump and use GRANT SEQUENCE if only SELECT, UPDATE, or ALL are used. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073 Index: doc/src/sgml/ref/grant.sgml =================================================================== RCS file: /cvsroot/pgsql/doc/src/sgml/ref/grant.sgml,v retrieving revision 1.50 diff -c -c -r1.50 grant.sgml *** doc/src/sgml/ref/grant.sgml 20 Oct 2005 19:18:01 -0000 1.50 --- doc/src/sgml/ref/grant.sgml 6 Jan 2006 20:33:45 -0000 *************** *** 25,30 **** --- 25,35 ---- ON [ TABLE ] <replaceable class="PARAMETER">tablename</replaceable> [, ...] TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ] + GRANT { { SELECT | UPDATE } + [,...] | ALL [ PRIVILEGES ] } + ON SEQUENCE <replaceable class="PARAMETER">tablename</replaceable> [, ...] + TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ] + GRANT { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] } ON DATABASE <replaceable>dbname</replaceable> [, ...] TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ] *************** *** 511,517 **** <para> The <literal>RULE</literal> privilege, and privileges on ! databases, tablespaces, schemas, languages, and sequences are <productname>PostgreSQL</productname> extensions. </para> </refsect1> --- 516,522 ---- <para> The <literal>RULE</literal> privilege, and privileges on ! databases, tablespaces, schemas, and languages are <productname>PostgreSQL</productname> extensions. </para> </refsect1> Index: doc/src/sgml/ref/revoke.sgml =================================================================== RCS file: /cvsroot/pgsql/doc/src/sgml/ref/revoke.sgml,v retrieving revision 1.35 diff -c -c -r1.35 revoke.sgml *** doc/src/sgml/ref/revoke.sgml 20 Oct 2005 19:18:01 -0000 1.35 --- doc/src/sgml/ref/revoke.sgml 6 Jan 2006 20:33:46 -0000 *************** *** 28,33 **** --- 28,40 ---- [ CASCADE | RESTRICT ] REVOKE [ GRANT OPTION FOR ] + { { SELECT | UPDATE } + [,...] | ALL [ PRIVILEGES ] } + ON SEQUENCE <replaceable class="PARAMETER">tablename</replaceable> [, ...] + FROM { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] + [ CASCADE | RESTRICT ] + + REVOKE [ GRANT OPTION FOR ] { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] } ON DATABASE <replaceable>dbname</replaceable> [, ...] FROM { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] Index: src/backend/catalog/aclchk.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/catalog/aclchk.c,v retrieving revision 1.123 diff -c -c -r1.123 aclchk.c *** src/backend/catalog/aclchk.c 1 Dec 2005 02:03:00 -0000 1.123 --- src/backend/catalog/aclchk.c 6 Jan 2006 20:33:46 -0000 *************** *** 283,288 **** --- 283,289 ---- switch (stmt->objtype) { case ACL_OBJECT_RELATION: + case ACL_OBJECT_SEQUENCE: all_privileges = ACL_ALL_RIGHTS_RELATION; errormsg = _("invalid privilege type %s for table"); break; *************** *** 356,361 **** --- 357,363 ---- switch (istmt->objtype) { case ACL_OBJECT_RELATION: + case ACL_OBJECT_SEQUENCE: ExecGrant_Relation(istmt); break; case ACL_OBJECT_DATABASE: *************** *** 395,400 **** --- 397,403 ---- switch (objtype) { case ACL_OBJECT_RELATION: + case ACL_OBJECT_SEQUENCE: foreach(cell, objnames) { Oid relOid; *************** *** 577,582 **** --- 580,599 ---- errmsg("\"%s\" is a composite type", NameStr(pg_class_tuple->relname)))); + if (istmt->objtype == ACL_OBJECT_SEQUENCE) + { + if (pg_class_tuple->relkind != RELKIND_SEQUENCE) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("\"%s\" is not a sequence", + NameStr(pg_class_tuple->relname)))); + if (istmt->privileges != ACL_ALL_RIGHTS_RELATION && + istmt->privileges & ~(ACL_SELECT | ACL_UPDATE)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_GRANT_OPERATION), + errmsg("sequences only support SELECT and UPDATE privileges"))); + } + /* * Get owner ID and working copy of existing ACL. If there's no ACL, * substitute the proper default. Index: src/backend/catalog/pg_shdepend.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/catalog/pg_shdepend.c,v retrieving revision 1.6 diff -c -c -r1.6 pg_shdepend.c *** src/backend/catalog/pg_shdepend.c 1 Dec 2005 02:03:00 -0000 1.6 --- src/backend/catalog/pg_shdepend.c 6 Jan 2006 20:33:47 -0000 *************** *** 1133,1138 **** --- 1133,1139 ---- switch (sdepForm->classid) { case RelationRelationId: + /* could be a sequence */ istmt.objtype = ACL_OBJECT_RELATION; break; case DatabaseRelationId: Index: src/backend/parser/gram.y =================================================================== RCS file: /cvsroot/pgsql/src/backend/parser/gram.y,v retrieving revision 2.521 diff -c -c -r2.521 gram.y *** src/backend/parser/gram.y 29 Dec 2005 04:53:18 -0000 2.521 --- src/backend/parser/gram.y 6 Jan 2006 20:33:49 -0000 *************** *** 3322,3327 **** --- 3322,3334 ---- n->objs = $2; $$ = n; } + | SEQUENCE qualified_name_list + { + PrivTarget *n = makeNode(PrivTarget); + n->objtype = ACL_OBJECT_SEQUENCE; + n->objs = $2; + $$ = n; + } | FUNCTION function_with_argtypes_list { PrivTarget *n = makeNode(PrivTarget); Index: src/backend/utils/adt/acl.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/utils/adt/acl.c,v retrieving revision 1.129 diff -c -c -r1.129 acl.c *** src/backend/utils/adt/acl.c 18 Nov 2005 02:38:23 -0000 1.129 --- src/backend/utils/adt/acl.c 6 Jan 2006 20:33:50 -0000 *************** *** 542,547 **** --- 542,548 ---- switch (objtype) { case ACL_OBJECT_RELATION: + case ACL_OBJECT_SEQUENCE: world_default = ACL_NO_RIGHTS; owner_default = ACL_ALL_RIGHTS_RELATION; break; Index: src/bin/pg_dump/dumputils.c =================================================================== RCS file: /cvsroot/pgsql/src/bin/pg_dump/dumputils.c,v retrieving revision 1.23 diff -c -c -r1.23 dumputils.c *** src/bin/pg_dump/dumputils.c 3 Dec 2005 21:06:18 -0000 1.23 --- src/bin/pg_dump/dumputils.c 6 Jan 2006 20:33:51 -0000 *************** *** 395,404 **** --- 395,420 ---- /* Scan individual ACL items */ for (i = 0; i < naclitems; i++) { + const char *outType = type; + if (!parseAclItem(aclitems[i], type, name, remoteVersion, grantee, grantor, privs, privswgo)) return false; + /* + * For backward compatibility, non-SEQUENCE GRANT statements can + * assign non-SELECT, non-UPDATE permissions. We allow those + * to be dumped by changing SEQUENCE to TABLE. + */ + if (strcmp(outType, "SEQUENCE") == 0) + { + if (strcmp(privs->data, "SELECT") != 0 && + strcmp(privs->data, "UPDATE") != 0 && + strcmp(privs->data, "SELECT,UPDATE") != 0 && + strcmp(privs->data, "ALL") != 0) + outType = "TABLE"; + } + if (grantor->len == 0 && owner) printfPQExpBuffer(grantor, "%s", owner); *************** *** 419,433 **** : strcmp(privs->data, "ALL") != 0) { appendPQExpBuffer(firstsql, "REVOKE ALL ON %s %s FROM %s;\n", ! type, name, fmtId(grantee->data)); if (privs->len > 0) appendPQExpBuffer(firstsql, "GRANT %s ON %s %s TO %s;\n", ! privs->data, type, name, fmtId(grantee->data)); if (privswgo->len > 0) appendPQExpBuffer(firstsql, "GRANT %s ON %s %s TO %s WITH GRANT OPTION;\n", ! privswgo->data, type, name, fmtId(grantee->data)); } } --- 435,449 ---- : strcmp(privs->data, "ALL") != 0) { appendPQExpBuffer(firstsql, "REVOKE ALL ON %s %s FROM %s;\n", ! outType, name, fmtId(grantee->data)); if (privs->len > 0) appendPQExpBuffer(firstsql, "GRANT %s ON %s %s TO %s;\n", ! privs->data, outType, name, fmtId(grantee->data)); if (privswgo->len > 0) appendPQExpBuffer(firstsql, "GRANT %s ON %s %s TO %s WITH GRANT OPTION;\n", ! privswgo->data, outType, name, fmtId(grantee->data)); } } *************** *** 444,450 **** if (privs->len > 0) { appendPQExpBuffer(secondsql, "GRANT %s ON %s %s TO ", ! privs->data, type, name); if (grantee->len == 0) appendPQExpBuffer(secondsql, "PUBLIC;\n"); else if (strncmp(grantee->data, "group ", --- 460,466 ---- if (privs->len > 0) { appendPQExpBuffer(secondsql, "GRANT %s ON %s %s TO ", ! privs->data, outType, name); if (grantee->len == 0) appendPQExpBuffer(secondsql, "PUBLIC;\n"); else if (strncmp(grantee->data, "group ", *************** *** 457,463 **** if (privswgo->len > 0) { appendPQExpBuffer(secondsql, "GRANT %s ON %s %s TO ", ! privswgo->data, type, name); if (grantee->len == 0) appendPQExpBuffer(secondsql, "PUBLIC"); else if (strncmp(grantee->data, "group ", --- 473,479 ---- if (privswgo->len > 0) { appendPQExpBuffer(secondsql, "GRANT %s ON %s %s TO ", ! privswgo->data, outType, name); if (grantee->len == 0) appendPQExpBuffer(secondsql, "PUBLIC"); else if (strncmp(grantee->data, "group ", *************** *** 480,489 **** * If we didn't find any owner privs, the owner must have revoked 'em all */ if (!found_owner_privs && owner) - { appendPQExpBuffer(firstsql, "REVOKE ALL ON %s %s FROM %s;\n", type, name, fmtId(owner)); - } destroyPQExpBuffer(grantee); destroyPQExpBuffer(grantor); --- 496,503 ---- *************** *** 568,574 **** resetPQExpBuffer(privs); resetPQExpBuffer(privswgo); ! if (strcmp(type, "TABLE") == 0) { CONVERT_PRIV('a', "INSERT"); CONVERT_PRIV('r', "SELECT"); --- 582,588 ---- resetPQExpBuffer(privs); resetPQExpBuffer(privswgo); ! if (strcmp(type, "TABLE") == 0 || strcmp(type, "SEQUENCE") == 0) { CONVERT_PRIV('a', "INSERT"); CONVERT_PRIV('r', "SELECT"); Index: src/bin/pg_dump/pg_dump.c =================================================================== RCS file: /cvsroot/pgsql/src/bin/pg_dump/pg_dump.c,v retrieving revision 1.425 diff -c -c -r1.425 pg_dump.c *** src/bin/pg_dump/pg_dump.c 6 Jan 2006 19:08:33 -0000 1.425 --- src/bin/pg_dump/pg_dump.c 6 Jan 2006 20:33:56 -0000 *************** *** 6776,6782 **** /* Handle the ACL here */ namecopy = strdup(fmtId(tbinfo->dobj.name)); ! dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId, "TABLE", namecopy, tbinfo->dobj.name, tbinfo->dobj.namespace->dobj.name, tbinfo->rolname, tbinfo->relacl); --- 6776,6784 ---- /* Handle the ACL here */ namecopy = strdup(fmtId(tbinfo->dobj.name)); ! dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId, ! /* Issue GRANT SEQUENCE, if applicable */ ! tbinfo->relkind != RELKIND_SEQUENCE ? "TABLE" : "SEQUENCE", namecopy, tbinfo->dobj.name, tbinfo->dobj.namespace->dobj.name, tbinfo->rolname, tbinfo->relacl); Index: src/include/nodes/parsenodes.h =================================================================== RCS file: /cvsroot/pgsql/src/include/nodes/parsenodes.h,v retrieving revision 1.298 diff -c -c -r1.298 parsenodes.h *** src/include/nodes/parsenodes.h 7 Dec 2005 15:20:55 -0000 1.298 --- src/include/nodes/parsenodes.h 6 Jan 2006 20:33:56 -0000 *************** *** 884,890 **** */ typedef enum GrantObjectType { ! ACL_OBJECT_RELATION, /* table, view, sequence */ ACL_OBJECT_DATABASE, /* database */ ACL_OBJECT_FUNCTION, /* function */ ACL_OBJECT_LANGUAGE, /* procedural language */ --- 884,891 ---- */ typedef enum GrantObjectType { ! ACL_OBJECT_RELATION, /* table, view */ ! ACL_OBJECT_SEQUENCE, /* sequence */ ACL_OBJECT_DATABASE, /* database */ ACL_OBJECT_FUNCTION, /* function */ ACL_OBJECT_LANGUAGE, /* procedural language */
Tom Lane wrote: > Marko Kreen <markokr@gmail.com> writes: > > But my question is rather - is there any scenario where setval() should > > go with nextval()? > > > It seems that their pairing is an accident and should be fixed. > > I think the original argument for the current design was that with > enough nextval's you can duplicate the effect of a setval. This is only > strictly true if the sequence is CYCLE mode, and even then it'd take a > whole lot of patience to wrap an int8 sequence around ... but the > distinction between them is not so large as you make it out to be. > > In any case I think we are wasting our time discussing it, and instead > should be looking through the SQL2003 spec to see what it requires. > Bruce couldn't find anything in it about this but I can't believe the > info isn't there somewhere. What I did was to read through the GRANT and SEQUENCE sections, then I dumped it to text and did a grep for 'grant' or perm* appearing on the same line as sequence, and came up with nothing. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
On 1/6/06, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Marko Kreen <markokr@gmail.com> writes: > > But my question is rather - is there any scenario where setval() should > > go with nextval()? > > > It seems that their pairing is an accident and should be fixed. > > I think the original argument for the current design was that with > enough nextval's you can duplicate the effect of a setval. This is only > strictly true if the sequence is CYCLE mode, and even then it'd take a > whole lot of patience to wrap an int8 sequence around ... but the > distinction between them is not so large as you make it out to be. With bigserial this is more like CPU DoS, while other users can work normally. > In any case I think we are wasting our time discussing it, and instead > should be looking through the SQL2003 spec to see what it requires. > Bruce couldn't find anything in it about this but I can't believe the > info isn't there somewhere. Google tells that Oracle has ALTER and SELECT; DB2 has ALTER and USAGE. I found SQL2003 pdf's too ... from my reading it has only USAGE. 5WD-02-Foundation-2003-09.pdf: page 724 -> General Rules -> #2 page 740 -> Syntax rules -> #3 Everything combined: SELECT: currval UPDATE: nextval USAGE: currval, nextval ALTER: setval Confusing? -- marko
Marko Kreen wrote: > > In any case I think we are wasting our time discussing it, and instead > > should be looking through the SQL2003 spec to see what it requires. > > Bruce couldn't find anything in it about this but I can't believe the > > info isn't there somewhere. > > Google tells that Oracle has ALTER and SELECT; DB2 has ALTER and USAGE. > > I found SQL2003 pdf's too ... from my reading it has only USAGE. > > 5WD-02-Foundation-2003-09.pdf: > page 724 -> General Rules -> #2 > page 740 -> Syntax rules -> #3 I admit I am terrible at understanding the standard, but I can't find anything relevant on the page numbers you mentioned. Are those the document pages or the page numbers displayed by the PDF viewer? What is the section heading? I am using the same filename you have. > Everything combined: > SELECT: currval > UPDATE: nextval > USAGE: currval, nextval > ALTER: setval > > Confusing? I see USAGE in the standard, but not ALTER. We don't support USAGE so I am guessing our SELECT/UPDATE behavior is OK. Does this mean we should only allow owners to do setval(), rather than binding it to INSERT? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
On 1/6/06, Bruce Momjian <pgman@candle.pha.pa.us> wrote: > Marko Kreen wrote: > > I found SQL2003 pdf's too ... from my reading it has only USAGE. > > > > 5WD-02-Foundation-2003-09.pdf: > > page 724 -> General Rules -> #2 > > page 740 -> Syntax rules -> #3 > > I admit I am terrible at understanding the standard, but I can't find > anything relevant on the page numbers you mentioned. Are those the > document pages or the page numbers displayed by the PDF viewer? What is > the section heading? I am using the same filename you have. Those are print page numbers. (In case you have dead-tree variant :) And I got them here: http://www.wiscorp.com/SQLStandards.html Uh, and they are bit wrong. Ok here are they fully: 11.62 <sequence generator definition> General rules (page 727 printed/751 real) point #2 12.3 <privileges> Syntax rules (page 740 printed/764 real) point #3 > > Everything combined: > > SELECT: currval > > UPDATE: nextval > > USAGE: currval, nextval > > ALTER: setval > > > > Confusing? > > I see USAGE in the standard, but not ALTER. We don't support USAGE so I > am guessing our SELECT/UPDATE behavior is OK. No, we still want to separate setval from nextval. > Does this mean we should > only allow owners to do setval(), rather than binding it to INSERT? My first reaction is that it should be grantable, although I can't find any reasons for it, except backwards compatibility. How about this: SELECT: currval INSERT: nextval USAGE: currval, nextval UPDATE: setval -- marko
Marko Kreen wrote: > On 1/6/06, Bruce Momjian <pgman@candle.pha.pa.us> wrote: > > Marko Kreen wrote: > > > I found SQL2003 pdf's too ... from my reading it has only USAGE. > > > > > > 5WD-02-Foundation-2003-09.pdf: > > > page 724 -> General Rules -> #2 > > > page 740 -> Syntax rules -> #3 > > > > I admit I am terrible at understanding the standard, but I can't find > > anything relevant on the page numbers you mentioned. Are those the > > document pages or the page numbers displayed by the PDF viewer? What is > > the section heading? I am using the same filename you have. > > Those are print page numbers. (In case you have dead-tree variant :) > And I got them here: http://www.wiscorp.com/SQLStandards.html > > Uh, and they are bit wrong. Ok here are they fully: > > 11.62 <sequence generator definition> > General rules (page 727 printed/751 real) point #2 > > 12.3 <privileges> > Syntax rules (page 740 printed/764 real) point #3 OK, I see it now, and in an earlier email I quoted the part where I think USAGE links in to nextval(). I was looking for something obvious. :-) > > > Everything combined: > > > SELECT: currval > > > UPDATE: nextval > > > USAGE: currval, nextval > > > ALTER: setval > > > > > > Confusing? > > > > I see USAGE in the standard, but not ALTER. We don't support USAGE so I > > am guessing our SELECT/UPDATE behavior is OK. > > No, we still want to separate setval from nextval. My point was that currval -> SELECT and nextval -> UPDATE was correct. I see now that I am wrong and that the standard wants USAGE. That combined was every db's behavior combined, right? I got confused. > > Does this mean we should > > only allow owners to do setval(), rather than binding it to INSERT? > > My first reaction is that it should be grantable, although > I can't find any reasons for it, except backwards compatibility. > > How about this: > > SELECT: currval > INSERT: nextval > USAGE: currval, nextval > UPDATE: setval I think nextval() is naturally UPDATE. I am thinking setval would be INSERT, and with setval() being used less, it would perhaps be a better choice for a change anyway. However, in doing the pg_dump part of the patch, I perhaps see a problem. If someone does: GRANT UPDATE ON seq1 TO PUBLIC; do we give them nextval() and setval() permissions? If they do: GRANT UPDATE ON SEQUENCE seq1 TO PUBLIC; -------- they only set nextval()? That seems quite confusing. Can we change UPDATE for both GRANT syntaxes, and somehow have people fix them up after they load in 8.2? How many non-owners do setval()? FYI, we could support USAGE just on sequences, and have it map to UPDATE, but pg_dump it out as USAGE. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> FYI, we could support USAGE just on sequences, and have it map to
> UPDATE, but pg_dump it out as USAGE.
It seems the spec doesn't cover setval() and currval(), which is not
too surprising given those aren't standard.
Here is a proposal:
SELECT priv -> allows currval() and SELECT * FROM seq
USAGE priv -> allows nextval() (required by SQL2003)
UPDATE priv -> allows setval() and nextval()
I was originally thinking of a separate privilege bit for setval(), but
that's sort of silly, as you can get (approximately) the effect of
nextval() via setval(). Not much point in prohibiting nextval() to
someone who can do setval().
This is 100% upward compatible with our current definition, and it meets
both the SQL spec and Marko's desire to have a way of granting only
nextval() privilege.
BTW, what about lastval()? I'm not sure we can usefully associate any
privilege check with that, since it's not clear which sequence it
applies to. Does it make sense to remember what sequence the value came
from and privilege-check against that, or is that just too weird?
regards, tom lane
Tom, > BTW, what about lastval()? I'm not sure we can usefully associate any > privilege check with that, since it's not clear which sequence it > applies to. Does it make sense to remember what sequence the value came > from and privilege-check against that, or is that just too weird? Hmmm. Yet another problem with lastval(). Darn those MySQL migrators! I'd say that lastval() needs to be defined as the superuser with "security definer". Hmmm, although does that carry over to sequences the superuser doesn't own? How are we handling it now? Overal, it's hard to get too concerned about this, since a user can't really get anything out of lastval() if he doesn't have permissions on the sequence he's trying to query, in order to run currval. -- --Josh Josh Berkus Aglio Database Solutions San Francisco
Josh Berkus <josh@agliodbs.com> writes:
>> BTW, what about lastval()?
> Overal, it's hard to get too concerned about this, since a user can't
> really get anything out of lastval() if he doesn't have permissions on the
> sequence he's trying to query, in order to run currval.
Well, no, consider my example to Marko: there could be a SECURITY
DEFINER function that has the privilege to run nextval(). After
that, if lastval() isn't privilege-checked then code that doesn't
have any privilege at all on the sequence could get at the value.
However, looking at the source code I see that lastval() does in fact
insist on SELECT rights on the sequence the value is coming from.
So I guess we can just leave that as-is.
regards, tom lane
On 1/7/06, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > FYI, we could support USAGE just on sequences, and have it map to > > UPDATE, but pg_dump it out as USAGE. > > It seems the spec doesn't cover setval() and currval(), which is not > too surprising given those aren't standard. > > Here is a proposal: > > SELECT priv -> allows currval() and SELECT * FROM seq > > USAGE priv -> allows nextval() (required by SQL2003) > > UPDATE priv -> allows setval() and nextval() > > I was originally thinking of a separate privilege bit for setval(), but > that's sort of silly, as you can get (approximately) the effect of > nextval() via setval(). Not much point in prohibiting nextval() to > someone who can do setval(). > > This is 100% upward compatible with our current definition, and it meets > both the SQL spec and Marko's desire to have a way of granting only > nextval() privilege. Good point about compatibility. But makes the common case ugly. "For regular usage you need to grant SELECT, USAGE ..." Huh? :) How about this: SELECT: currval INSERT: nextval UPDATE: nextval, setval USAGE: nextval, currval With this the user needs only to remember SQL2003 syntax to cover 99.9% use cases. And when he wants to play more finegrained then he can combine with the SELECT, INSERT, UPDATE. The above table seem bit messy, but I see it as much easier to explain to somebody. > BTW, what about lastval()? I'm not sure we can usefully associate any > privilege check with that, since it's not clear which sequence it > applies to. Does it make sense to remember what sequence the value came > from and privilege-check against that, or is that just too weird? Hmm. So it means with lastval() user can see the state of sequences he has no access to? Seems like the privilege check would be good idea. -- marko
Marko Kreen wrote: > On 1/7/06, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > > FYI, we could support USAGE just on sequences, and have it map to > > > UPDATE, but pg_dump it out as USAGE. > > > > It seems the spec doesn't cover setval() and currval(), which is not > > too surprising given those aren't standard. > > > > Here is a proposal: > > > > SELECT priv -> allows currval() and SELECT * FROM seq > > > > USAGE priv -> allows nextval() (required by SQL2003) > > > > UPDATE priv -> allows setval() and nextval() > > > > I was originally thinking of a separate privilege bit for setval(), but > > that's sort of silly, as you can get (approximately) the effect of > > nextval() via setval(). Not much point in prohibiting nextval() to > > someone who can do setval(). > > > > This is 100% upward compatible with our current definition, and it meets > > both the SQL spec and Marko's desire to have a way of granting only > > nextval() privilege. > > Good point about compatibility. But makes the common case ugly. > "For regular usage you need to grant SELECT, USAGE ..." Huh? :) > > How about this: > > SELECT: currval > INSERT: nextval > UPDATE: nextval, setval > USAGE: nextval, currval > > With this the user needs only to remember SQL2003 syntax > to cover 99.9% use cases. And when he wants to play more > finegrained then he can combine with the SELECT, INSERT, UPDATE. I think we should use Tom's suggestion, for two reasons. First, the common case currently needs both SELECT and UPDATE, and I have heard no one complain about it. Second, I think USAGE is better assocated with nextval() and UPDATE with both nextval() and setval(). > The above table seem bit messy, but I see it as much easier to explain > to somebody. I am confused about your list above, so I can't see how that would be easy to explain. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Marko Kreen <markokr@gmail.com> writes:
> Good point about compatibility. But makes the common case ugly.
> "For regular usage you need to grant SELECT, USAGE ..." Huh? :)
> How about this:
> SELECT: currval
> INSERT: nextval
> UPDATE: nextval, setval
> USAGE: nextval, currval
Seems a little weird. Hmm ... what is the use-case for allowing someone
to do nextval but not currval? I can't see one. How about we simplify
this to
SELECT: currval
UPDATE: nextval, setval
USAGE: nextval, currval
This is still upward compatible with our old behavior, which is
SELECT: currval
UPDATE: nextval, setval
and it still meets the SQL spec's requirement that USAGE allow nextval,
and USAGE is the only one you need for "normal" usage.
regards, tom lane
Tom Lane wrote: > Marko Kreen <markokr@gmail.com> writes: > > Good point about compatibility. But makes the common case ugly. > > "For regular usage you need to grant SELECT, USAGE ..." Huh? :) > > > How about this: > > > SELECT: currval > > INSERT: nextval > > UPDATE: nextval, setval > > USAGE: nextval, currval > > Seems a little weird. Hmm ... what is the use-case for allowing someone > to do nextval but not currval? I can't see one. How about we simplify > this to > > SELECT: currval > UPDATE: nextval, setval > USAGE: nextval, currval > > This is still upward compatible with our old behavior, which is > > SELECT: currval > UPDATE: nextval, setval > > and it still meets the SQL spec's requirement that USAGE allow nextval, > and USAGE is the only one you need for "normal" usage. I think your original proposal was better. Why is it important that we have a single-keyword usage for the common case? No one has complained about what we have now and that requires two keywords just like your proposal. We don't have a shorthand for GRANT INSERT/UPDATE/DELETE. In fact, if it was backward-compatible I would suggest we make UPDATE just setval. Does the standard require USAGE to support currval? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Does the standard require USAGE to support currval?
currval isn't in the standard (unless I missed something), so it has
nothing to say one way or the other on the point.
Basically what we seem to be homing in on is to keep SELECT and UPDATE
privileges doing what they do now and then add a USAGE privilege.
I think I agree with Marko that USAGE should mean nextval + currval;
it already must overlap UPDATE and so there's no very good reason why it
shouldn't overlap SELECT too. Furthermore there's no plausible use-case
where you'd want to grant nextval but not currval, so why not keep the
notation simple?
regards, tom lane
I wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> Does the standard require USAGE to support currval?
> currval isn't in the standard (unless I missed something), so it has
> nothing to say one way or the other on the point.
Wait, I take that back. Remember our previous discussions about this
point: the spec's NEXT VALUE FOR construct is *not* equivalent to
nextval, because they specify that the sequence advances just once per
command even if the command says NEXT VALUE FOR in multiple places.
This means that NEXT VALUE FOR is effectively both nextval and currval;
the first one in a command does nextval and the rest do currval.
Accordingly, I think it's reasonable to read the spec as saying that
USAGE privilege encompasses both nextval and currval.
regards, tom lane
Should UPDATE also allow currval()? Your logic below seems to suggest that. --------------------------------------------------------------------------- Tom Lane wrote: > I wrote: > > Bruce Momjian <pgman@candle.pha.pa.us> writes: > >> Does the standard require USAGE to support currval? > > > currval isn't in the standard (unless I missed something), so it has > > nothing to say one way or the other on the point. > > Wait, I take that back. Remember our previous discussions about this > point: the spec's NEXT VALUE FOR construct is *not* equivalent to > nextval, because they specify that the sequence advances just once per > command even if the command says NEXT VALUE FOR in multiple places. > This means that NEXT VALUE FOR is effectively both nextval and currval; > the first one in a command does nextval and the rest do currval. > > Accordingly, I think it's reasonable to read the spec as saying that > USAGE privilege encompasses both nextval and currval. > > regards, tom lane > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Tom Lane wrote: > I wrote: > > Bruce Momjian <pgman@candle.pha.pa.us> writes: > >> Does the standard require USAGE to support currval? > > > currval isn't in the standard (unless I missed something), so it has > > nothing to say one way or the other on the point. > > Wait, I take that back. Remember our previous discussions about this > point: the spec's NEXT VALUE FOR construct is *not* equivalent to > nextval, because they specify that the sequence advances just once per > command even if the command says NEXT VALUE FOR in multiple places. > This means that NEXT VALUE FOR is effectively both nextval and currval; > the first one in a command does nextval and the rest do currval. > > Accordingly, I think it's reasonable to read the spec as saying that > USAGE privilege encompasses both nextval and currval. Here's a patch that more closely matches the ideas proposed. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073 Index: doc/src/sgml/ref/grant.sgml =================================================================== RCS file: /cvsroot/pgsql/doc/src/sgml/ref/grant.sgml,v retrieving revision 1.50 diff -c -c -r1.50 grant.sgml *** doc/src/sgml/ref/grant.sgml 20 Oct 2005 19:18:01 -0000 1.50 --- doc/src/sgml/ref/grant.sgml 7 Jan 2006 06:00:14 -0000 *************** *** 25,30 **** --- 25,35 ---- ON [ TABLE ] <replaceable class="PARAMETER">tablename</replaceable> [, ...] TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ] + GRANT { { SELECT | USAGE | UPDATE } + [,...] | ALL [ PRIVILEGES ] } + ON SEQUENCE <replaceable class="PARAMETER">tablename</replaceable> [, ...] + TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ] + GRANT { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] } ON DATABASE <replaceable>dbname</replaceable> [, ...] TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ] *************** *** 260,265 **** --- 265,274 ---- also met). Essentially this allows the grantee to <quote>look up</> objects within the schema. </para> + <para> + For sequences, this privilege allows the use of the + <function>currval</function> and <function>nextval</function> functions. + </para> </listitem> </varlistentry> *************** *** 511,517 **** <para> The <literal>RULE</literal> privilege, and privileges on ! databases, tablespaces, schemas, languages, and sequences are <productname>PostgreSQL</productname> extensions. </para> </refsect1> --- 520,526 ---- <para> The <literal>RULE</literal> privilege, and privileges on ! databases, tablespaces, schemas, and languages are <productname>PostgreSQL</productname> extensions. </para> </refsect1> Index: doc/src/sgml/ref/revoke.sgml =================================================================== RCS file: /cvsroot/pgsql/doc/src/sgml/ref/revoke.sgml,v retrieving revision 1.35 diff -c -c -r1.35 revoke.sgml *** doc/src/sgml/ref/revoke.sgml 20 Oct 2005 19:18:01 -0000 1.35 --- doc/src/sgml/ref/revoke.sgml 7 Jan 2006 06:00:14 -0000 *************** *** 28,33 **** --- 28,40 ---- [ CASCADE | RESTRICT ] REVOKE [ GRANT OPTION FOR ] + { { SELECT | UPDATE } + [,...] | ALL [ PRIVILEGES ] } + ON SEQUENCE <replaceable class="PARAMETER">tablename</replaceable> [, ...] + FROM { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] + [ CASCADE | RESTRICT ] + + REVOKE [ GRANT OPTION FOR ] { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] } ON DATABASE <replaceable>dbname</replaceable> [, ...] FROM { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] Index: src/backend/catalog/aclchk.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/catalog/aclchk.c,v retrieving revision 1.123 diff -c -c -r1.123 aclchk.c *** src/backend/catalog/aclchk.c 1 Dec 2005 02:03:00 -0000 1.123 --- src/backend/catalog/aclchk.c 7 Jan 2006 06:00:27 -0000 *************** *** 286,291 **** --- 286,295 ---- all_privileges = ACL_ALL_RIGHTS_RELATION; errormsg = _("invalid privilege type %s for table"); break; + case ACL_OBJECT_SEQUENCE: + all_privileges = ACL_ALL_RIGHTS_SEQUENCE; + errormsg = _("invalid privilege type %s for sequence"); + break; case ACL_OBJECT_DATABASE: all_privileges = ACL_ALL_RIGHTS_DATABASE; errormsg = _("invalid privilege type %s for database"); *************** *** 356,361 **** --- 360,366 ---- switch (istmt->objtype) { case ACL_OBJECT_RELATION: + case ACL_OBJECT_SEQUENCE: ExecGrant_Relation(istmt); break; case ACL_OBJECT_DATABASE: *************** *** 395,400 **** --- 400,406 ---- switch (objtype) { case ACL_OBJECT_RELATION: + case ACL_OBJECT_SEQUENCE: foreach(cell, objnames) { Oid relOid; *************** *** 577,582 **** --- 583,602 ---- errmsg("\"%s\" is a composite type", NameStr(pg_class_tuple->relname)))); + if (istmt->objtype == ACL_OBJECT_SEQUENCE) + { + if (pg_class_tuple->relkind != RELKIND_SEQUENCE) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("\"%s\" is not a sequence", + NameStr(pg_class_tuple->relname)))); + if (istmt->privileges != ACL_ALL_RIGHTS_RELATION && + istmt->privileges & ~(ACL_SELECT | ACL_USAGE | ACL_UPDATE)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_GRANT_OPERATION), + errmsg("sequences only support SELECT, USAGE, and UPDATE privileges"))); + } + /* * Get owner ID and working copy of existing ACL. If there's no ACL, * substitute the proper default. Index: src/backend/catalog/pg_shdepend.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/catalog/pg_shdepend.c,v retrieving revision 1.6 diff -c -c -r1.6 pg_shdepend.c *** src/backend/catalog/pg_shdepend.c 1 Dec 2005 02:03:00 -0000 1.6 --- src/backend/catalog/pg_shdepend.c 7 Jan 2006 06:00:29 -0000 *************** *** 1133,1138 **** --- 1133,1139 ---- switch (sdepForm->classid) { case RelationRelationId: + /* could be a sequence */ istmt.objtype = ACL_OBJECT_RELATION; break; case DatabaseRelationId: Index: src/backend/commands/sequence.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/commands/sequence.c,v retrieving revision 1.126 diff -c -c -r1.126 sequence.c *** src/backend/commands/sequence.c 22 Nov 2005 18:17:09 -0000 1.126 --- src/backend/commands/sequence.c 7 Jan 2006 06:00:30 -0000 *************** *** 422,428 **** /* open and AccessShareLock sequence */ init_sequence(relid, &elm, &seqrel); ! if (pg_class_aclcheck(elm->relid, GetUserId(), ACL_UPDATE) != ACLCHECK_OK) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied for sequence %s", --- 422,429 ---- /* open and AccessShareLock sequence */ init_sequence(relid, &elm, &seqrel); ! if (pg_class_aclcheck(elm->relid, GetUserId(), ACL_USAGE) != ACLCHECK_OK && ! pg_class_aclcheck(elm->relid, GetUserId(), ACL_UPDATE) != ACLCHECK_OK) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied for sequence %s", *************** *** 613,619 **** /* open and AccessShareLock sequence */ init_sequence(relid, &elm, &seqrel); ! if (pg_class_aclcheck(elm->relid, GetUserId(), ACL_SELECT) != ACLCHECK_OK) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied for sequence %s", --- 614,621 ---- /* open and AccessShareLock sequence */ init_sequence(relid, &elm, &seqrel); ! if (pg_class_aclcheck(elm->relid, GetUserId(), ACL_SELECT) != ACLCHECK_OK && ! pg_class_aclcheck(elm->relid, GetUserId(), ACL_USAGE) != ACLCHECK_OK) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied for sequence %s", *************** *** 657,663 **** /* nextval() must have already been called for this sequence */ Assert(last_used_seq->increment != 0); ! if (pg_class_aclcheck(last_used_seq->relid, GetUserId(), ACL_SELECT) != ACLCHECK_OK) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied for sequence %s", --- 659,666 ---- /* nextval() must have already been called for this sequence */ Assert(last_used_seq->increment != 0); ! if (pg_class_aclcheck(last_used_seq->relid, GetUserId(), ACL_SELECT) != ACLCHECK_OK && ! pg_class_aclcheck(last_used_seq->relid, GetUserId(), ACL_USAGE) != ACLCHECK_OK) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied for sequence %s", Index: src/backend/parser/gram.y =================================================================== RCS file: /cvsroot/pgsql/src/backend/parser/gram.y,v retrieving revision 2.521 diff -c -c -r2.521 gram.y *** src/backend/parser/gram.y 29 Dec 2005 04:53:18 -0000 2.521 --- src/backend/parser/gram.y 7 Jan 2006 06:00:36 -0000 *************** *** 3322,3327 **** --- 3322,3334 ---- n->objs = $2; $$ = n; } + | SEQUENCE qualified_name_list + { + PrivTarget *n = makeNode(PrivTarget); + n->objtype = ACL_OBJECT_SEQUENCE; + n->objs = $2; + $$ = n; + } | FUNCTION function_with_argtypes_list { PrivTarget *n = makeNode(PrivTarget); Index: src/backend/utils/adt/acl.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/utils/adt/acl.c,v retrieving revision 1.129 diff -c -c -r1.129 acl.c *** src/backend/utils/adt/acl.c 18 Nov 2005 02:38:23 -0000 1.129 --- src/backend/utils/adt/acl.c 7 Jan 2006 06:00:39 -0000 *************** *** 542,547 **** --- 542,548 ---- switch (objtype) { case ACL_OBJECT_RELATION: + case ACL_OBJECT_SEQUENCE: world_default = ACL_NO_RIGHTS; owner_default = ACL_ALL_RIGHTS_RELATION; break; Index: src/bin/pg_dump/dumputils.c =================================================================== RCS file: /cvsroot/pgsql/src/bin/pg_dump/dumputils.c,v retrieving revision 1.23 diff -c -c -r1.23 dumputils.c *** src/bin/pg_dump/dumputils.c 3 Dec 2005 21:06:18 -0000 1.23 --- src/bin/pg_dump/dumputils.c 7 Jan 2006 06:00:42 -0000 *************** *** 22,28 **** #define supports_grant_options(version) ((version) >= 70400) static bool parseAclItem(const char *item, const char *type, const char *name, ! int remoteVersion, PQExpBuffer grantee, PQExpBuffer grantor, PQExpBuffer privs, PQExpBuffer privswgo); static char *copyAclUserName(PQExpBuffer output, char *input); --- 22,28 ---- #define supports_grant_options(version) ((version) >= 70400) static bool parseAclItem(const char *item, const char *type, const char *name, ! int remoteVersion, bool *is_valid_for_sequence, PQExpBuffer grantee, PQExpBuffer grantor, PQExpBuffer privs, PQExpBuffer privswgo); static char *copyAclUserName(PQExpBuffer output, char *input); *************** *** 395,404 **** --- 395,417 ---- /* Scan individual ACL items */ for (i = 0; i < naclitems; i++) { + const char *outType = type; + bool is_valid_for_sequence; + if (!parseAclItem(aclitems[i], type, name, remoteVersion, + &is_valid_for_sequence, grantee, grantor, privs, privswgo)) return false; + /* + * For backward compatibility, non-SEQUENCE GRANT statements can + * assign non-SELECT, non-UPDATE permissions. We allow those + * to be dumped by changing SEQUENCE to TABLE. USAGE can be + * assigned only to sequences. + */ + if (strcmp(outType, "SEQUENCE") == 0 && !is_valid_for_sequence) + outType = "TABLE"; + if (grantor->len == 0 && owner) printfPQExpBuffer(grantor, "%s", owner); *************** *** 419,433 **** : strcmp(privs->data, "ALL") != 0) { appendPQExpBuffer(firstsql, "REVOKE ALL ON %s %s FROM %s;\n", ! type, name, fmtId(grantee->data)); if (privs->len > 0) appendPQExpBuffer(firstsql, "GRANT %s ON %s %s TO %s;\n", ! privs->data, type, name, fmtId(grantee->data)); if (privswgo->len > 0) appendPQExpBuffer(firstsql, "GRANT %s ON %s %s TO %s WITH GRANT OPTION;\n", ! privswgo->data, type, name, fmtId(grantee->data)); } } --- 432,446 ---- : strcmp(privs->data, "ALL") != 0) { appendPQExpBuffer(firstsql, "REVOKE ALL ON %s %s FROM %s;\n", ! outType, name, fmtId(grantee->data)); if (privs->len > 0) appendPQExpBuffer(firstsql, "GRANT %s ON %s %s TO %s;\n", ! privs->data, outType, name, fmtId(grantee->data)); if (privswgo->len > 0) appendPQExpBuffer(firstsql, "GRANT %s ON %s %s TO %s WITH GRANT OPTION;\n", ! privswgo->data, outType, name, fmtId(grantee->data)); } } *************** *** 444,450 **** if (privs->len > 0) { appendPQExpBuffer(secondsql, "GRANT %s ON %s %s TO ", ! privs->data, type, name); if (grantee->len == 0) appendPQExpBuffer(secondsql, "PUBLIC;\n"); else if (strncmp(grantee->data, "group ", --- 457,463 ---- if (privs->len > 0) { appendPQExpBuffer(secondsql, "GRANT %s ON %s %s TO ", ! privs->data, outType, name); if (grantee->len == 0) appendPQExpBuffer(secondsql, "PUBLIC;\n"); else if (strncmp(grantee->data, "group ", *************** *** 457,463 **** if (privswgo->len > 0) { appendPQExpBuffer(secondsql, "GRANT %s ON %s %s TO ", ! privswgo->data, type, name); if (grantee->len == 0) appendPQExpBuffer(secondsql, "PUBLIC"); else if (strncmp(grantee->data, "group ", --- 470,476 ---- if (privswgo->len > 0) { appendPQExpBuffer(secondsql, "GRANT %s ON %s %s TO ", ! privswgo->data, outType, name); if (grantee->len == 0) appendPQExpBuffer(secondsql, "PUBLIC"); else if (strncmp(grantee->data, "group ", *************** *** 480,489 **** * If we didn't find any owner privs, the owner must have revoked 'em all */ if (!found_owner_privs && owner) - { appendPQExpBuffer(firstsql, "REVOKE ALL ON %s %s FROM %s;\n", type, name, fmtId(owner)); - } destroyPQExpBuffer(grantee); destroyPQExpBuffer(grantor); --- 493,500 ---- *************** *** 517,523 **** */ static bool parseAclItem(const char *item, const char *type, const char *name, ! int remoteVersion, PQExpBuffer grantee, PQExpBuffer grantor, PQExpBuffer privs, PQExpBuffer privswgo) { --- 528,534 ---- */ static bool parseAclItem(const char *item, const char *type, const char *name, ! int remoteVersion, bool *is_valid_for_sequence, PQExpBuffer grantee, PQExpBuffer grantor, PQExpBuffer privs, PQExpBuffer privswgo) { *************** *** 530,535 **** --- 541,548 ---- buf = strdup(item); + *is_valid_for_sequence = true; + /* user or group name is string up to = */ eqpos = copyAclUserName(grantee, buf); if (*eqpos != '=') *************** *** 547,554 **** --- 560,573 ---- else resetPQExpBuffer(grantor); + if (strcmp(type, "SEQUENCE") == 0 && + /* SELECT, USAGE, UPDATE, ALL */ + strspn(eqpos + 1, "rUw*") != strlen(eqpos + 1)) + *is_valid_for_sequence = false; + /* privilege codes */ #define CONVERT_PRIV(code, keywd) \ + do { \ if ((pos = strchr(eqpos + 1, code))) \ { \ if (*(pos + 1) == '*') \ *************** *** 563,578 **** } \ } \ else \ ! all_with_go = all_without_go = false resetPQExpBuffer(privs); resetPQExpBuffer(privswgo); ! if (strcmp(type, "TABLE") == 0) { CONVERT_PRIV('a', "INSERT"); CONVERT_PRIV('r', "SELECT"); CONVERT_PRIV('R', "RULE"); if (remoteVersion >= 70200) { --- 582,600 ---- } \ } \ else \ ! all_with_go = all_without_go = false; \ ! } while (0) resetPQExpBuffer(privs); resetPQExpBuffer(privswgo); ! if (strcmp(type, "TABLE") == 0 || strcmp(type, "SEQUENCE") == 0) { CONVERT_PRIV('a', "INSERT"); CONVERT_PRIV('r', "SELECT"); CONVERT_PRIV('R', "RULE"); + if (strcmp(type, "SEQUENCE") == 0) + CONVERT_PRIV('U', "USAGE"); if (remoteVersion >= 70200) { Index: src/bin/pg_dump/pg_dump.c =================================================================== RCS file: /cvsroot/pgsql/src/bin/pg_dump/pg_dump.c,v retrieving revision 1.425 diff -c -c -r1.425 pg_dump.c *** src/bin/pg_dump/pg_dump.c 6 Jan 2006 19:08:33 -0000 1.425 --- src/bin/pg_dump/pg_dump.c 7 Jan 2006 06:00:49 -0000 *************** *** 6776,6782 **** /* Handle the ACL here */ namecopy = strdup(fmtId(tbinfo->dobj.name)); ! dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId, "TABLE", namecopy, tbinfo->dobj.name, tbinfo->dobj.namespace->dobj.name, tbinfo->rolname, tbinfo->relacl); --- 6776,6784 ---- /* Handle the ACL here */ namecopy = strdup(fmtId(tbinfo->dobj.name)); ! dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId, ! /* Issue GRANT SEQUENCE, if applicable */ ! tbinfo->relkind != RELKIND_SEQUENCE ? "TABLE" : "SEQUENCE", namecopy, tbinfo->dobj.name, tbinfo->dobj.namespace->dobj.name, tbinfo->rolname, tbinfo->relacl); Index: src/include/nodes/parsenodes.h =================================================================== RCS file: /cvsroot/pgsql/src/include/nodes/parsenodes.h,v retrieving revision 1.298 diff -c -c -r1.298 parsenodes.h *** src/include/nodes/parsenodes.h 7 Dec 2005 15:20:55 -0000 1.298 --- src/include/nodes/parsenodes.h 7 Jan 2006 06:00:52 -0000 *************** *** 884,890 **** */ typedef enum GrantObjectType { ! ACL_OBJECT_RELATION, /* table, view, sequence */ ACL_OBJECT_DATABASE, /* database */ ACL_OBJECT_FUNCTION, /* function */ ACL_OBJECT_LANGUAGE, /* procedural language */ --- 884,891 ---- */ typedef enum GrantObjectType { ! ACL_OBJECT_RELATION, /* table, view */ ! ACL_OBJECT_SEQUENCE, /* sequence */ ACL_OBJECT_DATABASE, /* database */ ACL_OBJECT_FUNCTION, /* function */ ACL_OBJECT_LANGUAGE, /* procedural language */ Index: src/include/utils/acl.h =================================================================== RCS file: /cvsroot/pgsql/src/include/utils/acl.h,v retrieving revision 1.91 diff -c -c -r1.91 acl.h *** src/include/utils/acl.h 1 Dec 2005 02:03:01 -0000 1.91 --- src/include/utils/acl.h 7 Jan 2006 06:00:52 -0000 *************** *** 143,148 **** --- 143,149 ---- * Bitmasks defining "all rights" for each supported object type */ #define ACL_ALL_RIGHTS_RELATION (ACL_INSERT|ACL_SELECT|ACL_UPDATE|ACL_DELETE|ACL_RULE|ACL_REFERENCES|ACL_TRIGGER) + #define ACL_ALL_RIGHTS_SEQUENCE (ACL_SELECT|ACL_UPDATE|ACL_USAGE) #define ACL_ALL_RIGHTS_DATABASE (ACL_CREATE|ACL_CREATE_TEMP) #define ACL_ALL_RIGHTS_FUNCTION (ACL_EXECUTE) #define ACL_ALL_RIGHTS_LANGUAGE (ACL_USAGE)
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Should UPDATE also allow currval()? Your logic below seems to suggest
> that.
I thought about that, but there are a couple of reasons not to:
1. It'd be a change from the current behavior of UPDATE privilege.
2. If there's someone out there who really does want write-only
privileges for sequences, they'd be out in the cold.
I don't find either of these very compelling, but the case for changing
the behavior of UPDATE isn't strong either. I think backwards
compatibility should carry the day if there's not a strong argument
in favor of change.
regards, tom lane
On 1/7/06, Bruce Momjian <pgman@candle.pha.pa.us> wrote: > Marko Kreen wrote: > > The above table seem bit messy, but I see it as much easier to explain > > to somebody. > > I am confused about your list above, so I can't see how that would be > easy to explain. Easy as in "use GRANT USAGE, forget about rest". You are confused because you know the old way and look them together. I would have liked to say "the rest are for fine-grained access control", but with Tom's final proposal, the explanation would continue "SELECT, UPDATE are for backwards compatibility". Attached is a patch that fixes tablename->seqname and puts USAGE as first in list to show it's the preferred way. I think it should be mentioned somewhere explicitly, but I cant find proper place for it. In the Compatibility section for GRANT? -- marko
Attachment
Tom, all, > SELECT: currval > UPDATE: nextval, setval > USAGE: nextval, currval +1. --Josh
Bruce Momjian wrote:
> Tom Lane wrote:
> > I wrote:
> > > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > >> Does the standard require USAGE to support currval?
> >
> > > currval isn't in the standard (unless I missed something), so it has
> > > nothing to say one way or the other on the point.
> >
> > Wait, I take that back. Remember our previous discussions about this
> > point: the spec's NEXT VALUE FOR construct is *not* equivalent to
> > nextval, because they specify that the sequence advances just once per
> > command even if the command says NEXT VALUE FOR in multiple places.
> > This means that NEXT VALUE FOR is effectively both nextval and currval;
> > the first one in a command does nextval and the rest do currval.
> >
> > Accordingly, I think it's reasonable to read the spec as saying that
> > USAGE privilege encompasses both nextval and currval.
>
> Here's a patch that more closely matches the ideas proposed.
Here is an updated patch. I hit a few issues.
At first I was just going to continue allowing table-like permissions
for sequences if a GRANT [TABLE] was used, and add the new
USAGE/SELECT/UPDATE capability only for GRANT SEQUENCE. The problem was
that you could create a non-dumpable permission setup if you added
DELETE permission to a sequence using GRANT TABLE, and USAGE permission
using GRANT SEQUENCE. That couldn't be dumped with TABLE or with
SEQUENCE, and I didn't want to do a double-dump of GRANT to fit that,
nor did I want to throw an warning during the dump run.
What I did was to throw a warning if an invalid permission is specified
for a sequence in GRANT TABLE. By doing this, un-dumpable permission
combinations will not be loaded into an 8.2 database. (GRANT ALL ON
TABLE sets the sequence-only permissions.)
test=> GRANT DELETE ON seq TO PUBLIC;
WARNING: invalid privilege type DELETE for sequence
WARNING: no privileges were granted
GRANT
test=> GRANT DELETE,SELECT ON seq TO PUBLIC;
WARNING: invalid privilege type DELETE for sequence
GRANT
This seemed the safest backward-compatible setup. It will have to be
mentioned in the release notes so users know they might get warnings
from loading sequences into 8.2.
You might think that it is unlikely for a DELETE permission to be
assigned to a sequences, but a simple GRANT ALL and REVOKE INSERT in 8.1
will cause:
test=> CREATE TABLE tab(x INTEGER);
CREATE TABLE
test=> GRANT ALL ON tab TO PUBLIC;
GRANT
test=> REVOKE INSERT ON tab FROM PUBLIC;
REVOKE
yields in pg_dump output:
GRANT SELECT,RULE,UPDATE,DELETE,REFERENCES,TRIGGER ON TABLE tab
TO PUBLIC;
This test was done on a table, but in 8.1 the same would appear for a
sequence with these warnings on load into 8.2:
WARNING: invalid privilege type RULE for sequence
WARNING: invalid privilege type DELETE for sequence
WARNING: invalid privilege type REFERENCES for sequence
WARNING: invalid privilege type TRIGGER for sequence
GRANT
Another tricky case was this:
test=> GRANT DELETE ON tab, seq TO PUBLIC;
GRANT allows multiple objects to be listed, as illustrated above. The
current code checks for valid permissions in one place because it
assumes all listed objects are of the same type and accept the same
permissions. Because GRANT TABLE must allow only valid permissions for
sequences (to avoid un-dumpable output) I had to throw an error if a
sequence is mixed with a non-sequence, and the permission did not apply
to both sequences and non-sequences, rather than throw a warning like I
usually do for invalid sequence permissions:
test=> REVOKE DELETE ON seq, tab FROM PUBLIC;
WARNING: invalid privilege type DELETE for sequence
ERROR: DELETE privilege invalid for command mixing sequences and non-sequences
test=> REVOKE SELECT ON tab, seq FROM PUBLIC;
REVOKE
Because allowing sequences to use GRANT TABLE is only for backward
compatibility, I think this is fine. If not, we would have to split
apart the permission checking for tables from the existing routine, and
lose modularity in the code.
This patch also contains Marko's documentation adjustments.
Would someone look at the change in src/backend/catalog/pg_shdepend.c
for shared dependencies? We don't have any system catalog sequences let
alone any shared catalog sequences, so I assume we are OK with assuming
it is a relation. I added a comment just in case.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Index: doc/src/sgml/ref/grant.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/ref/grant.sgml,v
retrieving revision 1.50
diff -c -c -r1.50 grant.sgml
*** doc/src/sgml/ref/grant.sgml 20 Oct 2005 19:18:01 -0000 1.50
--- doc/src/sgml/ref/grant.sgml 10 Jan 2006 01:18:32 -0000
***************
*** 25,30 ****
--- 25,35 ----
ON [ TABLE ] <replaceable class="PARAMETER">tablename</replaceable> [, ...]
TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable
class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ]
+ GRANT { { USAGE | SELECT | UPDATE }
+ [,...] | ALL [ PRIVILEGES ] }
+ ON SEQUENCE <replaceable class="PARAMETER">sequencename</replaceable> [, ...]
+ TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable
class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ]
+
GRANT { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] }
ON DATABASE <replaceable>dbname</replaceable> [, ...]
TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable
class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ]
***************
*** 260,265 ****
--- 265,274 ----
also met). Essentially this allows the grantee to <quote>look up</>
objects within the schema.
</para>
+ <para>
+ For sequences, this privilege allows the use of the
+ <function>currval</function> and <function>nextval</function> functions.
+ </para>
</listitem>
</varlistentry>
***************
*** 511,517 ****
<para>
The <literal>RULE</literal> privilege, and privileges on
! databases, tablespaces, schemas, languages, and sequences are
<productname>PostgreSQL</productname> extensions.
</para>
</refsect1>
--- 520,526 ----
<para>
The <literal>RULE</literal> privilege, and privileges on
! databases, tablespaces, schemas, and languages are
<productname>PostgreSQL</productname> extensions.
</para>
</refsect1>
Index: doc/src/sgml/ref/revoke.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/ref/revoke.sgml,v
retrieving revision 1.35
diff -c -c -r1.35 revoke.sgml
*** doc/src/sgml/ref/revoke.sgml 20 Oct 2005 19:18:01 -0000 1.35
--- doc/src/sgml/ref/revoke.sgml 10 Jan 2006 01:18:32 -0000
***************
*** 28,33 ****
--- 28,40 ----
[ CASCADE | RESTRICT ]
REVOKE [ GRANT OPTION FOR ]
+ { { USAGE | SELECT | UPDATE }
+ [,...] | ALL [ PRIVILEGES ] }
+ ON SEQUENCE <replaceable class="PARAMETER">sequencename</replaceable> [, ...]
+ FROM { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable
class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...]
+ [ CASCADE | RESTRICT ]
+
+ REVOKE [ GRANT OPTION FOR ]
{ { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] }
ON DATABASE <replaceable>dbname</replaceable> [, ...]
FROM { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable
class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...]
Index: src/backend/catalog/aclchk.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/catalog/aclchk.c,v
retrieving revision 1.123
diff -c -c -r1.123 aclchk.c
*** src/backend/catalog/aclchk.c 1 Dec 2005 02:03:00 -0000 1.123
--- src/backend/catalog/aclchk.c 10 Jan 2006 01:18:34 -0000
***************
*** 164,169 ****
--- 164,172 ----
case ACL_KIND_CLASS:
whole_mask = ACL_ALL_RIGHTS_RELATION;
break;
+ case ACL_KIND_SEQUENCE:
+ whole_mask = ACL_ALL_RIGHTS_SEQUENCE;
+ break;
case ACL_KIND_DATABASE:
whole_mask = ACL_ALL_RIGHTS_DATABASE;
break;
***************
*** 277,319 ****
get_roleid_checked(grantee->rolname));
}
- /*
- * Convert stmt->privileges, a textual list, into an AclMode bitmask.
- */
- switch (stmt->objtype)
- {
- case ACL_OBJECT_RELATION:
- all_privileges = ACL_ALL_RIGHTS_RELATION;
- errormsg = _("invalid privilege type %s for table");
- break;
- case ACL_OBJECT_DATABASE:
- all_privileges = ACL_ALL_RIGHTS_DATABASE;
- errormsg = _("invalid privilege type %s for database");
- break;
- case ACL_OBJECT_FUNCTION:
- all_privileges = ACL_ALL_RIGHTS_FUNCTION;
- errormsg = _("invalid privilege type %s for function");
- break;
- case ACL_OBJECT_LANGUAGE:
- all_privileges = ACL_ALL_RIGHTS_LANGUAGE;
- errormsg = _("invalid privilege type %s for language");
- break;
- case ACL_OBJECT_NAMESPACE:
- all_privileges = ACL_ALL_RIGHTS_NAMESPACE;
- errormsg = _("invalid privilege type %s for namespace");
- break;
- case ACL_OBJECT_TABLESPACE:
- all_privileges = ACL_ALL_RIGHTS_TABLESPACE;
- errormsg = _("invalid privilege type %s for tablespace");
- break;
- default:
- /* keep compiler quiet */
- all_privileges = ACL_NO_RIGHTS;
- errormsg = NULL;
- elog(ERROR, "unrecognized GrantStmt.objtype: %d",
- (int) stmt->objtype);
- }
-
if (stmt->privileges == NIL)
{
istmt.all_privs = true;
--- 280,285 ----
***************
*** 327,343 ****
{
istmt.all_privs = false;
istmt.privileges = ACL_NO_RIGHTS;
foreach(cell, stmt->privileges)
{
char *privname = strVal(lfirst(cell));
AclMode priv = string_to_privilege(privname);
! if (priv & ~((AclMode) all_privileges))
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_GRANT_OPERATION),
! errmsg(errormsg,
privilege_to_string(priv))));
!
istmt.privileges |= priv;
}
}
--- 293,429 ----
{
istmt.all_privs = false;
istmt.privileges = ACL_NO_RIGHTS;
+
foreach(cell, stmt->privileges)
{
char *privname = strVal(lfirst(cell));
AclMode priv = string_to_privilege(privname);
! /*
! * The GRANT TABLE syntax can be used for sequences and
! * non-sequences, so we have to look at the relkind to
! * determine the supported permissions.
! */
! if (stmt->objtype == ACL_OBJECT_RELATION)
! {
! ListCell *cell2;
! bool skip_priv = false;
! bool non_seq_found = false;
!
! /*
! * GRANT can have sequences and non-sequence objects
! * in the same command, so loop over each object.
! */
! foreach(cell2, istmt.objects)
! {
! Oid relOid = lfirst_oid(cell2);
! Form_pg_class pg_class_tuple;
! HeapTuple tuple;
!
! tuple = SearchSysCache(RELOID,
! ObjectIdGetDatum(relOid),
! 0, 0, 0);
! if (!HeapTupleIsValid(tuple))
! elog(ERROR, "cache lookup failed for relation %u", relOid);
! pg_class_tuple = (Form_pg_class) GETSTRUCT(tuple);
!
! if (pg_class_tuple->relkind == RELKIND_SEQUENCE)
! {
! all_privileges = ACL_ALL_RIGHTS_SEQUENCE;
! errormsg = _("invalid privilege type %s for sequence");
! /*
! * For backward compatibility, throw just a warning
! * for invalid sequence permissions when using the
! * non-sequence GRANT syntax is used.
! */
! if (priv & ~((AclMode) ACL_ALL_RIGHTS_SEQUENCE))
! {
! ereport(WARNING,
! (errcode(ERRCODE_INVALID_GRANT_OPERATION),
! errmsg(_("invalid privilege type %s for sequence"),
! privilege_to_string(priv))));
! /* Skip assigning this priviledge */
! skip_priv = true;
! }
! }
! else
! {
! if (priv & ~((AclMode) ACL_ALL_RIGHTS_RELATION))
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_GRANT_OPERATION),
! errmsg(_("invalid privilege type %s for table"),
! privilege_to_string(priv))));
! non_seq_found = true;
! }
! ReleaseSysCache(tuple);
! }
! /* If we get here, we have issued only warnings */
! if (skip_priv)
! {
! if (non_seq_found)
! /*
! * If we get here, someone has issued a command like:
! *
! * GRANT DELETE ON tab, seq TO PUBLIC
! *
! * In thise case, the DELETE is valid for the table
! * but not for the sequences. We don't want to continue
! * processing with a permission that will only partly
! * succeed, so we ERROR.
! */
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_GRANT_OPERATION),
! errmsg(_("%s privilege invalid for command mixing sequences and non-sequences"),
privilege_to_string(priv))));
! priv = 0;
! }
! }
! else
! {
! /*
! * Convert stmt->privileges, a textual list, into an AclMode bitmask.
! */
! switch (stmt->objtype)
! {
! /* ACL_OBJECT_RELATION: handled above */
! case ACL_OBJECT_SEQUENCE:
! all_privileges = ACL_ALL_RIGHTS_SEQUENCE;
! errormsg = _("invalid privilege type %s for sequence");
! break;
! case ACL_OBJECT_DATABASE:
! all_privileges = ACL_ALL_RIGHTS_DATABASE;
! errormsg = _("invalid privilege type %s for database");
! break;
! case ACL_OBJECT_FUNCTION:
! all_privileges = ACL_ALL_RIGHTS_FUNCTION;
! errormsg = _("invalid privilege type %s for function");
! break;
! case ACL_OBJECT_LANGUAGE:
! all_privileges = ACL_ALL_RIGHTS_LANGUAGE;
! errormsg = _("invalid privilege type %s for language");
! break;
! case ACL_OBJECT_NAMESPACE:
! all_privileges = ACL_ALL_RIGHTS_NAMESPACE;
! errormsg = _("invalid privilege type %s for namespace");
! break;
! case ACL_OBJECT_TABLESPACE:
! all_privileges = ACL_ALL_RIGHTS_TABLESPACE;
! errormsg = _("invalid privilege type %s for tablespace");
! break;
! default:
! /* keep compiler quiet */
! all_privileges = ACL_NO_RIGHTS;
! errormsg = NULL;
! elog(ERROR, "unrecognized GrantStmt.objtype: %d",
! (int) stmt->objtype);
! }
! if (priv & ~((AclMode) all_privileges))
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_GRANT_OPERATION),
! errmsg(errormsg,
! privilege_to_string(priv))));
! }
!
istmt.privileges |= priv;
}
}
***************
*** 356,361 ****
--- 442,448 ----
switch (istmt->objtype)
{
case ACL_OBJECT_RELATION:
+ case ACL_OBJECT_SEQUENCE:
ExecGrant_Relation(istmt);
break;
case ACL_OBJECT_DATABASE:
***************
*** 395,400 ****
--- 482,488 ----
switch (objtype)
{
case ACL_OBJECT_RELATION:
+ case ACL_OBJECT_SEQUENCE:
foreach(cell, objnames)
{
Oid relOid;
***************
*** 523,537 ****
return objects;
}
static void
ExecGrant_Relation(InternalGrant *istmt)
{
Relation relation;
ListCell *cell;
- if (istmt->all_privs && istmt->privileges == ACL_NO_RIGHTS)
- istmt->privileges = ACL_ALL_RIGHTS_RELATION;
-
relation = heap_open(RelationRelationId, RowExclusiveLock);
foreach(cell, istmt->objects)
--- 611,625 ----
return objects;
}
+ /*
+ * This processes both sequences and non-sequences.
+ */
static void
ExecGrant_Relation(InternalGrant *istmt)
{
Relation relation;
ListCell *cell;
relation = heap_open(RelationRelationId, RowExclusiveLock);
foreach(cell, istmt->objects)
***************
*** 577,582 ****
--- 665,689 ----
errmsg("\"%s\" is a composite type",
NameStr(pg_class_tuple->relname))));
+ /* Used GRANT SEQUENCE on a non-sequence? */
+ if (istmt->objtype == ACL_OBJECT_SEQUENCE &&
+ pg_class_tuple->relkind != RELKIND_SEQUENCE)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is not a sequence",
+ NameStr(pg_class_tuple->relname))));
+
+ /* Adjust the default permissions based on whether it is a sequence */
+ if (istmt->all_privs && istmt->privileges == ACL_NO_RIGHTS)
+ {
+ if (pg_class_tuple->relkind == RELKIND_SEQUENCE)
+ this_privileges = ACL_ALL_RIGHTS_SEQUENCE;
+ else
+ this_privileges = ACL_ALL_RIGHTS_RELATION;
+ }
+ else
+ this_privileges = istmt->privileges;
+
/*
* Get owner ID and working copy of existing ACL. If there's no ACL,
* substitute the proper default.
***************
*** 585,596 ****
aclDatum = SysCacheGetAttr(RELOID, tuple, Anum_pg_class_relacl,
&isNull);
if (isNull)
! old_acl = acldefault(ACL_OBJECT_RELATION, ownerId);
else
old_acl = DatumGetAclPCopy(aclDatum);
/* Determine ID to do the grant as, and available grant options */
! select_best_grantor(GetUserId(), istmt->privileges,
old_acl, ownerId,
&grantorId, &avail_goptions);
--- 692,705 ----
aclDatum = SysCacheGetAttr(RELOID, tuple, Anum_pg_class_relacl,
&isNull);
if (isNull)
! old_acl = acldefault(pg_class_tuple->relkind == RELKIND_SEQUENCE ?
! ACL_OBJECT_SEQUENCE : ACL_OBJECT_RELATION,
! ownerId);
else
old_acl = DatumGetAclPCopy(aclDatum);
/* Determine ID to do the grant as, and available grant options */
! select_best_grantor(GetUserId(), this_privileges,
old_acl, ownerId,
&grantorId, &avail_goptions);
***************
*** 600,607 ****
*/
this_privileges =
restrict_and_check_grant(istmt->is_grant, avail_goptions,
! istmt->all_privs, istmt->privileges,
! relOid, grantorId, ACL_KIND_CLASS,
NameStr(pg_class_tuple->relname));
/*
--- 709,718 ----
*/
this_privileges =
restrict_and_check_grant(istmt->is_grant, avail_goptions,
! istmt->all_privs, this_privileges,
! relOid, grantorId,
! pg_class_tuple->relkind == RELKIND_SEQUENCE
! ? ACL_KIND_SEQUENCE : ACL_KIND_CLASS,
NameStr(pg_class_tuple->relname));
/*
***************
*** 1336,1341 ****
--- 1447,1454 ----
{
/* ACL_KIND_CLASS */
gettext_noop("permission denied for relation %s"),
+ /* ACL_KIND_SEQUENCE */
+ gettext_noop("permission denied for sequence %s"),
/* ACL_KIND_DATABASE */
gettext_noop("permission denied for database %s"),
/* ACL_KIND_PROC */
***************
*** 1360,1365 ****
--- 1473,1480 ----
{
/* ACL_KIND_CLASS */
gettext_noop("must be owner of relation %s"),
+ /* ACL_KIND_SEQUENCE */
+ gettext_noop("must be owner of sequence %s"),
/* ACL_KIND_DATABASE */
gettext_noop("must be owner of database %s"),
/* ACL_KIND_PROC */
***************
*** 1439,1444 ****
--- 1554,1560 ----
switch (objkind)
{
case ACL_KIND_CLASS:
+ case ACL_KIND_SEQUENCE:
return pg_class_aclmask(table_oid, roleid, mask, how);
case ACL_KIND_DATABASE:
return pg_database_aclmask(table_oid, roleid, mask, how);
***************
*** 1500,1508 ****
*
* As of 7.4 we have some updatable system views; those shouldn't be
* protected in this way. Assume the view rules can take care of
! * themselves.
*/
! if ((mask & (ACL_INSERT | ACL_UPDATE | ACL_DELETE)) &&
IsSystemClass(classForm) &&
classForm->relkind != RELKIND_VIEW &&
!has_rolcatupdate(roleid) &&
--- 1616,1624 ----
*
* As of 7.4 we have some updatable system views; those shouldn't be
* protected in this way. Assume the view rules can take care of
! * themselves. ACL_USAGE is if we ever have system sequences.
*/
! if ((mask & (ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_USAGE)) &&
IsSystemClass(classForm) &&
classForm->relkind != RELKIND_VIEW &&
!has_rolcatupdate(roleid) &&
***************
*** 1511,1517 ****
#ifdef ACLDEBUG
elog(DEBUG2, "permission denied for system catalog update");
#endif
! mask &= ~(ACL_INSERT | ACL_UPDATE | ACL_DELETE);
}
/*
--- 1627,1633 ----
#ifdef ACLDEBUG
elog(DEBUG2, "permission denied for system catalog update");
#endif
! mask &= ~(ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_USAGE);
}
/*
***************
*** 1536,1542 ****
if (isNull)
{
/* No ACL, so build default ACL */
! acl = acldefault(ACL_OBJECT_RELATION, ownerId);
aclDatum = (Datum) 0;
}
else
--- 1652,1660 ----
if (isNull)
{
/* No ACL, so build default ACL */
! acl = acldefault(classForm->relkind == RELKIND_SEQUENCE ?
! ACL_OBJECT_SEQUENCE : ACL_OBJECT_RELATION,
! ownerId);
aclDatum = (Datum) 0;
}
else
Index: src/backend/catalog/pg_shdepend.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/catalog/pg_shdepend.c,v
retrieving revision 1.6
diff -c -c -r1.6 pg_shdepend.c
*** src/backend/catalog/pg_shdepend.c 1 Dec 2005 02:03:00 -0000 1.6
--- src/backend/catalog/pg_shdepend.c 10 Jan 2006 01:18:35 -0000
***************
*** 1133,1138 ****
--- 1133,1139 ----
switch (sdepForm->classid)
{
case RelationRelationId:
+ /* could be a sequence? */
istmt.objtype = ACL_OBJECT_RELATION;
break;
case DatabaseRelationId:
Index: src/backend/commands/sequence.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/sequence.c,v
retrieving revision 1.126
diff -c -c -r1.126 sequence.c
*** src/backend/commands/sequence.c 22 Nov 2005 18:17:09 -0000 1.126
--- src/backend/commands/sequence.c 10 Jan 2006 01:18:37 -0000
***************
*** 422,428 ****
/* open and AccessShareLock sequence */
init_sequence(relid, &elm, &seqrel);
! if (pg_class_aclcheck(elm->relid, GetUserId(), ACL_UPDATE) != ACLCHECK_OK)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied for sequence %s",
--- 422,429 ----
/* open and AccessShareLock sequence */
init_sequence(relid, &elm, &seqrel);
! if (pg_class_aclcheck(elm->relid, GetUserId(), ACL_USAGE) != ACLCHECK_OK &&
! pg_class_aclcheck(elm->relid, GetUserId(), ACL_UPDATE) != ACLCHECK_OK)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied for sequence %s",
***************
*** 613,619 ****
/* open and AccessShareLock sequence */
init_sequence(relid, &elm, &seqrel);
! if (pg_class_aclcheck(elm->relid, GetUserId(), ACL_SELECT) != ACLCHECK_OK)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied for sequence %s",
--- 614,621 ----
/* open and AccessShareLock sequence */
init_sequence(relid, &elm, &seqrel);
! if (pg_class_aclcheck(elm->relid, GetUserId(), ACL_SELECT) != ACLCHECK_OK &&
! pg_class_aclcheck(elm->relid, GetUserId(), ACL_USAGE) != ACLCHECK_OK)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied for sequence %s",
***************
*** 657,663 ****
/* nextval() must have already been called for this sequence */
Assert(last_used_seq->increment != 0);
! if (pg_class_aclcheck(last_used_seq->relid, GetUserId(), ACL_SELECT) != ACLCHECK_OK)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied for sequence %s",
--- 659,666 ----
/* nextval() must have already been called for this sequence */
Assert(last_used_seq->increment != 0);
! if (pg_class_aclcheck(last_used_seq->relid, GetUserId(), ACL_SELECT) != ACLCHECK_OK &&
! pg_class_aclcheck(last_used_seq->relid, GetUserId(), ACL_USAGE) != ACLCHECK_OK)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied for sequence %s",
Index: src/backend/parser/gram.y
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.521
diff -c -c -r2.521 gram.y
*** src/backend/parser/gram.y 29 Dec 2005 04:53:18 -0000 2.521
--- src/backend/parser/gram.y 10 Jan 2006 01:18:47 -0000
***************
*** 3322,3327 ****
--- 3322,3334 ----
n->objs = $2;
$$ = n;
}
+ | SEQUENCE qualified_name_list
+ {
+ PrivTarget *n = makeNode(PrivTarget);
+ n->objtype = ACL_OBJECT_SEQUENCE;
+ n->objs = $2;
+ $$ = n;
+ }
| FUNCTION function_with_argtypes_list
{
PrivTarget *n = makeNode(PrivTarget);
Index: src/backend/utils/adt/acl.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/acl.c,v
retrieving revision 1.129
diff -c -c -r1.129 acl.c
*** src/backend/utils/adt/acl.c 18 Nov 2005 02:38:23 -0000 1.129
--- src/backend/utils/adt/acl.c 10 Jan 2006 01:18:50 -0000
***************
*** 545,550 ****
--- 545,554 ----
world_default = ACL_NO_RIGHTS;
owner_default = ACL_ALL_RIGHTS_RELATION;
break;
+ case ACL_OBJECT_SEQUENCE:
+ world_default = ACL_NO_RIGHTS;
+ owner_default = ACL_ALL_RIGHTS_SEQUENCE;
+ break;
case ACL_OBJECT_DATABASE:
world_default = ACL_CREATE_TEMP; /* not NO_RIGHTS! */
owner_default = ACL_ALL_RIGHTS_DATABASE;
Index: src/bin/pg_dump/dumputils.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/pg_dump/dumputils.c,v
retrieving revision 1.23
diff -c -c -r1.23 dumputils.c
*** src/bin/pg_dump/dumputils.c 3 Dec 2005 21:06:18 -0000 1.23
--- src/bin/pg_dump/dumputils.c 10 Jan 2006 01:18:51 -0000
***************
*** 22,28 ****
#define supports_grant_options(version) ((version) >= 70400)
static bool parseAclItem(const char *item, const char *type, const char *name,
! int remoteVersion,
PQExpBuffer grantee, PQExpBuffer grantor,
PQExpBuffer privs, PQExpBuffer privswgo);
static char *copyAclUserName(PQExpBuffer output, char *input);
--- 22,28 ----
#define supports_grant_options(version) ((version) >= 70400)
static bool parseAclItem(const char *item, const char *type, const char *name,
! int remoteVersion, bool *is_valid_for_sequence,
PQExpBuffer grantee, PQExpBuffer grantor,
PQExpBuffer privs, PQExpBuffer privswgo);
static char *copyAclUserName(PQExpBuffer output, char *input);
***************
*** 395,404 ****
--- 395,416 ----
/* Scan individual ACL items */
for (i = 0; i < naclitems; i++)
{
+ const char *outType = type;
+ bool is_valid_for_sequence;
+
if (!parseAclItem(aclitems[i], type, name, remoteVersion,
+ &is_valid_for_sequence,
grantee, grantor, privs, privswgo))
return false;
+ /*
+ * For backward compatibility, non-SEQUENCE GRANT statements issue
+ * warnings rather than errors for invalid sequence permissions.
+ * This should only happen in pre-8.2 databases.
+ */
+ if (strcmp(outType, "SEQUENCE") == 0 && !is_valid_for_sequence)
+ outType = "TABLE";
+
if (grantor->len == 0 && owner)
printfPQExpBuffer(grantor, "%s", owner);
***************
*** 419,433 ****
: strcmp(privs->data, "ALL") != 0)
{
appendPQExpBuffer(firstsql, "REVOKE ALL ON %s %s FROM %s;\n",
! type, name,
fmtId(grantee->data));
if (privs->len > 0)
appendPQExpBuffer(firstsql, "GRANT %s ON %s %s TO %s;\n",
! privs->data, type, name,
fmtId(grantee->data));
if (privswgo->len > 0)
appendPQExpBuffer(firstsql, "GRANT %s ON %s %s TO %s WITH GRANT OPTION;\n",
! privswgo->data, type, name,
fmtId(grantee->data));
}
}
--- 431,445 ----
: strcmp(privs->data, "ALL") != 0)
{
appendPQExpBuffer(firstsql, "REVOKE ALL ON %s %s FROM %s;\n",
! outType, name,
fmtId(grantee->data));
if (privs->len > 0)
appendPQExpBuffer(firstsql, "GRANT %s ON %s %s TO %s;\n",
! privs->data, outType, name,
fmtId(grantee->data));
if (privswgo->len > 0)
appendPQExpBuffer(firstsql, "GRANT %s ON %s %s TO %s WITH GRANT OPTION;\n",
! privswgo->data, outType, name,
fmtId(grantee->data));
}
}
***************
*** 444,450 ****
if (privs->len > 0)
{
appendPQExpBuffer(secondsql, "GRANT %s ON %s %s TO ",
! privs->data, type, name);
if (grantee->len == 0)
appendPQExpBuffer(secondsql, "PUBLIC;\n");
else if (strncmp(grantee->data, "group ",
--- 456,462 ----
if (privs->len > 0)
{
appendPQExpBuffer(secondsql, "GRANT %s ON %s %s TO ",
! privs->data, outType, name);
if (grantee->len == 0)
appendPQExpBuffer(secondsql, "PUBLIC;\n");
else if (strncmp(grantee->data, "group ",
***************
*** 457,463 ****
if (privswgo->len > 0)
{
appendPQExpBuffer(secondsql, "GRANT %s ON %s %s TO ",
! privswgo->data, type, name);
if (grantee->len == 0)
appendPQExpBuffer(secondsql, "PUBLIC");
else if (strncmp(grantee->data, "group ",
--- 469,475 ----
if (privswgo->len > 0)
{
appendPQExpBuffer(secondsql, "GRANT %s ON %s %s TO ",
! privswgo->data, outType, name);
if (grantee->len == 0)
appendPQExpBuffer(secondsql, "PUBLIC");
else if (strncmp(grantee->data, "group ",
***************
*** 480,489 ****
* If we didn't find any owner privs, the owner must have revoked 'em all
*/
if (!found_owner_privs && owner)
- {
appendPQExpBuffer(firstsql, "REVOKE ALL ON %s %s FROM %s;\n",
type, name, fmtId(owner));
- }
destroyPQExpBuffer(grantee);
destroyPQExpBuffer(grantor);
--- 492,499 ----
***************
*** 517,523 ****
*/
static bool
parseAclItem(const char *item, const char *type, const char *name,
! int remoteVersion,
PQExpBuffer grantee, PQExpBuffer grantor,
PQExpBuffer privs, PQExpBuffer privswgo)
{
--- 527,533 ----
*/
static bool
parseAclItem(const char *item, const char *type, const char *name,
! int remoteVersion, bool *is_valid_for_sequence,
PQExpBuffer grantee, PQExpBuffer grantor,
PQExpBuffer privs, PQExpBuffer privswgo)
{
***************
*** 530,535 ****
--- 540,547 ----
buf = strdup(item);
+ *is_valid_for_sequence = true;
+
/* user or group name is string up to = */
eqpos = copyAclUserName(grantee, buf);
if (*eqpos != '=')
***************
*** 547,554 ****
--- 559,572 ----
else
resetPQExpBuffer(grantor);
+ if (strcmp(type, "SEQUENCE") == 0 &&
+ /* SELECT, USAGE, UPDATE, ALL */
+ strspn(eqpos + 1, "rUw*") != strlen(eqpos + 1))
+ *is_valid_for_sequence = false;
+
/* privilege codes */
#define CONVERT_PRIV(code, keywd) \
+ do { \
if ((pos = strchr(eqpos + 1, code))) \
{ \
if (*(pos + 1) == '*') \
***************
*** 563,578 ****
} \
} \
else \
! all_with_go = all_without_go = false
resetPQExpBuffer(privs);
resetPQExpBuffer(privswgo);
! if (strcmp(type, "TABLE") == 0)
{
CONVERT_PRIV('a', "INSERT");
CONVERT_PRIV('r', "SELECT");
CONVERT_PRIV('R', "RULE");
if (remoteVersion >= 70200)
{
--- 581,599 ----
} \
} \
else \
! all_with_go = all_without_go = false; \
! } while (0)
resetPQExpBuffer(privs);
resetPQExpBuffer(privswgo);
! if (strcmp(type, "TABLE") == 0 || strcmp(type, "SEQUENCE") == 0)
{
CONVERT_PRIV('a', "INSERT");
CONVERT_PRIV('r', "SELECT");
CONVERT_PRIV('R', "RULE");
+ if (strcmp(type, "SEQUENCE") == 0)
+ CONVERT_PRIV('U', "USAGE");
if (remoteVersion >= 70200)
{
Index: src/bin/pg_dump/pg_dump.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/pg_dump/pg_dump.c,v
retrieving revision 1.426
diff -c -c -r1.426 pg_dump.c
*** src/bin/pg_dump/pg_dump.c 9 Jan 2006 21:16:17 -0000 1.426
--- src/bin/pg_dump/pg_dump.c 10 Jan 2006 01:18:56 -0000
***************
*** 6788,6794 ****
/* Handle the ACL here */
namecopy = strdup(fmtId(tbinfo->dobj.name));
! dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId, "TABLE",
namecopy, tbinfo->dobj.name,
tbinfo->dobj.namespace->dobj.name, tbinfo->rolname,
tbinfo->relacl);
--- 6788,6796 ----
/* Handle the ACL here */
namecopy = strdup(fmtId(tbinfo->dobj.name));
! dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId,
! /* Issue GRANT SEQUENCE, if applicable */
! tbinfo->relkind != RELKIND_SEQUENCE ? "TABLE" : "SEQUENCE",
namecopy, tbinfo->dobj.name,
tbinfo->dobj.namespace->dobj.name, tbinfo->rolname,
tbinfo->relacl);
Index: src/include/nodes/parsenodes.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/nodes/parsenodes.h,v
retrieving revision 1.298
diff -c -c -r1.298 parsenodes.h
*** src/include/nodes/parsenodes.h 7 Dec 2005 15:20:55 -0000 1.298
--- src/include/nodes/parsenodes.h 10 Jan 2006 01:18:58 -0000
***************
*** 884,890 ****
*/
typedef enum GrantObjectType
{
! ACL_OBJECT_RELATION, /* table, view, sequence */
ACL_OBJECT_DATABASE, /* database */
ACL_OBJECT_FUNCTION, /* function */
ACL_OBJECT_LANGUAGE, /* procedural language */
--- 884,891 ----
*/
typedef enum GrantObjectType
{
! ACL_OBJECT_RELATION, /* table, view */
! ACL_OBJECT_SEQUENCE, /* sequence */
ACL_OBJECT_DATABASE, /* database */
ACL_OBJECT_FUNCTION, /* function */
ACL_OBJECT_LANGUAGE, /* procedural language */
Index: src/include/utils/acl.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/utils/acl.h,v
retrieving revision 1.91
diff -c -c -r1.91 acl.h
*** src/include/utils/acl.h 1 Dec 2005 02:03:01 -0000 1.91
--- src/include/utils/acl.h 10 Jan 2006 01:18:58 -0000
***************
*** 143,148 ****
--- 143,149 ----
* Bitmasks defining "all rights" for each supported object type
*/
#define ACL_ALL_RIGHTS_RELATION
(ACL_INSERT|ACL_SELECT|ACL_UPDATE|ACL_DELETE|ACL_RULE|ACL_REFERENCES|ACL_TRIGGER)
+ #define ACL_ALL_RIGHTS_SEQUENCE (ACL_USAGE|ACL_SELECT|ACL_UPDATE)
#define ACL_ALL_RIGHTS_DATABASE (ACL_CREATE|ACL_CREATE_TEMP)
#define ACL_ALL_RIGHTS_FUNCTION (ACL_EXECUTE)
#define ACL_ALL_RIGHTS_LANGUAGE (ACL_USAGE)
***************
*** 169,174 ****
--- 170,176 ----
typedef enum AclObjectKind
{
ACL_KIND_CLASS, /* pg_class */
+ ACL_KIND_SEQUENCE, /* pg_sequence */
ACL_KIND_DATABASE, /* pg_database */
ACL_KIND_PROC, /* pg_proc */
ACL_KIND_OPER, /* pg_operator */
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> At first I was just going to continue allowing table-like permissions
> for sequences if a GRANT [TABLE] was used, and add the new
> USAGE/SELECT/UPDATE capability only for GRANT SEQUENCE. The problem was
> that you could create a non-dumpable permission setup if you added
> DELETE permission to a sequence using GRANT TABLE, and USAGE permission
> using GRANT SEQUENCE. That couldn't be dumped with TABLE or with
> SEQUENCE, and I didn't want to do a double-dump of GRANT to fit that,
> nor did I want to throw an warning during the dump run.
Just ignore the inapplicable permissions during pg_dump. I think you're
making this harder than it needs to be...
> test=> REVOKE DELETE ON seq, tab FROM PUBLIC;
> WARNING: invalid privilege type DELETE for sequence
> ERROR: DELETE privilege invalid for command mixing sequences and non-sequences
This is just plain silly. If you're going to go to that length, why not
rearrange the code to avoid the problem instead?
> Would someone look at the change in src/backend/catalog/pg_shdepend.c
> for shared dependencies? We don't have any system catalog sequences let
> alone any shared catalog sequences, so I assume we are OK with assuming
> it is a relation.
We might have such in the future though.
regards, tom lane
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > At first I was just going to continue allowing table-like permissions
> > for sequences if a GRANT [TABLE] was used, and add the new
> > USAGE/SELECT/UPDATE capability only for GRANT SEQUENCE. The problem was
> > that you could create a non-dumpable permission setup if you added
> > DELETE permission to a sequence using GRANT TABLE, and USAGE permission
> > using GRANT SEQUENCE. That couldn't be dumped with TABLE or with
> > SEQUENCE, and I didn't want to do a double-dump of GRANT to fit that,
> > nor did I want to throw an warning during the dump run.
>
> Just ignore the inapplicable permissions during pg_dump. I think you're
> making this harder than it needs to be...
I don't think we should allow GRANT DELETE ON seq in 8.2 for invalid
permission. If we are going to say what permissions are supported by
sequences, we should enforce that rather than silently accept it, and
once we decide that, we need infrastructure to treat sequences and
non-sequences differently.
The dump illustration is just another example of why we have to tighten
this. I would be OK to allow it and preserve it, but not to allow it
and ignore it in a dump, basically throwing it away from dump to reload.
The fact we can't preserve it drove me in this direction.
> > test=> REVOKE DELETE ON seq, tab FROM PUBLIC;
> > WARNING: invalid privilege type DELETE for sequence
> > ERROR: DELETE privilege invalid for command mixing sequences and non-sequences
>
> This is just plain silly. If you're going to go to that length, why not
> rearrange the code to avoid the problem instead?
Ignoring your insult, the code is structured this way:
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 could still do
that single permission check in each object-type-specific routine and
just do the loop for the GRANT TABLE case. Does that seem worth it?
Another solution would be to save the permission bits in a List one per
object rather than as a single value for all objects, but that mucks up
the API for all objects just to catch the sequence case of GRANT TABLE.
> > Would someone look at the change in src/backend/catalog/pg_shdepend.c
> > for shared dependencies? We don't have any system catalog sequences let
> > alone any shared catalog sequences, so I assume we are OK with assuming
> > it is a relation.
>
> We might have such in the future though.
The problem is that the case statement triggers off of
RelationRelationId:
switch (sdepForm->classid)
{
case RelationRelationId:
/* could be a sequence? */
istmt.objtype = ACL_OBJECT_RELATION;
The problem is that pg_class holds both sequences and non-sequences, so
I am suspecting the fix will require another field in the pg_shdepend
table, which seems wasteful considering we have no shared catalog
sequences.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce Momjian <pgman@candle.pha.pa.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...
> I don't think we should allow GRANT DELETE ON seq in 8.2 for invalid
> permission.
That's fine, but pg_dump has to continue to work against old servers,
so it's going to have to be coded to ignore inapplicable permissions
anyway. Contorting the server-side code to avoid that is pointless.
> Ignoring your insult, the code is structured this way:
> 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.
regards, tom lane
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.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@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Index: doc/src/sgml/ref/grant.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/ref/grant.sgml,v
retrieving revision 1.50
diff -c -c -r1.50 grant.sgml
*** doc/src/sgml/ref/grant.sgml 20 Oct 2005 19:18:01 -0000 1.50
--- doc/src/sgml/ref/grant.sgml 11 Jan 2006 22:40:11 -0000
***************
*** 25,30 ****
--- 25,35 ----
ON [ TABLE ] <replaceable class="PARAMETER">tablename</replaceable> [, ...]
TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable
class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ]
+ GRANT { { USAGE | SELECT | UPDATE }
+ [,...] | ALL [ PRIVILEGES ] }
+ ON SEQUENCE <replaceable class="PARAMETER">sequencename</replaceable> [, ...]
+ TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable
class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ]
+
GRANT { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] }
ON DATABASE <replaceable>dbname</replaceable> [, ...]
TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable
class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ]
***************
*** 260,265 ****
--- 265,274 ----
also met). Essentially this allows the grantee to <quote>look up</>
objects within the schema.
</para>
+ <para>
+ For sequences, this privilege allows the use of the
+ <function>currval</function> and <function>nextval</function> functions.
+ </para>
</listitem>
</varlistentry>
***************
*** 511,517 ****
<para>
The <literal>RULE</literal> privilege, and privileges on
! databases, tablespaces, schemas, languages, and sequences are
<productname>PostgreSQL</productname> extensions.
</para>
</refsect1>
--- 520,526 ----
<para>
The <literal>RULE</literal> privilege, and privileges on
! databases, tablespaces, schemas, and languages are
<productname>PostgreSQL</productname> extensions.
</para>
</refsect1>
Index: doc/src/sgml/ref/revoke.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/ref/revoke.sgml,v
retrieving revision 1.35
diff -c -c -r1.35 revoke.sgml
*** doc/src/sgml/ref/revoke.sgml 20 Oct 2005 19:18:01 -0000 1.35
--- doc/src/sgml/ref/revoke.sgml 11 Jan 2006 22:40:11 -0000
***************
*** 28,33 ****
--- 28,40 ----
[ CASCADE | RESTRICT ]
REVOKE [ GRANT OPTION FOR ]
+ { { USAGE | SELECT | UPDATE }
+ [,...] | ALL [ PRIVILEGES ] }
+ ON SEQUENCE <replaceable class="PARAMETER">sequencename</replaceable> [, ...]
+ FROM { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable
class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...]
+ [ CASCADE | RESTRICT ]
+
+ REVOKE [ GRANT OPTION FOR ]
{ { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] }
ON DATABASE <replaceable>dbname</replaceable> [, ...]
FROM { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable
class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...]
Index: src/backend/catalog/aclchk.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/catalog/aclchk.c,v
retrieving revision 1.123
diff -c -c -r1.123 aclchk.c
*** src/backend/catalog/aclchk.c 1 Dec 2005 02:03:00 -0000 1.123
--- src/backend/catalog/aclchk.c 11 Jan 2006 22:40:12 -0000
***************
*** 164,169 ****
--- 164,172 ----
case ACL_KIND_CLASS:
whole_mask = ACL_ALL_RIGHTS_RELATION;
break;
+ case ACL_KIND_SEQUENCE:
+ whole_mask = ACL_ALL_RIGHTS_SEQUENCE;
+ break;
case ACL_KIND_DATABASE:
whole_mask = ACL_ALL_RIGHTS_DATABASE;
break;
***************
*** 212,233 ****
if (this_privileges == 0)
ereport(WARNING,
(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! errmsg("no privileges were granted")));
else if (!all_privs && this_privileges != privileges)
ereport(WARNING,
(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! errmsg("not all privileges were granted")));
}
else
{
if (this_privileges == 0)
ereport(WARNING,
(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! errmsg("no privileges could be revoked")));
else if (!all_privs && this_privileges != privileges)
ereport(WARNING,
(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! errmsg("not all privileges could be revoked")));
}
return this_privileges;
--- 215,236 ----
if (this_privileges == 0)
ereport(WARNING,
(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! errmsg("no privileges were granted for \"%s\"", objname)));
else if (!all_privs && this_privileges != privileges)
ereport(WARNING,
(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! errmsg("not all privileges were granted for \"%s\"", objname)));
}
else
{
if (this_privileges == 0)
ereport(WARNING,
(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! errmsg("no privileges could be revoked for \"%s\"", objname)));
else if (!all_privs && this_privileges != privileges)
ereport(WARNING,
(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! errmsg("not all privileges could be revoked for \"%s\"", objname)));
}
return this_privileges;
***************
*** 282,290 ****
*/
switch (stmt->objtype)
{
case ACL_OBJECT_RELATION:
! all_privileges = ACL_ALL_RIGHTS_RELATION;
! errormsg = _("invalid privilege type %s for table");
break;
case ACL_OBJECT_DATABASE:
all_privileges = ACL_ALL_RIGHTS_DATABASE;
--- 285,302 ----
*/
switch (stmt->objtype)
{
+ /*
+ * Because this might be a sequence, we test both relation
+ * and sequence bits, and later do a more limited test
+ * when we know the object type.
+ */
case ACL_OBJECT_RELATION:
! all_privileges = ACL_ALL_RIGHTS_RELATION | ACL_ALL_RIGHTS_SEQUENCE;
! errormsg = _("invalid privilege type %s for relation");
! break;
! case ACL_OBJECT_SEQUENCE:
! all_privileges = ACL_ALL_RIGHTS_SEQUENCE;
! errormsg = _("invalid privilege type %s for sequence");
break;
case ACL_OBJECT_DATABASE:
all_privileges = ACL_ALL_RIGHTS_DATABASE;
***************
*** 327,332 ****
--- 339,345 ----
{
istmt.all_privs = false;
istmt.privileges = ACL_NO_RIGHTS;
+
foreach(cell, stmt->privileges)
{
char *privname = strVal(lfirst(cell));
***************
*** 356,361 ****
--- 369,375 ----
switch (istmt->objtype)
{
case ACL_OBJECT_RELATION:
+ case ACL_OBJECT_SEQUENCE:
ExecGrant_Relation(istmt);
break;
case ACL_OBJECT_DATABASE:
***************
*** 395,400 ****
--- 409,415 ----
switch (objtype)
{
case ACL_OBJECT_RELATION:
+ case ACL_OBJECT_SEQUENCE:
foreach(cell, objnames)
{
Oid relOid;
***************
*** 523,537 ****
return objects;
}
static void
ExecGrant_Relation(InternalGrant *istmt)
{
Relation relation;
ListCell *cell;
- if (istmt->all_privs && istmt->privileges == ACL_NO_RIGHTS)
- istmt->privileges = ACL_ALL_RIGHTS_RELATION;
-
relation = heap_open(RelationRelationId, RowExclusiveLock);
foreach(cell, istmt->objects)
--- 538,552 ----
return objects;
}
+ /*
+ * This processes both sequences and non-sequences.
+ */
static void
ExecGrant_Relation(InternalGrant *istmt)
{
Relation relation;
ListCell *cell;
relation = heap_open(RelationRelationId, RowExclusiveLock);
foreach(cell, istmt->objects)
***************
*** 577,582 ****
--- 592,660 ----
errmsg("\"%s\" is a composite type",
NameStr(pg_class_tuple->relname))));
+ /* Used GRANT SEQUENCE on a non-sequence? */
+ if (istmt->objtype == ACL_OBJECT_SEQUENCE &&
+ pg_class_tuple->relkind != RELKIND_SEQUENCE)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is not a sequence",
+ NameStr(pg_class_tuple->relname))));
+
+ /* Adjust the default permissions based on whether it is a sequence */
+ if (istmt->all_privs && istmt->privileges == ACL_NO_RIGHTS)
+ {
+ if (pg_class_tuple->relkind == RELKIND_SEQUENCE)
+ this_privileges = ACL_ALL_RIGHTS_SEQUENCE;
+ else
+ this_privileges = ACL_ALL_RIGHTS_RELATION;
+ }
+ else
+ this_privileges = istmt->privileges;
+
+ /*
+ * The GRANT TABLE syntax can be used for sequences and
+ * non-sequences, so we have to look at the relkind to
+ * determine the supported permissions. The OR of
+ * table and sequence permissions were already checked.
+ */
+ if (istmt->objtype == ACL_OBJECT_RELATION)
+ {
+ if (pg_class_tuple->relkind == RELKIND_SEQUENCE)
+ {
+ /*
+ * For backward compatibility, throw just a warning
+ * for invalid sequence permissions when using the
+ * non-sequence GRANT syntax is used.
+ */
+ if (this_privileges & ~((AclMode) ACL_ALL_RIGHTS_SEQUENCE))
+ {
+ /*
+ * Mention the object name because the user needs to
+ * know which operations succeeded. This is required
+ * because WARNING allows the command to continue.
+ */
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_GRANT_OPERATION),
+ errmsg("sequence \"%s\" only supports USAGE, SELECT, and UPDATE",
+ NameStr(pg_class_tuple->relname))));
+ this_privileges &= (AclMode) ACL_ALL_RIGHTS_SEQUENCE;
+ }
+ }
+ else
+ {
+ if (this_privileges & ~((AclMode) ACL_ALL_RIGHTS_RELATION))
+ /*
+ * USAGE is the only permission supported by sequences
+ * but not by non-sequences. Don't mention the object
+ * name because we didn't in the combined TABLE |
+ * SEQUENCE check.
+ */
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_GRANT_OPERATION),
+ errmsg("invalid privilege type USAGE for table")));
+ }
+ }
+
/*
* Get owner ID and working copy of existing ACL. If there's no ACL,
* substitute the proper default.
***************
*** 585,596 ****
aclDatum = SysCacheGetAttr(RELOID, tuple, Anum_pg_class_relacl,
&isNull);
if (isNull)
! old_acl = acldefault(ACL_OBJECT_RELATION, ownerId);
else
old_acl = DatumGetAclPCopy(aclDatum);
/* Determine ID to do the grant as, and available grant options */
! select_best_grantor(GetUserId(), istmt->privileges,
old_acl, ownerId,
&grantorId, &avail_goptions);
--- 663,676 ----
aclDatum = SysCacheGetAttr(RELOID, tuple, Anum_pg_class_relacl,
&isNull);
if (isNull)
! old_acl = acldefault(pg_class_tuple->relkind == RELKIND_SEQUENCE ?
! ACL_OBJECT_SEQUENCE : ACL_OBJECT_RELATION,
! ownerId);
else
old_acl = DatumGetAclPCopy(aclDatum);
/* Determine ID to do the grant as, and available grant options */
! select_best_grantor(GetUserId(), this_privileges,
old_acl, ownerId,
&grantorId, &avail_goptions);
***************
*** 600,607 ****
*/
this_privileges =
restrict_and_check_grant(istmt->is_grant, avail_goptions,
! istmt->all_privs, istmt->privileges,
! relOid, grantorId, ACL_KIND_CLASS,
NameStr(pg_class_tuple->relname));
/*
--- 680,689 ----
*/
this_privileges =
restrict_and_check_grant(istmt->is_grant, avail_goptions,
! istmt->all_privs, this_privileges,
! relOid, grantorId,
! pg_class_tuple->relkind == RELKIND_SEQUENCE
! ? ACL_KIND_SEQUENCE : ACL_KIND_CLASS,
NameStr(pg_class_tuple->relname));
/*
***************
*** 1336,1341 ****
--- 1418,1425 ----
{
/* ACL_KIND_CLASS */
gettext_noop("permission denied for relation %s"),
+ /* ACL_KIND_SEQUENCE */
+ gettext_noop("permission denied for sequence %s"),
/* ACL_KIND_DATABASE */
gettext_noop("permission denied for database %s"),
/* ACL_KIND_PROC */
***************
*** 1360,1365 ****
--- 1444,1451 ----
{
/* ACL_KIND_CLASS */
gettext_noop("must be owner of relation %s"),
+ /* ACL_KIND_SEQUENCE */
+ gettext_noop("must be owner of sequence %s"),
/* ACL_KIND_DATABASE */
gettext_noop("must be owner of database %s"),
/* ACL_KIND_PROC */
***************
*** 1439,1444 ****
--- 1525,1531 ----
switch (objkind)
{
case ACL_KIND_CLASS:
+ case ACL_KIND_SEQUENCE:
return pg_class_aclmask(table_oid, roleid, mask, how);
case ACL_KIND_DATABASE:
return pg_database_aclmask(table_oid, roleid, mask, how);
***************
*** 1500,1508 ****
*
* As of 7.4 we have some updatable system views; those shouldn't be
* protected in this way. Assume the view rules can take care of
! * themselves.
*/
! if ((mask & (ACL_INSERT | ACL_UPDATE | ACL_DELETE)) &&
IsSystemClass(classForm) &&
classForm->relkind != RELKIND_VIEW &&
!has_rolcatupdate(roleid) &&
--- 1587,1595 ----
*
* As of 7.4 we have some updatable system views; those shouldn't be
* protected in this way. Assume the view rules can take care of
! * themselves. ACL_USAGE is if we ever have system sequences.
*/
! if ((mask & (ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_USAGE)) &&
IsSystemClass(classForm) &&
classForm->relkind != RELKIND_VIEW &&
!has_rolcatupdate(roleid) &&
***************
*** 1511,1517 ****
#ifdef ACLDEBUG
elog(DEBUG2, "permission denied for system catalog update");
#endif
! mask &= ~(ACL_INSERT | ACL_UPDATE | ACL_DELETE);
}
/*
--- 1598,1604 ----
#ifdef ACLDEBUG
elog(DEBUG2, "permission denied for system catalog update");
#endif
! mask &= ~(ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_USAGE);
}
/*
***************
*** 1536,1542 ****
if (isNull)
{
/* No ACL, so build default ACL */
! acl = acldefault(ACL_OBJECT_RELATION, ownerId);
aclDatum = (Datum) 0;
}
else
--- 1623,1631 ----
if (isNull)
{
/* No ACL, so build default ACL */
! acl = acldefault(classForm->relkind == RELKIND_SEQUENCE ?
! ACL_OBJECT_SEQUENCE : ACL_OBJECT_RELATION,
! ownerId);
aclDatum = (Datum) 0;
}
else
Index: src/backend/catalog/pg_shdepend.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/catalog/pg_shdepend.c,v
retrieving revision 1.6
diff -c -c -r1.6 pg_shdepend.c
*** src/backend/catalog/pg_shdepend.c 1 Dec 2005 02:03:00 -0000 1.6
--- src/backend/catalog/pg_shdepend.c 11 Jan 2006 22:40:12 -0000
***************
*** 1133,1140 ****
switch (sdepForm->classid)
{
case RelationRelationId:
! istmt.objtype = ACL_OBJECT_RELATION;
break;
case DatabaseRelationId:
istmt.objtype = ACL_OBJECT_DATABASE;
break;
--- 1133,1157 ----
switch (sdepForm->classid)
{
case RelationRelationId:
! {
! /* is it a sequence or non-sequence? */
! Form_pg_class pg_class_tuple;
! HeapTuple tuple;
!
! tuple = SearchSysCache(RELOID,
! ObjectIdGetDatum(sdepForm->objid),
! 0, 0, 0);
! if (!HeapTupleIsValid(tuple))
! elog(ERROR, "cache lookup failed for relation %u",
! sdepForm->objid);
! pg_class_tuple = (Form_pg_class) GETSTRUCT(tuple);
! if (pg_class_tuple->relkind == RELKIND_SEQUENCE)
! istmt.objtype = ACL_OBJECT_SEQUENCE;
! else
! istmt.objtype = ACL_OBJECT_RELATION;
! ReleaseSysCache(tuple);
break;
+ }
case DatabaseRelationId:
istmt.objtype = ACL_OBJECT_DATABASE;
break;
Index: src/backend/commands/sequence.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/sequence.c,v
retrieving revision 1.126
diff -c -c -r1.126 sequence.c
*** src/backend/commands/sequence.c 22 Nov 2005 18:17:09 -0000 1.126
--- src/backend/commands/sequence.c 11 Jan 2006 22:40:13 -0000
***************
*** 422,428 ****
/* open and AccessShareLock sequence */
init_sequence(relid, &elm, &seqrel);
! if (pg_class_aclcheck(elm->relid, GetUserId(), ACL_UPDATE) != ACLCHECK_OK)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied for sequence %s",
--- 422,429 ----
/* open and AccessShareLock sequence */
init_sequence(relid, &elm, &seqrel);
! if (pg_class_aclcheck(elm->relid, GetUserId(), ACL_USAGE) != ACLCHECK_OK &&
! pg_class_aclcheck(elm->relid, GetUserId(), ACL_UPDATE) != ACLCHECK_OK)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied for sequence %s",
***************
*** 613,619 ****
/* open and AccessShareLock sequence */
init_sequence(relid, &elm, &seqrel);
! if (pg_class_aclcheck(elm->relid, GetUserId(), ACL_SELECT) != ACLCHECK_OK)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied for sequence %s",
--- 614,621 ----
/* open and AccessShareLock sequence */
init_sequence(relid, &elm, &seqrel);
! if (pg_class_aclcheck(elm->relid, GetUserId(), ACL_SELECT) != ACLCHECK_OK &&
! pg_class_aclcheck(elm->relid, GetUserId(), ACL_USAGE) != ACLCHECK_OK)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied for sequence %s",
***************
*** 657,663 ****
/* nextval() must have already been called for this sequence */
Assert(last_used_seq->increment != 0);
! if (pg_class_aclcheck(last_used_seq->relid, GetUserId(), ACL_SELECT) != ACLCHECK_OK)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied for sequence %s",
--- 659,666 ----
/* nextval() must have already been called for this sequence */
Assert(last_used_seq->increment != 0);
! if (pg_class_aclcheck(last_used_seq->relid, GetUserId(), ACL_SELECT) != ACLCHECK_OK &&
! pg_class_aclcheck(last_used_seq->relid, GetUserId(), ACL_USAGE) != ACLCHECK_OK)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied for sequence %s",
Index: src/backend/parser/gram.y
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.521
diff -c -c -r2.521 gram.y
*** src/backend/parser/gram.y 29 Dec 2005 04:53:18 -0000 2.521
--- src/backend/parser/gram.y 11 Jan 2006 22:40:20 -0000
***************
*** 3322,3327 ****
--- 3322,3334 ----
n->objs = $2;
$$ = n;
}
+ | SEQUENCE qualified_name_list
+ {
+ PrivTarget *n = makeNode(PrivTarget);
+ n->objtype = ACL_OBJECT_SEQUENCE;
+ n->objs = $2;
+ $$ = n;
+ }
| FUNCTION function_with_argtypes_list
{
PrivTarget *n = makeNode(PrivTarget);
Index: src/backend/utils/adt/acl.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/acl.c,v
retrieving revision 1.129
diff -c -c -r1.129 acl.c
*** src/backend/utils/adt/acl.c 18 Nov 2005 02:38:23 -0000 1.129
--- src/backend/utils/adt/acl.c 11 Jan 2006 22:40:25 -0000
***************
*** 545,550 ****
--- 545,554 ----
world_default = ACL_NO_RIGHTS;
owner_default = ACL_ALL_RIGHTS_RELATION;
break;
+ case ACL_OBJECT_SEQUENCE:
+ world_default = ACL_NO_RIGHTS;
+ owner_default = ACL_ALL_RIGHTS_SEQUENCE;
+ break;
case ACL_OBJECT_DATABASE:
world_default = ACL_CREATE_TEMP; /* not NO_RIGHTS! */
owner_default = ACL_ALL_RIGHTS_DATABASE;
Index: src/bin/pg_dump/dumputils.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/pg_dump/dumputils.c,v
retrieving revision 1.24
diff -c -c -r1.24 dumputils.c
*** src/bin/pg_dump/dumputils.c 11 Jan 2006 21:24:30 -0000 1.24
--- src/bin/pg_dump/dumputils.c 11 Jan 2006 22:40:30 -0000
***************
*** 22,29 ****
#define supports_grant_options(version) ((version) >= 70400)
static bool parseAclItem(const char *item, const char *type, const char *name,
! int remoteVersion,
! PQExpBuffer grantee, PQExpBuffer grantor,
PQExpBuffer privs, PQExpBuffer privswgo);
static char *copyAclUserName(PQExpBuffer output, char *input);
static void AddAcl(PQExpBuffer aclbuf, const char *keyword);
--- 22,28 ----
#define supports_grant_options(version) ((version) >= 70400)
static bool parseAclItem(const char *item, const char *type, const char *name,
! int remoteVersion, PQExpBuffer grantee, PQExpBuffer grantor,
PQExpBuffer privs, PQExpBuffer privswgo);
static char *copyAclUserName(PQExpBuffer output, char *input);
static void AddAcl(PQExpBuffer aclbuf, const char *keyword);
***************
*** 326,332 ****
*
* name: the object name, in the form to use in the commands (already quoted)
* type: the object type (as seen in GRANT command: must be one of
! * TABLE, FUNCTION, LANGUAGE, SCHEMA, DATABASE, or TABLESPACE)
* acls: the ACL string fetched from the database
* owner: username of object owner (will be passed through fmtId); can be
* NULL or empty string to indicate "no owner known"
--- 325,331 ----
*
* name: the object name, in the form to use in the commands (already quoted)
* type: the object type (as seen in GRANT command: must be one of
! * TABLE, SEQUENCE, FUNCTION, LANGUAGE, SCHEMA, DATABASE, or TABLESPACE)
* acls: the ACL string fetched from the database
* owner: username of object owner (will be passed through fmtId); can be
* NULL or empty string to indicate "no owner known"
***************
*** 515,522 ****
*/
static bool
parseAclItem(const char *item, const char *type, const char *name,
! int remoteVersion,
! PQExpBuffer grantee, PQExpBuffer grantor,
PQExpBuffer privs, PQExpBuffer privswgo)
{
char *buf;
--- 514,520 ----
*/
static bool
parseAclItem(const char *item, const char *type, const char *name,
! int remoteVersion, PQExpBuffer grantee, PQExpBuffer grantor,
PQExpBuffer privs, PQExpBuffer privswgo)
{
char *buf;
***************
*** 547,552 ****
--- 545,551 ----
/* privilege codes */
#define CONVERT_PRIV(code, keywd) \
+ do { \
if ((pos = strchr(eqpos + 1, code))) \
{ \
if (*(pos + 1) == '*') \
***************
*** 561,576 ****
} \
} \
else \
! all_with_go = all_without_go = false
resetPQExpBuffer(privs);
resetPQExpBuffer(privswgo);
! if (strcmp(type, "TABLE") == 0)
{
CONVERT_PRIV('a', "INSERT");
CONVERT_PRIV('r', "SELECT");
CONVERT_PRIV('R', "RULE");
if (remoteVersion >= 70200)
{
--- 560,578 ----
} \
} \
else \
! all_with_go = all_without_go = false; \
! } while (0)
resetPQExpBuffer(privs);
resetPQExpBuffer(privswgo);
! if (strcmp(type, "TABLE") == 0 || strcmp(type, "SEQUENCE") == 0)
{
CONVERT_PRIV('a', "INSERT");
CONVERT_PRIV('r', "SELECT");
CONVERT_PRIV('R', "RULE");
+ if (strcmp(type, "SEQUENCE") == 0)
+ CONVERT_PRIV('U', "USAGE");
if (remoteVersion >= 70200)
{
Index: src/bin/pg_dump/pg_backup_archiver.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/pg_dump/pg_backup_archiver.c,v
retrieving revision 1.118
diff -c -c -r1.118 pg_backup_archiver.c
*** src/bin/pg_dump/pg_backup_archiver.c 22 Nov 2005 18:17:28 -0000 1.118
--- src/bin/pg_dump/pg_backup_archiver.c 11 Jan 2006 22:40:31 -0000
***************
*** 1889,1895 ****
if (strcmp(ropt->schemaNames, te->namespace) != 0)
return 0;
}
! if ((strcmp(te->desc, "TABLE") == 0) || (strcmp(te->desc, "TABLE DATA") == 0))
{
if (!ropt->selTable)
return 0;
--- 1889,1896 ----
if (strcmp(ropt->schemaNames, te->namespace) != 0)
return 0;
}
! if (strcmp(te->desc, "TABLE") == 0 ||
! strcmp(te->desc, "TABLE DATA") == 0)
{
if (!ropt->selTable)
return 0;
***************
*** 2276,2283 ****
const char *type = te->desc;
/* Use ALTER TABLE for views and sequences */
! if (strcmp(type, "VIEW") == 0 ||
! strcmp(type, "SEQUENCE") == 0)
type = "TABLE";
/* objects named by a schema and name */
--- 2277,2283 ----
const char *type = te->desc;
/* Use ALTER TABLE for views and sequences */
! if (strcmp(type, "VIEW") == 0 || strcmp(type, "SEQUENCE") == 0)
type = "TABLE";
/* objects named by a schema and name */
Index: src/bin/pg_dump/pg_dump.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/pg_dump/pg_dump.c,v
retrieving revision 1.426
diff -c -c -r1.426 pg_dump.c
*** src/bin/pg_dump/pg_dump.c 9 Jan 2006 21:16:17 -0000 1.426
--- src/bin/pg_dump/pg_dump.c 11 Jan 2006 22:40:36 -0000
***************
*** 6788,6794 ****
/* Handle the ACL here */
namecopy = strdup(fmtId(tbinfo->dobj.name));
! dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId, "TABLE",
namecopy, tbinfo->dobj.name,
tbinfo->dobj.namespace->dobj.name, tbinfo->rolname,
tbinfo->relacl);
--- 6788,6804 ----
/* Handle the ACL here */
namecopy = strdup(fmtId(tbinfo->dobj.name));
! dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId,
! /*
! * Use GRANT SEQUENCE only when dumping a >= 8.2 database
! * because it supports the USAGE permission.
! * >= 8.2 databases still supports GRANT TABLE for loading
! * sequences from previous versions. It issues warnings
! * for invalid permissions.
! */
! (tbinfo->relkind == RELKIND_SEQUENCE &&
! fout->remoteVersion >= 80200)
! ? "SEQUENCE" : "TABLE",
namecopy, tbinfo->dobj.name,
tbinfo->dobj.namespace->dobj.name, tbinfo->rolname,
tbinfo->relacl);
Index: src/include/nodes/parsenodes.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/nodes/parsenodes.h,v
retrieving revision 1.298
diff -c -c -r1.298 parsenodes.h
*** src/include/nodes/parsenodes.h 7 Dec 2005 15:20:55 -0000 1.298
--- src/include/nodes/parsenodes.h 11 Jan 2006 22:40:38 -0000
***************
*** 884,890 ****
*/
typedef enum GrantObjectType
{
! ACL_OBJECT_RELATION, /* table, view, sequence */
ACL_OBJECT_DATABASE, /* database */
ACL_OBJECT_FUNCTION, /* function */
ACL_OBJECT_LANGUAGE, /* procedural language */
--- 884,891 ----
*/
typedef enum GrantObjectType
{
! ACL_OBJECT_RELATION, /* table, view */
! ACL_OBJECT_SEQUENCE, /* sequence */
ACL_OBJECT_DATABASE, /* database */
ACL_OBJECT_FUNCTION, /* function */
ACL_OBJECT_LANGUAGE, /* procedural language */
Index: src/include/utils/acl.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/utils/acl.h,v
retrieving revision 1.91
diff -c -c -r1.91 acl.h
*** src/include/utils/acl.h 1 Dec 2005 02:03:01 -0000 1.91
--- src/include/utils/acl.h 11 Jan 2006 22:40:38 -0000
***************
*** 143,148 ****
--- 143,149 ----
* Bitmasks defining "all rights" for each supported object type
*/
#define ACL_ALL_RIGHTS_RELATION
(ACL_INSERT|ACL_SELECT|ACL_UPDATE|ACL_DELETE|ACL_RULE|ACL_REFERENCES|ACL_TRIGGER)
+ #define ACL_ALL_RIGHTS_SEQUENCE (ACL_USAGE|ACL_SELECT|ACL_UPDATE)
#define ACL_ALL_RIGHTS_DATABASE (ACL_CREATE|ACL_CREATE_TEMP)
#define ACL_ALL_RIGHTS_FUNCTION (ACL_EXECUTE)
#define ACL_ALL_RIGHTS_LANGUAGE (ACL_USAGE)
***************
*** 169,174 ****
--- 170,176 ----
typedef enum AclObjectKind
{
ACL_KIND_CLASS, /* pg_class */
+ ACL_KIND_SEQUENCE, /* pg_sequence */
ACL_KIND_DATABASE, /* pg_database */
ACL_KIND_PROC, /* pg_proc */
ACL_KIND_OPER, /* pg_operator */
Index: src/test/regress/expected/privileges.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/privileges.out,v
retrieving revision 1.32
diff -c -c -r1.32 privileges.out
*** src/test/regress/expected/privileges.out 15 Aug 2005 02:40:30 -0000 1.32
--- src/test/regress/expected/privileges.out 11 Jan 2006 22:40:39 -0000
***************
*** 90,96 ****
COPY atest2 FROM stdin; -- fail
ERROR: permission denied for relation atest2
GRANT ALL ON atest1 TO PUBLIC; -- fail
! WARNING: no privileges were granted
-- checks in subquery, both ok
SELECT * FROM atest1 WHERE ( b IN ( SELECT col1 FROM atest2 ) );
a | b
--- 90,96 ----
COPY atest2 FROM stdin; -- fail
ERROR: permission denied for relation atest2
GRANT ALL ON atest1 TO PUBLIC; -- fail
! WARNING: no privileges were granted for "atest1"
-- checks in subquery, both ok
SELECT * FROM atest1 WHERE ( b IN ( SELECT col1 FROM atest2 ) );
a | b
***************
*** 227,233 ****
HINT: Only superusers may use untrusted languages.
SET SESSION AUTHORIZATION regressuser1;
GRANT USAGE ON LANGUAGE sql TO regressuser2; -- fail
! WARNING: no privileges were granted
CREATE FUNCTION testfunc1(int) RETURNS int AS 'select 2 * $1;' LANGUAGE sql;
CREATE FUNCTION testfunc2(int) RETURNS int AS 'select 3 * $1;' LANGUAGE sql;
REVOKE ALL ON FUNCTION testfunc1(int), testfunc2(int) FROM PUBLIC;
--- 227,233 ----
HINT: Only superusers may use untrusted languages.
SET SESSION AUTHORIZATION regressuser1;
GRANT USAGE ON LANGUAGE sql TO regressuser2; -- fail
! WARNING: no privileges were granted for "sql"
CREATE FUNCTION testfunc1(int) RETURNS int AS 'select 2 * $1;' LANGUAGE sql;
CREATE FUNCTION testfunc2(int) RETURNS int AS 'select 3 * $1;' LANGUAGE sql;
REVOKE ALL ON FUNCTION testfunc1(int), testfunc2(int) FROM PUBLIC;
***************
*** 551,557 ****
SET SESSION AUTHORIZATION regressuser2;
GRANT SELECT ON atest4 TO regressuser3;
GRANT UPDATE ON atest4 TO regressuser3; -- fail
! WARNING: no privileges were granted
SET SESSION AUTHORIZATION regressuser1;
REVOKE SELECT ON atest4 FROM regressuser3; -- does nothing
SELECT has_table_privilege('regressuser3', 'atest4', 'SELECT'); -- true
--- 551,557 ----
SET SESSION AUTHORIZATION regressuser2;
GRANT SELECT ON atest4 TO regressuser3;
GRANT UPDATE ON atest4 TO regressuser3; -- fail
! WARNING: no privileges were granted for "atest4"
SET SESSION AUTHORIZATION regressuser1;
REVOKE SELECT ON atest4 FROM regressuser3; -- does nothing
SELECT has_table_privilege('regressuser3', 'atest4', 'SELECT'); -- true
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@candle.pha.pa.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@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073 Index: doc/src/sgml/ref/grant.sgml =================================================================== RCS file: /cvsroot/pgsql/doc/src/sgml/ref/grant.sgml,v retrieving revision 1.50 diff -c -c -r1.50 grant.sgml *** doc/src/sgml/ref/grant.sgml 20 Oct 2005 19:18:01 -0000 1.50 --- doc/src/sgml/ref/grant.sgml 21 Jan 2006 01:16:10 -0000 *************** *** 25,30 **** --- 25,35 ---- ON [ TABLE ] <replaceable class="PARAMETER">tablename</replaceable> [, ...] TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ] + GRANT { { USAGE | SELECT | UPDATE } + [,...] | ALL [ PRIVILEGES ] } + ON SEQUENCE <replaceable class="PARAMETER">sequencename</replaceable> [, ...] + TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ] + GRANT { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] } ON DATABASE <replaceable>dbname</replaceable> [, ...] TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ] *************** *** 260,265 **** --- 265,274 ---- also met). Essentially this allows the grantee to <quote>look up</> objects within the schema. </para> + <para> + For sequences, this privilege allows the use of the + <function>currval</function> and <function>nextval</function> functions. + </para> </listitem> </varlistentry> *************** *** 511,517 **** <para> The <literal>RULE</literal> privilege, and privileges on ! databases, tablespaces, schemas, languages, and sequences are <productname>PostgreSQL</productname> extensions. </para> </refsect1> --- 520,526 ---- <para> The <literal>RULE</literal> privilege, and privileges on ! databases, tablespaces, schemas, and languages are <productname>PostgreSQL</productname> extensions. </para> </refsect1> Index: doc/src/sgml/ref/revoke.sgml =================================================================== RCS file: /cvsroot/pgsql/doc/src/sgml/ref/revoke.sgml,v retrieving revision 1.35 diff -c -c -r1.35 revoke.sgml *** doc/src/sgml/ref/revoke.sgml 20 Oct 2005 19:18:01 -0000 1.35 --- doc/src/sgml/ref/revoke.sgml 21 Jan 2006 01:16:10 -0000 *************** *** 28,33 **** --- 28,40 ---- [ CASCADE | RESTRICT ] REVOKE [ GRANT OPTION FOR ] + { { USAGE | SELECT | UPDATE } + [,...] | ALL [ PRIVILEGES ] } + ON SEQUENCE <replaceable class="PARAMETER">sequencename</replaceable> [, ...] + FROM { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] + [ CASCADE | RESTRICT ] + + REVOKE [ GRANT OPTION FOR ] { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] } ON DATABASE <replaceable>dbname</replaceable> [, ...] FROM { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] Index: src/backend/catalog/aclchk.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/catalog/aclchk.c,v retrieving revision 1.123 diff -c -c -r1.123 aclchk.c *** src/backend/catalog/aclchk.c 1 Dec 2005 02:03:00 -0000 1.123 --- src/backend/catalog/aclchk.c 21 Jan 2006 01:16:11 -0000 *************** *** 164,169 **** --- 164,172 ---- case ACL_KIND_CLASS: whole_mask = ACL_ALL_RIGHTS_RELATION; break; + case ACL_KIND_SEQUENCE: + whole_mask = ACL_ALL_RIGHTS_SEQUENCE; + break; case ACL_KIND_DATABASE: whole_mask = ACL_ALL_RIGHTS_DATABASE; break; *************** *** 212,233 **** if (this_privileges == 0) ereport(WARNING, (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED), ! errmsg("no privileges were granted"))); else if (!all_privs && this_privileges != privileges) ereport(WARNING, (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED), ! errmsg("not all privileges were granted"))); } else { if (this_privileges == 0) ereport(WARNING, (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED), ! errmsg("no privileges could be revoked"))); else if (!all_privs && this_privileges != privileges) ereport(WARNING, (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED), ! errmsg("not all privileges could be revoked"))); } return this_privileges; --- 215,236 ---- if (this_privileges == 0) ereport(WARNING, (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED), ! errmsg("no privileges were granted for \"%s\"", objname))); else if (!all_privs && this_privileges != privileges) ereport(WARNING, (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED), ! errmsg("not all privileges were granted for \"%s\"", objname))); } else { if (this_privileges == 0) ereport(WARNING, (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED), ! errmsg("no privileges could be revoked for \"%s\"", objname))); else if (!all_privs && this_privileges != privileges) ereport(WARNING, (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED), ! errmsg("not all privileges could be revoked for \"%s\"", objname))); } return this_privileges; *************** *** 282,290 **** */ switch (stmt->objtype) { case ACL_OBJECT_RELATION: ! all_privileges = ACL_ALL_RIGHTS_RELATION; ! errormsg = _("invalid privilege type %s for table"); break; case ACL_OBJECT_DATABASE: all_privileges = ACL_ALL_RIGHTS_DATABASE; --- 285,302 ---- */ switch (stmt->objtype) { + /* + * Because this might be a sequence, we test both relation + * and sequence bits, and later do a more limited test + * when we know the object type. + */ case ACL_OBJECT_RELATION: ! all_privileges = ACL_ALL_RIGHTS_RELATION | ACL_ALL_RIGHTS_SEQUENCE; ! errormsg = _("invalid privilege type %s for relation"); ! break; ! case ACL_OBJECT_SEQUENCE: ! all_privileges = ACL_ALL_RIGHTS_SEQUENCE; ! errormsg = _("invalid privilege type %s for sequence"); break; case ACL_OBJECT_DATABASE: all_privileges = ACL_ALL_RIGHTS_DATABASE; *************** *** 327,332 **** --- 339,345 ---- { istmt.all_privs = false; istmt.privileges = ACL_NO_RIGHTS; + foreach(cell, stmt->privileges) { char *privname = strVal(lfirst(cell)); *************** *** 356,361 **** --- 369,375 ---- switch (istmt->objtype) { case ACL_OBJECT_RELATION: + case ACL_OBJECT_SEQUENCE: ExecGrant_Relation(istmt); break; case ACL_OBJECT_DATABASE: *************** *** 395,400 **** --- 409,415 ---- switch (objtype) { case ACL_OBJECT_RELATION: + case ACL_OBJECT_SEQUENCE: foreach(cell, objnames) { Oid relOid; *************** *** 523,537 **** return objects; } static void ExecGrant_Relation(InternalGrant *istmt) { Relation relation; ListCell *cell; - if (istmt->all_privs && istmt->privileges == ACL_NO_RIGHTS) - istmt->privileges = ACL_ALL_RIGHTS_RELATION; - relation = heap_open(RelationRelationId, RowExclusiveLock); foreach(cell, istmt->objects) --- 538,552 ---- return objects; } + /* + * This processes both sequences and non-sequences. + */ static void ExecGrant_Relation(InternalGrant *istmt) { Relation relation; ListCell *cell; relation = heap_open(RelationRelationId, RowExclusiveLock); foreach(cell, istmt->objects) *************** *** 577,582 **** --- 592,660 ---- errmsg("\"%s\" is a composite type", NameStr(pg_class_tuple->relname)))); + /* Used GRANT SEQUENCE on a non-sequence? */ + if (istmt->objtype == ACL_OBJECT_SEQUENCE && + pg_class_tuple->relkind != RELKIND_SEQUENCE) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("\"%s\" is not a sequence", + NameStr(pg_class_tuple->relname)))); + + /* Adjust the default permissions based on whether it is a sequence */ + if (istmt->all_privs && istmt->privileges == ACL_NO_RIGHTS) + { + if (pg_class_tuple->relkind == RELKIND_SEQUENCE) + this_privileges = ACL_ALL_RIGHTS_SEQUENCE; + else + this_privileges = ACL_ALL_RIGHTS_RELATION; + } + else + this_privileges = istmt->privileges; + + /* + * The GRANT TABLE syntax can be used for sequences and + * non-sequences, so we have to look at the relkind to + * determine the supported permissions. The OR of + * table and sequence permissions were already checked. + */ + if (istmt->objtype == ACL_OBJECT_RELATION) + { + if (pg_class_tuple->relkind == RELKIND_SEQUENCE) + { + /* + * For backward compatibility, throw just a warning + * for invalid sequence permissions when using the + * non-sequence GRANT syntax is used. + */ + if (this_privileges & ~((AclMode) ACL_ALL_RIGHTS_SEQUENCE)) + { + /* + * Mention the object name because the user needs to + * know which operations succeeded. This is required + * because WARNING allows the command to continue. + */ + ereport(WARNING, + (errcode(ERRCODE_INVALID_GRANT_OPERATION), + errmsg("sequence \"%s\" only supports USAGE, SELECT, and UPDATE", + NameStr(pg_class_tuple->relname)))); + this_privileges &= (AclMode) ACL_ALL_RIGHTS_SEQUENCE; + } + } + else + { + if (this_privileges & ~((AclMode) ACL_ALL_RIGHTS_RELATION)) + /* + * USAGE is the only permission supported by sequences + * but not by non-sequences. Don't mention the object + * name because we didn't in the combined TABLE | + * SEQUENCE check. + */ + ereport(ERROR, + (errcode(ERRCODE_INVALID_GRANT_OPERATION), + errmsg("invalid privilege type USAGE for table"))); + } + } + /* * Get owner ID and working copy of existing ACL. If there's no ACL, * substitute the proper default. *************** *** 585,596 **** aclDatum = SysCacheGetAttr(RELOID, tuple, Anum_pg_class_relacl, &isNull); if (isNull) ! old_acl = acldefault(ACL_OBJECT_RELATION, ownerId); else old_acl = DatumGetAclPCopy(aclDatum); /* Determine ID to do the grant as, and available grant options */ ! select_best_grantor(GetUserId(), istmt->privileges, old_acl, ownerId, &grantorId, &avail_goptions); --- 663,676 ---- aclDatum = SysCacheGetAttr(RELOID, tuple, Anum_pg_class_relacl, &isNull); if (isNull) ! old_acl = acldefault(pg_class_tuple->relkind == RELKIND_SEQUENCE ? ! ACL_OBJECT_SEQUENCE : ACL_OBJECT_RELATION, ! ownerId); else old_acl = DatumGetAclPCopy(aclDatum); /* Determine ID to do the grant as, and available grant options */ ! select_best_grantor(GetUserId(), this_privileges, old_acl, ownerId, &grantorId, &avail_goptions); *************** *** 600,607 **** */ this_privileges = restrict_and_check_grant(istmt->is_grant, avail_goptions, ! istmt->all_privs, istmt->privileges, ! relOid, grantorId, ACL_KIND_CLASS, NameStr(pg_class_tuple->relname)); /* --- 680,689 ---- */ this_privileges = restrict_and_check_grant(istmt->is_grant, avail_goptions, ! istmt->all_privs, this_privileges, ! relOid, grantorId, ! pg_class_tuple->relkind == RELKIND_SEQUENCE ! ? ACL_KIND_SEQUENCE : ACL_KIND_CLASS, NameStr(pg_class_tuple->relname)); /* *************** *** 1336,1341 **** --- 1418,1425 ---- { /* ACL_KIND_CLASS */ gettext_noop("permission denied for relation %s"), + /* ACL_KIND_SEQUENCE */ + gettext_noop("permission denied for sequence %s"), /* ACL_KIND_DATABASE */ gettext_noop("permission denied for database %s"), /* ACL_KIND_PROC */ *************** *** 1360,1365 **** --- 1444,1451 ---- { /* ACL_KIND_CLASS */ gettext_noop("must be owner of relation %s"), + /* ACL_KIND_SEQUENCE */ + gettext_noop("must be owner of sequence %s"), /* ACL_KIND_DATABASE */ gettext_noop("must be owner of database %s"), /* ACL_KIND_PROC */ *************** *** 1439,1444 **** --- 1525,1531 ---- switch (objkind) { case ACL_KIND_CLASS: + case ACL_KIND_SEQUENCE: return pg_class_aclmask(table_oid, roleid, mask, how); case ACL_KIND_DATABASE: return pg_database_aclmask(table_oid, roleid, mask, how); *************** *** 1500,1508 **** * * As of 7.4 we have some updatable system views; those shouldn't be * protected in this way. Assume the view rules can take care of ! * themselves. */ ! if ((mask & (ACL_INSERT | ACL_UPDATE | ACL_DELETE)) && IsSystemClass(classForm) && classForm->relkind != RELKIND_VIEW && !has_rolcatupdate(roleid) && --- 1587,1595 ---- * * As of 7.4 we have some updatable system views; those shouldn't be * protected in this way. Assume the view rules can take care of ! * themselves. ACL_USAGE is if we ever have system sequences. */ ! if ((mask & (ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_USAGE)) && IsSystemClass(classForm) && classForm->relkind != RELKIND_VIEW && !has_rolcatupdate(roleid) && *************** *** 1511,1517 **** #ifdef ACLDEBUG elog(DEBUG2, "permission denied for system catalog update"); #endif ! mask &= ~(ACL_INSERT | ACL_UPDATE | ACL_DELETE); } /* --- 1598,1604 ---- #ifdef ACLDEBUG elog(DEBUG2, "permission denied for system catalog update"); #endif ! mask &= ~(ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_USAGE); } /* *************** *** 1536,1542 **** if (isNull) { /* No ACL, so build default ACL */ ! acl = acldefault(ACL_OBJECT_RELATION, ownerId); aclDatum = (Datum) 0; } else --- 1623,1631 ---- if (isNull) { /* No ACL, so build default ACL */ ! acl = acldefault(classForm->relkind == RELKIND_SEQUENCE ? ! ACL_OBJECT_SEQUENCE : ACL_OBJECT_RELATION, ! ownerId); aclDatum = (Datum) 0; } else Index: src/backend/catalog/pg_shdepend.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/catalog/pg_shdepend.c,v retrieving revision 1.6 diff -c -c -r1.6 pg_shdepend.c *** src/backend/catalog/pg_shdepend.c 1 Dec 2005 02:03:00 -0000 1.6 --- src/backend/catalog/pg_shdepend.c 21 Jan 2006 01:16:12 -0000 *************** *** 1133,1140 **** switch (sdepForm->classid) { case RelationRelationId: ! istmt.objtype = ACL_OBJECT_RELATION; break; case DatabaseRelationId: istmt.objtype = ACL_OBJECT_DATABASE; break; --- 1133,1157 ---- switch (sdepForm->classid) { case RelationRelationId: ! { ! /* is it a sequence or non-sequence? */ ! Form_pg_class pg_class_tuple; ! HeapTuple tuple; ! ! tuple = SearchSysCache(RELOID, ! ObjectIdGetDatum(sdepForm->objid), ! 0, 0, 0); ! if (!HeapTupleIsValid(tuple)) ! elog(ERROR, "cache lookup failed for relation %u", ! sdepForm->objid); ! pg_class_tuple = (Form_pg_class) GETSTRUCT(tuple); ! if (pg_class_tuple->relkind == RELKIND_SEQUENCE) ! istmt.objtype = ACL_OBJECT_SEQUENCE; ! else ! istmt.objtype = ACL_OBJECT_RELATION; ! ReleaseSysCache(tuple); break; + } case DatabaseRelationId: istmt.objtype = ACL_OBJECT_DATABASE; break; Index: src/backend/commands/sequence.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/commands/sequence.c,v retrieving revision 1.126 diff -c -c -r1.126 sequence.c *** src/backend/commands/sequence.c 22 Nov 2005 18:17:09 -0000 1.126 --- src/backend/commands/sequence.c 21 Jan 2006 01:16:12 -0000 *************** *** 422,428 **** /* open and AccessShareLock sequence */ init_sequence(relid, &elm, &seqrel); ! if (pg_class_aclcheck(elm->relid, GetUserId(), ACL_UPDATE) != ACLCHECK_OK) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied for sequence %s", --- 422,429 ---- /* open and AccessShareLock sequence */ init_sequence(relid, &elm, &seqrel); ! if (pg_class_aclcheck(elm->relid, GetUserId(), ACL_USAGE) != ACLCHECK_OK && ! pg_class_aclcheck(elm->relid, GetUserId(), ACL_UPDATE) != ACLCHECK_OK) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied for sequence %s", *************** *** 613,619 **** /* open and AccessShareLock sequence */ init_sequence(relid, &elm, &seqrel); ! if (pg_class_aclcheck(elm->relid, GetUserId(), ACL_SELECT) != ACLCHECK_OK) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied for sequence %s", --- 614,621 ---- /* open and AccessShareLock sequence */ init_sequence(relid, &elm, &seqrel); ! if (pg_class_aclcheck(elm->relid, GetUserId(), ACL_SELECT) != ACLCHECK_OK && ! pg_class_aclcheck(elm->relid, GetUserId(), ACL_USAGE) != ACLCHECK_OK) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied for sequence %s", *************** *** 657,663 **** /* nextval() must have already been called for this sequence */ Assert(last_used_seq->increment != 0); ! if (pg_class_aclcheck(last_used_seq->relid, GetUserId(), ACL_SELECT) != ACLCHECK_OK) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied for sequence %s", --- 659,666 ---- /* nextval() must have already been called for this sequence */ Assert(last_used_seq->increment != 0); ! if (pg_class_aclcheck(last_used_seq->relid, GetUserId(), ACL_SELECT) != ACLCHECK_OK && ! pg_class_aclcheck(last_used_seq->relid, GetUserId(), ACL_USAGE) != ACLCHECK_OK) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied for sequence %s", Index: src/backend/parser/gram.y =================================================================== RCS file: /cvsroot/pgsql/src/backend/parser/gram.y,v retrieving revision 2.521 diff -c -c -r2.521 gram.y *** src/backend/parser/gram.y 29 Dec 2005 04:53:18 -0000 2.521 --- src/backend/parser/gram.y 21 Jan 2006 01:16:16 -0000 *************** *** 3322,3327 **** --- 3322,3334 ---- n->objs = $2; $$ = n; } + | SEQUENCE qualified_name_list + { + PrivTarget *n = makeNode(PrivTarget); + n->objtype = ACL_OBJECT_SEQUENCE; + n->objs = $2; + $$ = n; + } | FUNCTION function_with_argtypes_list { PrivTarget *n = makeNode(PrivTarget); Index: src/backend/utils/adt/acl.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/utils/adt/acl.c,v retrieving revision 1.129 diff -c -c -r1.129 acl.c *** src/backend/utils/adt/acl.c 18 Nov 2005 02:38:23 -0000 1.129 --- src/backend/utils/adt/acl.c 21 Jan 2006 01:16:17 -0000 *************** *** 545,550 **** --- 545,554 ---- world_default = ACL_NO_RIGHTS; owner_default = ACL_ALL_RIGHTS_RELATION; break; + case ACL_OBJECT_SEQUENCE: + world_default = ACL_NO_RIGHTS; + owner_default = ACL_ALL_RIGHTS_SEQUENCE; + break; case ACL_OBJECT_DATABASE: world_default = ACL_CREATE_TEMP; /* not NO_RIGHTS! */ owner_default = ACL_ALL_RIGHTS_DATABASE; Index: src/bin/pg_dump/dumputils.c =================================================================== RCS file: /cvsroot/pgsql/src/bin/pg_dump/dumputils.c,v retrieving revision 1.24 diff -c -c -r1.24 dumputils.c *** src/bin/pg_dump/dumputils.c 11 Jan 2006 21:24:30 -0000 1.24 --- src/bin/pg_dump/dumputils.c 21 Jan 2006 01:16:18 -0000 *************** *** 22,29 **** #define supports_grant_options(version) ((version) >= 70400) static bool parseAclItem(const char *item, const char *type, const char *name, ! int remoteVersion, ! PQExpBuffer grantee, PQExpBuffer grantor, PQExpBuffer privs, PQExpBuffer privswgo); static char *copyAclUserName(PQExpBuffer output, char *input); static void AddAcl(PQExpBuffer aclbuf, const char *keyword); --- 22,28 ---- #define supports_grant_options(version) ((version) >= 70400) static bool parseAclItem(const char *item, const char *type, const char *name, ! int remoteVersion, PQExpBuffer grantee, PQExpBuffer grantor, PQExpBuffer privs, PQExpBuffer privswgo); static char *copyAclUserName(PQExpBuffer output, char *input); static void AddAcl(PQExpBuffer aclbuf, const char *keyword); *************** *** 326,332 **** * * name: the object name, in the form to use in the commands (already quoted) * type: the object type (as seen in GRANT command: must be one of ! * TABLE, FUNCTION, LANGUAGE, SCHEMA, DATABASE, or TABLESPACE) * acls: the ACL string fetched from the database * owner: username of object owner (will be passed through fmtId); can be * NULL or empty string to indicate "no owner known" --- 325,331 ---- * * name: the object name, in the form to use in the commands (already quoted) * type: the object type (as seen in GRANT command: must be one of ! * TABLE, SEQUENCE, FUNCTION, LANGUAGE, SCHEMA, DATABASE, or TABLESPACE) * acls: the ACL string fetched from the database * owner: username of object owner (will be passed through fmtId); can be * NULL or empty string to indicate "no owner known" *************** *** 515,522 **** */ static bool parseAclItem(const char *item, const char *type, const char *name, ! int remoteVersion, ! PQExpBuffer grantee, PQExpBuffer grantor, PQExpBuffer privs, PQExpBuffer privswgo) { char *buf; --- 514,520 ---- */ static bool parseAclItem(const char *item, const char *type, const char *name, ! int remoteVersion, PQExpBuffer grantee, PQExpBuffer grantor, PQExpBuffer privs, PQExpBuffer privswgo) { char *buf; *************** *** 547,552 **** --- 545,551 ---- /* privilege codes */ #define CONVERT_PRIV(code, keywd) \ + do { \ if ((pos = strchr(eqpos + 1, code))) \ { \ if (*(pos + 1) == '*') \ *************** *** 561,589 **** } \ } \ else \ ! all_with_go = all_without_go = false resetPQExpBuffer(privs); resetPQExpBuffer(privswgo); ! if (strcmp(type, "TABLE") == 0) { - CONVERT_PRIV('a', "INSERT"); CONVERT_PRIV('r', "SELECT"); ! CONVERT_PRIV('R', "RULE"); ! ! if (remoteVersion >= 70200) { ! CONVERT_PRIV('w', "UPDATE"); ! CONVERT_PRIV('d', "DELETE"); ! CONVERT_PRIV('x', "REFERENCES"); ! CONVERT_PRIV('t', "TRIGGER"); } else - { /* 7.0 and 7.1 have a simpler worldview */ CONVERT_PRIV('w', "UPDATE,DELETE"); - } } else if (strcmp(type, "FUNCTION") == 0) CONVERT_PRIV('X', "EXECUTE"); --- 560,597 ---- } \ } \ else \ ! all_with_go = all_without_go = false; \ ! } while (0) resetPQExpBuffer(privs); resetPQExpBuffer(privswgo); ! if (strcmp(type, "TABLE") == 0 || strcmp(type, "SEQUENCE") == 0) { CONVERT_PRIV('r', "SELECT"); ! ! if (strcmp(type, "SEQUENCE") == 0) ! /* sequence only */ ! CONVERT_PRIV('U', "USAGE"); ! else { ! /* table only */ ! CONVERT_PRIV('a', "INSERT"); ! CONVERT_PRIV('R', "RULE"); ! if (remoteVersion >= 70200) ! { ! CONVERT_PRIV('d', "DELETE"); ! CONVERT_PRIV('x', "REFERENCES"); ! CONVERT_PRIV('t', "TRIGGER"); ! } } + + /* UPDATE */ + if (remoteVersion >= 70200 || strcmp(type, "SEQUENCE") == 0) + CONVERT_PRIV('w', "UPDATE"); else /* 7.0 and 7.1 have a simpler worldview */ CONVERT_PRIV('w', "UPDATE,DELETE"); } else if (strcmp(type, "FUNCTION") == 0) CONVERT_PRIV('X', "EXECUTE"); Index: src/bin/pg_dump/pg_backup_archiver.c =================================================================== RCS file: /cvsroot/pgsql/src/bin/pg_dump/pg_backup_archiver.c,v retrieving revision 1.118 diff -c -c -r1.118 pg_backup_archiver.c *** src/bin/pg_dump/pg_backup_archiver.c 22 Nov 2005 18:17:28 -0000 1.118 --- src/bin/pg_dump/pg_backup_archiver.c 21 Jan 2006 01:16:19 -0000 *************** *** 1889,1895 **** if (strcmp(ropt->schemaNames, te->namespace) != 0) return 0; } ! if ((strcmp(te->desc, "TABLE") == 0) || (strcmp(te->desc, "TABLE DATA") == 0)) { if (!ropt->selTable) return 0; --- 1889,1896 ---- if (strcmp(ropt->schemaNames, te->namespace) != 0) return 0; } ! if (strcmp(te->desc, "TABLE") == 0 || ! strcmp(te->desc, "TABLE DATA") == 0) { if (!ropt->selTable) return 0; *************** *** 2276,2283 **** const char *type = te->desc; /* Use ALTER TABLE for views and sequences */ ! if (strcmp(type, "VIEW") == 0 || ! strcmp(type, "SEQUENCE") == 0) type = "TABLE"; /* objects named by a schema and name */ --- 2277,2283 ---- const char *type = te->desc; /* Use ALTER TABLE for views and sequences */ ! if (strcmp(type, "VIEW") == 0 || strcmp(type, "SEQUENCE") == 0) type = "TABLE"; /* objects named by a schema and name */ Index: src/bin/pg_dump/pg_dump.c =================================================================== RCS file: /cvsroot/pgsql/src/bin/pg_dump/pg_dump.c,v retrieving revision 1.426 diff -c -c -r1.426 pg_dump.c *** src/bin/pg_dump/pg_dump.c 9 Jan 2006 21:16:17 -0000 1.426 --- src/bin/pg_dump/pg_dump.c 21 Jan 2006 01:16:21 -0000 *************** *** 6788,6794 **** /* Handle the ACL here */ namecopy = strdup(fmtId(tbinfo->dobj.name)); ! dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId, "TABLE", namecopy, tbinfo->dobj.name, tbinfo->dobj.namespace->dobj.name, tbinfo->rolname, tbinfo->relacl); --- 6788,6795 ---- /* Handle the ACL here */ namecopy = strdup(fmtId(tbinfo->dobj.name)); ! dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId, ! (tbinfo->relkind == RELKIND_SEQUENCE) ? "SEQUENCE" : "TABLE", namecopy, tbinfo->dobj.name, tbinfo->dobj.namespace->dobj.name, tbinfo->rolname, tbinfo->relacl); Index: src/include/nodes/parsenodes.h =================================================================== RCS file: /cvsroot/pgsql/src/include/nodes/parsenodes.h,v retrieving revision 1.298 diff -c -c -r1.298 parsenodes.h *** src/include/nodes/parsenodes.h 7 Dec 2005 15:20:55 -0000 1.298 --- src/include/nodes/parsenodes.h 21 Jan 2006 01:16:22 -0000 *************** *** 884,890 **** */ typedef enum GrantObjectType { ! ACL_OBJECT_RELATION, /* table, view, sequence */ ACL_OBJECT_DATABASE, /* database */ ACL_OBJECT_FUNCTION, /* function */ ACL_OBJECT_LANGUAGE, /* procedural language */ --- 884,891 ---- */ typedef enum GrantObjectType { ! ACL_OBJECT_RELATION, /* table, view */ ! ACL_OBJECT_SEQUENCE, /* sequence */ ACL_OBJECT_DATABASE, /* database */ ACL_OBJECT_FUNCTION, /* function */ ACL_OBJECT_LANGUAGE, /* procedural language */ Index: src/include/utils/acl.h =================================================================== RCS file: /cvsroot/pgsql/src/include/utils/acl.h,v retrieving revision 1.91 diff -c -c -r1.91 acl.h *** src/include/utils/acl.h 1 Dec 2005 02:03:01 -0000 1.91 --- src/include/utils/acl.h 21 Jan 2006 01:16:23 -0000 *************** *** 143,148 **** --- 143,149 ---- * Bitmasks defining "all rights" for each supported object type */ #define ACL_ALL_RIGHTS_RELATION (ACL_INSERT|ACL_SELECT|ACL_UPDATE|ACL_DELETE|ACL_RULE|ACL_REFERENCES|ACL_TRIGGER) + #define ACL_ALL_RIGHTS_SEQUENCE (ACL_USAGE|ACL_SELECT|ACL_UPDATE) #define ACL_ALL_RIGHTS_DATABASE (ACL_CREATE|ACL_CREATE_TEMP) #define ACL_ALL_RIGHTS_FUNCTION (ACL_EXECUTE) #define ACL_ALL_RIGHTS_LANGUAGE (ACL_USAGE) *************** *** 169,174 **** --- 170,176 ---- typedef enum AclObjectKind { ACL_KIND_CLASS, /* pg_class */ + ACL_KIND_SEQUENCE, /* pg_sequence */ ACL_KIND_DATABASE, /* pg_database */ ACL_KIND_PROC, /* pg_proc */ ACL_KIND_OPER, /* pg_operator */ Index: src/test/regress/expected/privileges.out =================================================================== RCS file: /cvsroot/pgsql/src/test/regress/expected/privileges.out,v retrieving revision 1.32 diff -c -c -r1.32 privileges.out *** src/test/regress/expected/privileges.out 15 Aug 2005 02:40:30 -0000 1.32 --- src/test/regress/expected/privileges.out 21 Jan 2006 01:16:23 -0000 *************** *** 90,96 **** COPY atest2 FROM stdin; -- fail ERROR: permission denied for relation atest2 GRANT ALL ON atest1 TO PUBLIC; -- fail ! WARNING: no privileges were granted -- checks in subquery, both ok SELECT * FROM atest1 WHERE ( b IN ( SELECT col1 FROM atest2 ) ); a | b --- 90,96 ---- COPY atest2 FROM stdin; -- fail ERROR: permission denied for relation atest2 GRANT ALL ON atest1 TO PUBLIC; -- fail ! WARNING: no privileges were granted for "atest1" -- checks in subquery, both ok SELECT * FROM atest1 WHERE ( b IN ( SELECT col1 FROM atest2 ) ); a | b *************** *** 227,233 **** HINT: Only superusers may use untrusted languages. SET SESSION AUTHORIZATION regressuser1; GRANT USAGE ON LANGUAGE sql TO regressuser2; -- fail ! WARNING: no privileges were granted CREATE FUNCTION testfunc1(int) RETURNS int AS 'select 2 * $1;' LANGUAGE sql; CREATE FUNCTION testfunc2(int) RETURNS int AS 'select 3 * $1;' LANGUAGE sql; REVOKE ALL ON FUNCTION testfunc1(int), testfunc2(int) FROM PUBLIC; --- 227,233 ---- HINT: Only superusers may use untrusted languages. SET SESSION AUTHORIZATION regressuser1; GRANT USAGE ON LANGUAGE sql TO regressuser2; -- fail ! WARNING: no privileges were granted for "sql" CREATE FUNCTION testfunc1(int) RETURNS int AS 'select 2 * $1;' LANGUAGE sql; CREATE FUNCTION testfunc2(int) RETURNS int AS 'select 3 * $1;' LANGUAGE sql; REVOKE ALL ON FUNCTION testfunc1(int), testfunc2(int) FROM PUBLIC; *************** *** 551,557 **** SET SESSION AUTHORIZATION regressuser2; GRANT SELECT ON atest4 TO regressuser3; GRANT UPDATE ON atest4 TO regressuser3; -- fail ! WARNING: no privileges were granted SET SESSION AUTHORIZATION regressuser1; REVOKE SELECT ON atest4 FROM regressuser3; -- does nothing SELECT has_table_privilege('regressuser3', 'atest4', 'SELECT'); -- true --- 551,557 ---- SET SESSION AUTHORIZATION regressuser2; GRANT SELECT ON atest4 TO regressuser3; GRANT UPDATE ON atest4 TO regressuser3; -- fail ! WARNING: no privileges were granted for "atest4" SET SESSION AUTHORIZATION regressuser1; REVOKE SELECT ON atest4 FROM regressuser3; -- does nothing SELECT has_table_privilege('regressuser3', 'atest4', 'SELECT'); -- true