Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
Updated patch applied. I decided Tom was right to just ignore invalid
sequence permission from pre-8.2 databases, rather than try to use GRANT
TABLE; there was no reason to do it and avoiding it made the code
cleaner and more robust.
The changes were:
Add GRANT ON SEQUENCE syntax to support sequence-only permissions.
Continue to support GRANT ON [TABLE] for sequences for backward
compatibility; issue warning for invalid sequence permissions.
[ Backward compatibility warning message.]
Add USAGE permission for sequences that allows only currval() and
nextval(), not setval().
Mention object name in grant/revoke warnings because of possible
multi-object operations.
---
Bruce Momjian wrote:
Tom Lane wrote:
Bruce Momjian pgman@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 - 1.50
--- doc/src/sgml/ref/grant.sgml 21 Jan 2006 01:16:10 -
***
*** 25,30
--- 25,35
ON [ TABLE ] replaceable class=PARAMETERtablename/replaceable [,
...]
TO { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...] [
WITH GRANT OPTION ]
+ GRANT { { USAGE | SELECT | UPDATE }
+ [,...] | ALL [ PRIVILEGES ] }
+ ON SEQUENCE replaceable class=PARAMETERsequencename/replaceable [,
...]
+ TO { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...] [
WITH GRANT OPTION ]
+
GRANT { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] }
ON DATABASE replaceabledbname/replaceable [, ...]
TO { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...] [
WITH GRANT OPTION ]
***
*** 260,265
--- 265,274
also met). Essentially this allows the grantee to quotelook up/
objects within the schema.
/para
+ para
+For sequences, this privilege allows the use of the
+functioncurrval/function and functionnextval/function
functions.
+ /para
/listitem
/varlistentry
***
*** 511,517
para
The literalRULE/literal privilege, and privileges on
! databases, tablespaces, schemas, languages, and sequences are
productnamePostgreSQL/productname extensions.
/para
/refsect1
---
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 - 1.50
--- doc/src/sgml/ref/grant.sgml 11 Jan 2006 22:40:11 -
***
*** 25,30
--- 25,35
ON [ TABLE ] replaceable class=PARAMETERtablename/replaceable [,
...]
TO { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...] [
WITH GRANT OPTION ]
+ GRANT { { USAGE | SELECT | UPDATE }
+ [,...] | ALL [ PRIVILEGES ] }
+ ON SEQUENCE replaceable class=PARAMETERsequencename/replaceable [,
...]
+ TO { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...] [
WITH GRANT OPTION ]
+
GRANT { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] }
ON DATABASE replaceabledbname/replaceable [, ...]
TO { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...] [
WITH GRANT OPTION ]
***
*** 260,265
--- 265,274
also met). Essentially this allows the grantee to quotelook up/
objects within the schema.
/para
+ para
+For sequences, this privilege allows the use of the
+functioncurrval/function and functionnextval/function
functions.
+ /para
/listitem
/varlistentry
***
*** 511,517
para
The literalRULE/literal privilege, and privileges on
! databases, tablespaces, schemas, languages, and sequences are
productnamePostgreSQL/productname extensions.
/para
/refsect1
--- 520,526
para
The literalRULE/literal privilege, and privileges on
! databases, tablespaces, schemas, and languages are
productnamePostgreSQL/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.sgml20 Oct 2005 19:18:01 - 1.35
--- doc/src/sgml/ref/revoke.sgml11 Jan 2006 22:40:11 -
***
*** 28,33
--- 28,40
[ CASCADE | RESTRICT ]
REVOKE [ GRANT OPTION FOR ]
+ { { USAGE | SELECT | UPDATE }
+ [,...] | ALL [ PRIVILEGES ] }
+ ON SEQUENCE replaceable
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ***
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
Tom, all, SELECT: currval UPDATE: nextval, setval USAGE: nextval, currval +1. --Josh ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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
Index: pgsql/doc/src/sgml/ref/grant.sgml
===
*** pgsql.orig/doc/src/sgml/ref/grant.sgml
--- pgsql/doc/src/sgml/ref/grant.sgml
*** GRANT { { SELECT | INSERT | UPDATE | DEL
*** 25,33
ON [ TABLE ] replaceable class=PARAMETERtablename/replaceable [, ...]
TO { replaceable class=PARAMETERusername/replaceable | GROUP replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...] [ WITH GRANT OPTION ]
! GRANT { { SELECT | USAGE | UPDATE }
[,...] | ALL [ PRIVILEGES ] }
! ON SEQUENCE replaceable class=PARAMETERtablename/replaceable [, ...]
TO { replaceable class=PARAMETERusername/replaceable | GROUP replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...] [ WITH GRANT OPTION ]
GRANT { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] }
--- 25,33
ON [ TABLE ] replaceable class=PARAMETERtablename/replaceable [, ...]
TO { replaceable class=PARAMETERusername/replaceable | GROUP replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...] [ WITH GRANT OPTION ]
! GRANT { { USAGE | SELECT | UPDATE }
[,...] | ALL [ PRIVILEGES ] }
! ON SEQUENCE replaceable class=PARAMETERsequencename/replaceable [, ...]
TO { replaceable class=PARAMETERusername/replaceable | GROUP replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...] [ WITH GRANT OPTION ]
GRANT { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] }
Index: pgsql/doc/src/sgml/ref/revoke.sgml
===
*** pgsql.orig/doc/src/sgml/ref/revoke.sgml
--- pgsql/doc/src/sgml/ref/revoke.sgml
*** REVOKE [ GRANT OPTION FOR ]
*** 28,36
[ CASCADE | RESTRICT ]
REVOKE [ GRANT OPTION FOR ]
! { { SELECT | UPDATE }
[,...] | ALL [ PRIVILEGES ] }
! ON SEQUENCE replaceable class=PARAMETERtablename/replaceable [, ...]
FROM { replaceable class=PARAMETERusername/replaceable | GROUP replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...]
[ CASCADE | RESTRICT ]
--- 28,36
[ CASCADE | RESTRICT ]
REVOKE [ GRANT OPTION FOR ]
! { { USAGE | SELECT | UPDATE }
[,...] | ALL [ PRIVILEGES ] }
! ON SEQUENCE replaceable class=PARAMETERsequencename/replaceable [, ...]
FROM { replaceable class=PARAMETERusername/replaceable | GROUP replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...]
[ CASCADE | RESTRICT ]
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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] ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 - 1.50
--- doc/src/sgml/ref/grant.sgml 6 Jan 2006 15:23:16 -
***
*** 25,30
--- 25,35
ON [ TABLE ] replaceable class=PARAMETERtablename/replaceable [,
...]
TO { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...] [
WITH GRANT OPTION ]
+ GRANT { { SELECT | UPDATE }
+ [,...] | ALL [ PRIVILEGES ] }
+ ON SEQUENCE replaceable class=PARAMETERtablename/replaceable [, ...]
+ TO { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...] [
WITH GRANT OPTION ]
+
GRANT { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] }
ON DATABASE replaceabledbname/replaceable [, ...]
TO { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...] [
WITH GRANT OPTION ]
***
*** 511,517
para
The literalRULE/literal privilege, and privileges on
! databases, tablespaces, schemas, languages, and sequences are
productnamePostgreSQL/productname extensions.
/para
/refsect1
--- 516,522
para
The literalRULE/literal privilege, and privileges on
! databases, tablespaces, schemas, and languages are
productnamePostgreSQL/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.sgml20 Oct 2005 19:18:01 - 1.35
--- doc/src/sgml/ref/revoke.sgml6 Jan 2006 15:23:16 -
***
*** 28,33
--- 28,40
[ CASCADE | RESTRICT ]
REVOKE [ GRANT OPTION FOR ]
+ { { SELECT | UPDATE }
+ [,...] | ALL [ PRIVILEGES ] }
+ ON SEQUENCE replaceable class=PARAMETERtablename/replaceable [, ...]
+ FROM { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...]
+ [ CASCADE | RESTRICT ]
+
+ REVOKE [ GRANT OPTION FOR ]
{ { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] }
ON DATABASE replaceabledbname/replaceable [, ...]
FROM { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/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.c1 Dec 2005 02:03:00 - 1.123
--- src/backend/catalog/aclchk.c6 Jan 2006 15:23:17 -
***
*** 283,288
--- 283,289
switch (stmt-objtype)
{
case ACL_OBJECT_RELATION:
+ case ACL_OBJECT_SEQUENCE:
all_privileges =
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
Marko Kreen [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
On Fri, Jan 06, 2006 at 19:11:27 +0200, Marko Kreen [EMAIL PROTECTED] 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. ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
Bruno Wolff III wrote: On Fri, Jan 06, 2006 at 19:11:27 +0200, Marko Kreen [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
On 1/6/06, Tom Lane [EMAIL PROTECTED] wrote: Marko Kreen [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
On 1/6/06, Bruno Wolff III [EMAIL PROTECTED] wrote: On Fri, Jan 06, 2006 at 19:11:27 +0200, Marko Kreen [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
Marko Kreen [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 - 1.50
--- doc/src/sgml/ref/grant.sgml 6 Jan 2006 20:33:45 -
***
*** 25,30
--- 25,35
ON [ TABLE ] replaceable class=PARAMETERtablename/replaceable [,
...]
TO { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...] [
WITH GRANT OPTION ]
+ GRANT { { SELECT | UPDATE }
+ [,...] | ALL [ PRIVILEGES ] }
+ ON SEQUENCE replaceable class=PARAMETERtablename/replaceable [, ...]
+ TO { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...] [
WITH GRANT OPTION ]
+
GRANT { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] }
ON DATABASE replaceabledbname/replaceable [, ...]
TO { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...] [
WITH GRANT OPTION ]
***
*** 511,517
para
The literalRULE/literal privilege, and privileges on
! databases, tablespaces, schemas, languages, and sequences are
productnamePostgreSQL/productname extensions.
/para
/refsect1
--- 516,522
para
The literalRULE/literal privilege, and privileges on
! databases, tablespaces, schemas, and languages are
productnamePostgreSQL/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.sgml20 Oct 2005 19:18:01 - 1.35
--- doc/src/sgml/ref/revoke.sgml6 Jan 2006 20:33:46 -
***
*** 28,33
--- 28,40
[ CASCADE | RESTRICT ]
REVOKE [ GRANT OPTION FOR ]
+ { { SELECT | UPDATE }
+ [,...] | ALL [ PRIVILEGES ] }
+ ON SEQUENCE replaceable class=PARAMETERtablename/replaceable [, ...]
+ FROM { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...]
+ [ CASCADE | RESTRICT ]
+
+ REVOKE [ GRANT OPTION FOR ]
{ { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] }
ON DATABASE replaceabledbname/replaceable [, ...]
FROM { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/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.c1 Dec 2005 02:03:00 - 1.123
--- src/backend/catalog/aclchk.c6 Jan 2006 20:33:46 -
***
*** 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
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
Tom Lane wrote: Marko Kreen [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
On 1/6/06, Tom Lane [EMAIL PROTECTED] wrote: Marko Kreen [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
Jaime Casanova wrote: On 1/6/06, Tom Lane [EMAIL PROTECTED] wrote: Marko Kreen [EMAIL PROTECTED] 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. 5WD-02-Foundation-2003-09.pdf look at: 4.34.2 Privileges. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 112 and 12.3 privileges. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 739 this is taken from the 12.3 3) If object name specifies a domain name, collation name, character set name, transliteration name, schema-resolved user-defined type name, or sequence generator name, then privileges may specify USAGE. Otherwise, USAGE shall not be specified. Yes, I saw that, but how does that hook into nextval/setval/currval()? I think I see it in 6.13: a) If next value expression is contained in a schema definition, then the applicable privileges for the authorization identifier that owns the containing schema shall include USAGE privilege on the sequence generator identified by sequence generator name. Is that it? -- 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 ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
On 1/6/06, Bruce Momjian pgman@candle.pha.pa.us wrote: Jaime Casanova wrote: On 1/6/06, Tom Lane [EMAIL PROTECTED] wrote: Marko Kreen [EMAIL PROTECTED] 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. 5WD-02-Foundation-2003-09.pdf look at: 4.34.2 Privileges. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 112 and 12.3 privileges. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 739 this is taken from the 12.3 3) If object name specifies a domain name, collation name, character set name, transliteration name, schema-resolved user-defined type name, or sequence generator name, then privileges may specify USAGE. Otherwise, USAGE shall not be specified. Yes, I saw that, but how does that hook into nextval/setval/currval()? I think I see it in 6.13: a) If next value expression is contained in a schema definition, then the applicable privileges for the authorization identifier that owns the containing schema shall include USAGE privilege on the sequence generator identified by sequence generator name. Is that it? USAGE is the only privilege that the standard consider applicable for the owner? it covers all operations in sequences... -- regards, Jaime Casanova (DBA: DataBase Aniquilator ;) ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
On 1/7/06, Tom Lane [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
Marko Kreen [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
Tom Lane wrote: Marko Kreen [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 - 1.50
--- doc/src/sgml/ref/grant.sgml 7 Jan 2006 06:00:14 -
***
*** 25,30
--- 25,35
ON [ TABLE ] replaceable class=PARAMETERtablename/replaceable [,
...]
TO { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...] [
WITH GRANT OPTION ]
+ GRANT { { SELECT | USAGE | UPDATE }
+ [,...] | ALL [ PRIVILEGES ] }
+ ON SEQUENCE replaceable class=PARAMETERtablename/replaceable [, ...]
+ TO { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...] [
WITH GRANT OPTION ]
+
GRANT { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] }
ON DATABASE replaceabledbname/replaceable [, ...]
TO { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...] [
WITH GRANT OPTION ]
***
*** 260,265
--- 265,274
also met). Essentially this allows the grantee to quotelook up/
objects within the schema.
/para
+ para
+For sequences, this privilege allows the use of the
+functioncurrval/function and functionnextval/function
functions.
+ /para
/listitem
/varlistentry
***
*** 511,517
para
The literalRULE/literal privilege, and privileges on
! databases, tablespaces, schemas, languages, and sequences are
productnamePostgreSQL/productname extensions.
/para
/refsect1
--- 520,526
para
The literalRULE/literal privilege, and privileges on
! databases, tablespaces, schemas, and languages are
productnamePostgreSQL/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.sgml20 Oct 2005 19:18:01 - 1.35
--- doc/src/sgml/ref/revoke.sgml7 Jan 2006 06:00:14 -
***
*** 28,33
--- 28,40
[ CASCADE | RESTRICT ]
REVOKE [ GRANT OPTION FOR ]
+ { { SELECT | UPDATE }
+ [,...] | ALL [ PRIVILEGES ] }
+ ON SEQUENCE replaceable class=PARAMETERtablename/replaceable [, ...]
+ FROM { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/replaceable | PUBLIC } [, ...]
+ [ CASCADE | RESTRICT ]
+
+ REVOKE [ GRANT OPTION FOR ]
{ { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] }
ON DATABASE replaceabledbname/replaceable [, ...]
FROM { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/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.c1 Dec 2005 02:03:00 - 1.123
--- src/backend/catalog/aclchk.c7 Jan 2006 06:00:27 -
***
*** 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
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [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 - 1.50
--- doc/src/sgml/ref/grant.sgml 5 Jan 2006 18:18:37 -
***
*** 22,28
synopsis
GRANT { { SELECT | INSERT | UPDATE | DELETE | RULE | REFERENCES | TRIGGER }
[,...] | ALL [ PRIVILEGES ] }
! ON [ TABLE ] replaceable class=PARAMETERtablename/replaceable [,
...]
TO { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/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=PARAMETERtablename/replaceable [, ...]
TO { replaceable class=PARAMETERusername/replaceable | GROUP
replaceable class=PARAMETERgroupname/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 - 2.521
--- src/backend/parser/gram.y 5 Jan 2006 18:18:41 -
***
*** 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);
---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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 ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
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. ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
