Re: [HACKERS] Repetition of warning message while REVOKE
Stephen Frost writes: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> One thought is that the column cases should be phrased more like >> no privileges could be revoked for column "foo" of table "bar" > Looks like 'for column "foo" of relation "bar"' is more typical, so > that's what I did in the attached patch. I also cleaned up a few other > things I noticed in looking through the various messages/comments. Applied, except I omitted the one comment change because it didn't seem to me to clarify anything. Sequences are a subclass of relations, so "table or sequence" makes sense to me while "relation or sequence" doesn't. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Repetition of warning message while REVOKE
* Tom Lane (t...@sss.pgh.pa.us) wrote:
> One thought is that the column cases should be phrased more like
> no privileges could be revoked for column "foo" of table "bar"
> Check the messages associated with DROP cascading for the canonical
> phrasing here, but I think that's what it is.
Looks like 'for column "foo" of relation "bar"' is more typical, so
that's what I did in the attached patch. I also cleaned up a few other
things I noticed in looking through the various messages/comments.
Thanks!
Stephen
? src/backend/regex/.regcomp.c.swp
? src/pl/plpgsql/src/pl_scan.c
Index: src/backend/catalog/aclchk.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/catalog/aclchk.c,v
retrieving revision 1.163
diff -c -r1.163 aclchk.c
*** src/backend/catalog/aclchk.c 26 Feb 2010 02:00:35 - 1.163
--- src/backend/catalog/aclchk.c 5 Mar 2010 13:16:48 -
***
*** 304,327
if (is_grant)
{
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;
--- 304,365
if (is_grant)
{
if (this_privileges == 0)
! {
! if (objkind == ACL_KIND_COLUMN && colname)
! ereport(WARNING,
! (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! errmsg("no privileges were granted for column \"%s\" of relation \"%s\"",
! colname, objname)));
! else
! ereport(WARNING,
! (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! errmsg("no privileges were granted for \"%s\"", objname)));
! }
! else
! {
! if (!all_privs && this_privileges != privileges)
! {
! if (objkind == ACL_KIND_COLUMN && colname)
! ereport(WARNING,
! (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! errmsg("not all privileges were granted for column \"%s\" of relation \"%s\"",
! colname, objname)));
! else
! ereport(WARNING,
! (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! errmsg("not all privileges were granted for \"%s\"", objname)));
! }
! }
}
else
{
if (this_privileges == 0)
! {
! if (objkind == ACL_KIND_COLUMN && colname)
! ereport(WARNING,
! (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! errmsg("no privileges could be revoked for column \"%s\" of relation \"%s\"",
! colname, objname)));
! else
! ereport(WARNING,
! (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! errmsg("no privileges could be revoked for \"%s\"", objname)));
! }
! else
! {
! if (!all_privs && this_privileges != privileges)
! {
! if (objkind == ACL_KIND_COLUMN && colname)
! ereport(WARNING,
! (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! errmsg("not all privileges could be revoked for column \"%s\" of relation \"%s\"",
! colname, objname)));
! else
! ereport(WARNING,
! (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! errmsg("not all privileges could be revoked for \"%s\"", objname)));
! }
! }
}
return this_privileges;
***
*** 1657,1664
/*
* 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)
{
--- 1695,1702
/*
* 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 relation and sequence permissions were
! * already checked.
*/
if (istmt->objtype == ACL_OBJECT_RELATION)
{
***
*** 3046,3052
case ACLCHECK_NO_PRIV:
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("permission denied for column %s of relation %s",
colname, objectname)));
break;
case ACLCHECK_NOT_OWNER:
--- 3084,3090
case ACLCHECK_NO_PRIV:
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("permission denied for column \"%s\" of relation \"%s\"",
colname, objectname)));
b
Re: [HACKERS] Repetition of warning message while REVOKE
Stephen Frost writes: > * Joshua D. Drake (j...@commandprompt.com) wrote: >> Perhaps just add what can't be revoked? meaning: >> WARNING: no privileges could be revoked for "tbl" for column "foo" >> Then they aren't actually duplicate. > Yeah, they really aren't, after all. Yeah, I agree JD's solution is probably the simplest reasonable answer. > Attached is a patch to add column name to the error message when it's a > column-level failure. I'm not really thrilled with it, due to the > expansion of code and addition of a bunch of conditionals, but at least > this isn't a terribly complicated function.. It's a bit brute-force, but so was the original coding. Anybody see a way to make it cleaner/shorter? One thought is that the column cases should be phrased more like no privileges could be revoked for column "foo" of table "bar" Check the messages associated with DROP cascading for the canonical phrasing here, but I think that's what it is. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Repetition of warning message while REVOKE
All,
* Joshua D. Drake (j...@commandprompt.com) wrote:
> On Thu, 2010-03-04 at 11:23 -0500, Tom Lane wrote:
> > I'm not sure offhand about a reasonable way to rearrange the code to
> > avoid duplicate messages.
>
> Perhaps just add what can't be revoked? meaning:
> WARNING: no privileges could be revoked for "tbl" for column "foo"
> Then they aren't actually duplicate.
Yeah, they really aren't, after all. I don't know how we could
rearrange the code to prevent it- we're checking and trying to change
privileges on each of the columns in the table, after all.
Attached is a patch to add column name to the error message when it's a
column-level failure. I'm not really thrilled with it, due to the
expansion of code and addition of a bunch of conditionals, but at least
this isn't a terribly complicated function..
In the process of trying to build/run regression tests, but having
some build issues w/ HEAD (probably my fault).
Thanks,
Stephen
Index: src/backend/catalog/aclchk.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/catalog/aclchk.c,v
retrieving revision 1.163
diff -c -r1.163 aclchk.c
*** src/backend/catalog/aclchk.c 26 Feb 2010 02:00:35 - 1.163
--- src/backend/catalog/aclchk.c 5 Mar 2010 01:18:42 -
***
*** 304,327
if (is_grant)
{
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;
--- 304,365
if (is_grant)
{
if (this_privileges == 0)
! {
! if (objkind == ACL_KIND_COLUMN && colname)
! ereport(WARNING,
! (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! errmsg("no privileges were granted for \"%s\", for column \"%s\"",
! objname, colname)));
! else
! ereport(WARNING,
! (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! errmsg("no privileges were granted for \"%s\"", objname)));
! }
! else
! {
! if (!all_privs && this_privileges != privileges)
! {
! if (objkind == ACL_KIND_COLUMN && colname)
! ereport(WARNING,
! (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! errmsg("not all privileges were granted for \"%s\", for column \"%s\"",
! objname, colname)));
! else
! ereport(WARNING,
! (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! errmsg("not all privileges were granted for \"%s\"", objname)));
! }
! }
}
else
{
if (this_privileges == 0)
! {
! if (objkind == ACL_KIND_COLUMN && colname)
! ereport(WARNING,
! (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! errmsg("no privileges could be revoked for \"%s\", for column \"%s\"",
! objname, colname)));
! else
! ereport(WARNING,
! (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! errmsg("no privileges could be revoked for \"%s\"", objname)));
! }
! else
! {
! if (!all_privs && this_privileges != privileges)
! {
! if (objkind == ACL_KIND_COLUMN && colname)
! ereport(WARNING,
! (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! errmsg("not all privileges could be revoked for \"%s\", for column \"%s\"",
! objname, colname)));
! else
! ereport(WARNING,
! (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! errmsg("not all privileges could be revoked for \"%s\"", objname)));
! }
! }
}
return this_privileges;
signature.asc
Description: Digital signature
Re: [HACKERS] Repetition of warning message while REVOKE
On Thu, 2010-03-04 at 11:23 -0500, Tom Lane wrote: > I wrote: > > Piyush Newe writes: > >> create table tbl(col int); > >> create user usr; > >> grant select on tbl to usr; > >> \c postgres usr; > >> REVOKE SELECT on tbl from usr; > >> WARNING: no privileges could be revoked for "tbl" > >> WARNING: no privileges could be revoked for "tbl" > >> WARNING: no privileges could be revoked for "tbl" > >> WARNING: no privileges could be revoked for "tbl" > >> WARNING: no privileges could be revoked for "tbl" > >> WARNING: no privileges could be revoked for "tbl" > >> WARNING: no privileges could be revoked for "tbl" > >> WARNING: no privileges could be revoked for "tbl" > >> REVOKE > > > You really should mention what version you're testing, but for the > > archives: I confirm this on 8.4.x and HEAD. 8.3 seems to behave sanely. > > I traced through this and determined that the extra messages are a > consequence of the column-level-privileges patch. > restrict_and_check_grant is invoked both on the whole relation, and > on each column (since we have to get rid of any per-column SELECT > privilege that might have been granted). > > I'm not sure offhand about a reasonable way to rearrange the code to > avoid duplicate messages. Perhaps just add what can't be revoked? meaning: WARNING: no privileges could be revoked for "tbl" for column "foo" Then they aren't actually duplicate. Sincerely, Joshau D. Drake > > regards, tom lane > -- PostgreSQL.org Major Contributor Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564 Consulting, Training, Support, Custom Development, Engineering Respect is earned, not gained through arbitrary and repetitive use or Mr. or Sir. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Repetition of warning message while REVOKE
I wrote: > Piyush Newe writes: >> create table tbl(col int); >> create user usr; >> grant select on tbl to usr; >> \c postgres usr; >> REVOKE SELECT on tbl from usr; >> WARNING: no privileges could be revoked for "tbl" >> WARNING: no privileges could be revoked for "tbl" >> WARNING: no privileges could be revoked for "tbl" >> WARNING: no privileges could be revoked for "tbl" >> WARNING: no privileges could be revoked for "tbl" >> WARNING: no privileges could be revoked for "tbl" >> WARNING: no privileges could be revoked for "tbl" >> WARNING: no privileges could be revoked for "tbl" >> REVOKE > You really should mention what version you're testing, but for the > archives: I confirm this on 8.4.x and HEAD. 8.3 seems to behave sanely. I traced through this and determined that the extra messages are a consequence of the column-level-privileges patch. restrict_and_check_grant is invoked both on the whole relation, and on each column (since we have to get rid of any per-column SELECT privilege that might have been granted). I'm not sure offhand about a reasonable way to rearrange the code to avoid duplicate messages. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Repetition of warning message while REVOKE
Piyush Newe writes: > create table tbl(col int); > create user usr; > grant select on tbl to usr; > \c postgres usr; > REVOKE SELECT on tbl from usr; > WARNING: no privileges could be revoked for "tbl" > WARNING: no privileges could be revoked for "tbl" > WARNING: no privileges could be revoked for "tbl" > WARNING: no privileges could be revoked for "tbl" > WARNING: no privileges could be revoked for "tbl" > WARNING: no privileges could be revoked for "tbl" > WARNING: no privileges could be revoked for "tbl" > WARNING: no privileges could be revoked for "tbl" > REVOKE You really should mention what version you're testing, but for the archives: I confirm this on 8.4.x and HEAD. 8.3 seems to behave sanely. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Repetition of warning message while REVOKE
Hi, Description: === Repetition of warning message with revoke. How to reproduce : == > create table tbl(col int); > create user usr; > grant select on tbl to usr; > \c postgres usr; > REVOKE SELECT on tbl from usr; Actual output: WARNING: no privileges could be revoked for "tbl" WARNING: no privileges could be revoked for "tbl" WARNING: no privileges could be revoked for "tbl" WARNING: no privileges could be revoked for "tbl" WARNING: no privileges could be revoked for "tbl" WARNING: no privileges could be revoked for "tbl" WARNING: no privileges could be revoked for "tbl" WARNING: no privileges could be revoked for "tbl" REVOKE expected output: === Shouldn't print repetitive warnings. -- Piyush S Newe Principal Engineer EnterpriseDB office: +91 20 3058 9500 www.enterprisedb.com Website: www.enterprisedb.com EnterpriseDB Blog: http://blogs.enterprisedb.com/ Follow us on Twitter: http://www.twitter.com/enterprisedb This e-mail message (and any attachment) is intended for the use of the individual or entity to whom it is addressed. This message contains information from EnterpriseDB Corporation that may be privileged, confidential, or exempt from disclosure under applicable law. If you are not the intended recipient or authorized to receive this for the intended recipient, any use, dissemination, distribution, retention, archiving, or copying of this communication is strictly prohibited. If you have received this e-mail in error, please notify the sender immediately by reply e-mail and delete this message.
