Обсуждение: Repetition of warning message while REVOKE
<div class="bugEvent">Hi,<br /><br />Description:<br />===========<br /> Repetition of warning message with revoke.<br /><br/>How to reproduce : <br />==================<br /><br />> create table tbl(col int);<br /> > create user usr;<br/> > grant select on tbl to usr;<br /> > \c postgres usr;<br />> REVOKE SELECT on tbl from usr;<br /><br/> Actual output:<br />================<br /> WARNING: no privileges could be revoked for "tbl"<br /> WARNING: noprivileges could be revoked for "tbl"<br /> WARNING: no privileges could be revoked for "tbl"<br /> WARNING: no privilegescould be revoked for "tbl"<br /> WARNING: no privileges could be revoked for "tbl"<br /> WARNING: no privilegescould be revoked for "tbl"<br /> WARNING: no privileges could be revoked for "tbl"<br /> WARNING: no privilegescould be revoked for "tbl"<br /> REVOKE<br /><br /> expected output:<br />===============<br /> Shouldn't printrepetitive warnings.<br /></div><span class="bugEventHeader" dir="ltr"></span><br clear="all" />-- <br />Piyush S Newe<br/>Principal Engineer<br />EnterpriseDB<br />office: +91 20 3058 9500<br /><a href="http://www.enterprisedb.com">www.enterprisedb.com</a><br/><br />Website: <a href="http://www.enterprisedb.com">www.enterprisedb.com</a><br/>EnterpriseDB Blog: <a href="http://blogs.enterprisedb.com/">http://blogs.enterprisedb.com/</a><br/>Follow us on Twitter: <a href="http://www.twitter.com/enterprisedb">http://www.twitter.com/enterprisedb</a><br/><br />This e-mail message (and anyattachment) is intended for the use of the individual or entity to whom it is addressed. This message contains informationfrom EnterpriseDB Corporation that may be privileged, confidential, or exempt from disclosure under applicablelaw. 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 receivedthis e-mail in error, please notify the sender immediately by reply e-mail and delete this message.<br /><br />
Piyush Newe <piyush.newe@enterprisedb.com> 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
I wrote:
> Piyush Newe <piyush.newe@enterprisedb.com> 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
On Thu, 2010-03-04 at 11:23 -0500, Tom Lane wrote: > I wrote: > > Piyush Newe <piyush.newe@enterprisedb.com> 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.
On Thu, 2010-03-04 at 11:23 -0500, Tom Lane wrote: > I wrote: > > Piyush Newe <piyush.newe@enterprisedb.com> 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.
All,
* Joshua D. Drake (jd@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
Вложения
Stephen Frost <sfrost@snowman.net> writes:
> * Joshua D. Drake (jd@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 likeno 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
* Tom Lane (tgl@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
Вложения
Stephen Frost <sfrost@snowman.net> writes:
> * Tom Lane (tgl@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