Horton

Thread

Commits

  1. Refactor documentation about privileges to centralize the info.

  1. doc - improve description of default privileges

    Fabien COELHO <coelho@cri.ensmp.fr> — 2018-07-31T21:43:39Z

    I have not found a convenient presentation of the default privileges for 
different objects, and how to display them (if possible, not always).

The information is partly provided within the GRANT description, and not 
very explicit: eg it is said that owners have all possible perms, but 
which they are is not said explicitely, although they are implied by the 
different GRANT sysnopsys. Then some objects are given perms for the 
PUBLIC.

The attached patch tries to improve the documentation, in particular with 
an added table to summarizes my findings, so that they are recorded 
somewhere.

-- 
Fabien.

  2. Re: doc - improve description of default privileges

    Fabien COELHO <coelho@cri.ensmp.fr> — 2018-08-04T09:40:33Z

    > I have not found a convenient presentation of the default privileges for 
> different objects, and how to display them (if possible, not always).
>
> The information is partly provided within the GRANT description, and not very 
> explicit: eg it is said that owners have all possible perms, but which they 
> are is not said explicitely, although they are implied by the different GRANT 
> sysnopsys. Then some objects are given perms for the PUBLIC.
>
> The attached patch tries to improve the documentation, in particular with an 
> added table to summarizes my findings, so that they are recorded somewhere.

The attached fixes the tablespace entry that I forgot to fill in full.

-- 
Fabien.

  3. Re[2]: doc - improve description of default privileges

    Brad DeJong <bpd0018@gmail.com> — 2018-08-27T22:19:49Z

    Hi Fabien,

Thanks for writing this up - in particular the psql backslash commands.

comments on the patch ...

1) I think that adding the "This privilege is abbreviated ... when 
displayed." lines to the privilege descriptions is redundant. The 
abbreviations are already listed after the "The entries shown by \dp are 
interpreted thus:" line. Just change that line to something like "The 
entries shown by the psql backslash commands, like \dp, are interpreted 
thus:".

2) I think that the psql command table should go with the current text 
on "Use psql's \dp command to obtain ..." rather than in the Examples 
section. It seems like changing the "For non-table objects there are 
other \d commands ..." line to an introductory comment like "The 
following table lists the \d commands that are used for non-table 
objects along with the default privileges granted to the object's owner 
and PUBLIC.

3) The table title, "Default hardcoded access privileges per object's 
type", seems incomplete because it does not mention the psql commands 
part of the table.


------ Original Message ------
From: "Fabien COELHO" <coelho@cri.ensmp.fr>
To: "PostgreSQL Developers" <pgsql-hackers@lists.postgresql.org>
Sent: 8/4/2018 4:40:33 AM
Subject: Re: doc - improve description of default privileges

>
>>I have not found a convenient presentation of the default privileges 
>>for different objects, and how to display them (if possible, not 
>>always).
>>
>>The information is partly provided within the GRANT description, and 
>>not very explicit: eg it is said that owners have all possible perms, 
>>but which they are is not said explicitely, although they are implied 
>>by the different GRANT sysnopsys. Then some objects are given perms 
>>for the PUBLIC.
>>
>>The attached patch tries to improve the documentation, in particular 
>>with an added table to summarizes my findings, so that they are 
>>recorded somewhere.
>
>The attached fixes the tablespace entry that I forgot to fill in full.
>
>-- Fabien.

  4. Re[2]: doc - improve description of default privileges

    Fabien COELHO <coelho@cri.ensmp.fr> — 2018-08-30T07:33:41Z

    Hello Bradley,

> comments on the patch ...

Thanks for the review.

> 1) I think that adding the "This privilege is abbreviated ... when 
> displayed." lines to the privilege descriptions is redundant. The 
> abbreviations are already listed after the "The entries shown by \dp are 
> interpreted thus:" line. Just change that line to something like "The entries 
> shown by the psql backslash commands, like \dp, are interpreted thus:".

Ok, removed.

> 2) I think that the psql command table should go with the current text on 
> "Use psql's \dp command to obtain ..." rather than in the Examples section. 
> It seems like changing the "For non-table objects there are other \d commands 
> ..." line to an introductory comment like "The following table lists the \d 
> commands that are used for non-table objects along with the default 
> privileges granted to the object's owner and PUBLIC.

Ok, moved to the previous section.

> 3) The table title, "Default hardcoded access privileges per object's type", 
> seems incomplete because it does not mention the psql commands part of the 
> table.

Ok, added reference to psql backslash commands in title.

Find v3 attached.

-- 
Fabien.

  5. Re[3]: doc - improve description of default privileges

    Brad DeJong <bpd0018@gmail.com> — 2018-09-12T21:45:19Z

    on 2018-08-30, Fabien Coelho wrote ...
 > ... Find v3 attached. ...

Hi Fabien,

As we're coming up on the end of this commitfest ...

Is the reviewer supposed to move this to "ready for committer" or is the 
author supposed to do that?

Is the reviewer supposed to explicitly state "I've looked at your v3 
patch and have no further suggestions" (which is true) or is a lack of 
additional comments normally taken as acceptance?


Cheers.

  6. Re: Re[3]: doc - improve description of default privileges

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-09-12T21:48:30Z

    "Bradley DeJong" <bpd0018@gmail.com> writes:
> Is the reviewer supposed to move this to "ready for committer" or is the 
> author supposed to do that?

The reviewer does that, indicating signoff.

			regards, tom lane

  7. Re: Re[3]: doc - improve description of default privileges

    Brad DeJong <bpd0018@gmail.com> — 2018-09-14T11:27:09Z

    On 2018-09-12 Tom Lane wrote ...
> The reviewer does that, indicating signoff.
Thanks. I have now changed the status to "Ready for Committer".

  8. Re: doc - improve description of default privileges

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2018-09-28T19:01:04Z

    Some thoughts:

We should keep the GRANT reference page about GRANT.  There is a section
about Privileges in the Data Definition chapter, which we could use to
expand on general concepts.

The ALTER DEFAULT PRIVILEGES reference page would be another place this
could be put.

The Owner column is redundant, because it's always all applicable
privileges.  (Having this column would give the impression that it's not
always all privileges, so it would be confusing.)

Privileges should be listed using their full name (e.g., "SELECT"), not
their internal abbreviation letter.

The psql commands seem out of place here.  If you want to learn about
how to use psql, you can go to the psql documentation.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  9. Re: doc - improve description of default privileges

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2018-09-28T19:15:13Z

    On 2018-Sep-28, Peter Eisentraut wrote:

> The psql commands seem out of place here.  If you want to learn about
> how to use psql, you can go to the psql documentation.

There is a legitimate point in doing this, though, since the GRANT page
is already explaining how does psql display privileges.  Maybe the right
solution is move that stuff all to the psql documentation, and alter the
GRANT page to list privileges in terms of their names rather than the
way psql displays them.  (And of course add cross-links, so that it all
makes sense.)

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  10. Re: doc - improve description of default privileges

    Fabien COELHO <coelho@cri.ensmp.fr> — 2018-09-29T07:56:41Z

    > The Owner column is redundant, because it's always all applicable
> privileges.  (Having this column would give the impression that it's not
> always all privileges, so it would be confusing.)

The reason I put the owner column is to show (1) the privileges that apply 
to the objects (i.e. what is under "ALL") and (2) whether public's 
privileges are the same or not, because there are subtles differences, so 
I think it is interesting to have them side by side somewhere.

> Privileges should be listed using their full name (e.g., "SELECT"), not
> their internal abbreviation letter.

Hmmm... the psql commands listed in the table output the abbreviated 
letters. Using the words would result in a large table, but maybe it 
could use multiline cells.

> The psql commands seem out of place here.  If you want to learn about
> how to use psql, you can go to the psql documentation.

The information about how to display the permissions is currently not 
easily available, I had to test/look for it, noticed that it is not 
accessible on some objects, so ISTM that it is useful to have it 
somewhere.

Basically your points suggest that the table is maybe in the wrong place
and could be improved.

About the place, there is no simple choice:

  - backslash commands are "psql" specific
  - abbreviated letters are aclitem function specific, which
    happend to be used by psql.
  - full names are SQL specific (GRANT)
  - default permissions are object specific and can be modified...

Which means that the information tends to be scattered everywhere and 
overall pretty unclear unless you have read all the pages, hence my 
proposal to put some unified summary somewhere with all the relevant 
information. Any choice will have its downside, and removing information 
to better suit one place means that my point of having some kind of 
summary in one place is lost, which is the initial motivation for this 
patch.

-- 
Fabien.

  11. Re: doc - improve description of default privileges

    Michael Paquier <michael@paquier.xyz> — 2018-10-01T05:47:18Z

    On Fri, Sep 28, 2018 at 04:15:13PM -0300, Alvaro Herrera wrote:
> There is a legitimate point in doing this, though, since the GRANT page
> is already explaining how does psql display privileges.  Maybe the right
> solution is move that stuff all to the psql documentation, and alter the
> GRANT page to list privileges in terms of their names rather than the
> way psql displays them.  (And of course add cross-links, so that it all
> makes sense.)

This point is interesting, and that looks rather right to me.  I looked
at the patch and listing psql commands on the page for GRANT is a
strange concept in my opinion.  Moved to next CF.
--
Michael

  12. Re: doc - improve description of default privileges

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-11-15T00:03:32Z

    Progress on this patch seems to be blocked on the question of whether
we want to keep enlarging the amount of psql-specific information
in the GRANT reference page, or move that all somewhere else.

FWIW, I think I agree with Peter's position that moving it somewhere
else is the better option.  Section 5.6 "Privileges" seems like a
reasonable choice.

One reason for preferring that is that I don't think putting a <table> in
a reference page is a great idea.  I tried the patch in HEAD, and what
I see is that the table cross-reference renders as "Table 244", which is
a number that has no chance at all of holding still for any long period.
It's especially weird to read that in "man GRANT.7", where you're
supposedly reading a standalone document.  And while my version of "man"
makes a valiant effort to render the table in ASCII, it still doesn't look
great, and older man versions might do very poorly with that.  So I'm for
moving this to a part of the docs where we only need to worry about two
output formats not three.

A few thoughts on other issues:

* Perhaps we could fix Peter's complaint about the "Owner" column by
relabeling it "All Privileges".  I'd be inclined to label the last
column "Default PUBLIC Privileges", too, if we can fit that in.

* The phrase "relation-like objects" seems way too vague, especially since
one has to read it as excluding sequences, which surely are relations for
most purposes.  Is there a good reason not to just leave that entry as
"TABLE", full stop?  Or maybe it could be "TABLE, VIEW, etc" or some such.

* I don't think the use of "hardcoded" adds anything.

* Is it worth adding another table matching privilege names ("INSERT")
with their aclitem letters ("a"), rather than having the semi-formal
format currently appearing in grant.sgml?  There's also some related
material in 9.25 with the aclitem functions; it'd be worth unifying
that too maybe.

			regards, tom lane

  13. Re: doc - improve description of default privileges

    Fabien COELHO <coelho@cri.ensmp.fr> — 2018-11-18T19:05:46Z

    Hello Tom,

Thanks for this precise feedback.

> Progress on this patch seems to be blocked on the question of whether
> we want to keep enlarging the amount of psql-specific information
> in the GRANT reference page, or move that all somewhere else.

Yep.

> FWIW, I think I agree with Peter's position that moving it somewhere
> else is the better option.  Section 5.6 "Privileges" seems like a
> reasonable choice.

Ok.

> * Perhaps we could fix Peter's complaint about the "Owner" column by
> relabeling it "All Privileges".

Ok.

> I'd be inclined to label the last column "Default PUBLIC Privileges", 
> too, if we can fit that in.

Ok.

> * The phrase "relation-like objects" seems way too vague, especially since
> one has to read it as excluding sequences, which surely are relations for
> most purposes.  Is there a good reason not to just leave that entry as
> "TABLE", full stop?  Or maybe it could be "TABLE, VIEW, etc" or some such.

Ok.

> * I don't think the use of "hardcoded" adds anything.

Hmmm. As "default privileges" can be altered, the point is to describe the 
"default default privileges", but this looks absurd, hence the look for 
something to add the idea that there is another one. ISTM that removing 
"hardcoded" without replacing it makes the thing slightly ambiguous.
No big deal.

> * Is it worth adding another table matching privilege names ("INSERT")
> with their aclitem letters ("a"), rather than having the semi-formal
> format currently appearing in grant.sgml?

Indeed I thought about that, because the description is not easy to read.

> There's also some related material in 9.25 with the aclitem functions; 
> it'd be worth unifying that too maybe.

I've put a reference to it at least.

Attached v4:
  - moves the table to the privileges section
  - updates the table column headers
  - adds a privilege/aclitem letter mapping table
  - adds some appropriate links towards psql & aclitem

-- 
Fabien.

  14. Re: doc - improve description of default privileges

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-11-30T21:44:54Z

    Fabien COELHO <coelho@cri.ensmp.fr> writes:
> Attached v4:
>   - moves the table to the privileges section
>   - updates the table column headers
>   - adds a privilege/aclitem letter mapping table
>   - adds some appropriate links towards psql & aclitem

TBH, I don't think this goes nearly far enough.  It seems like it
is making the fragmentation of aclitem information worse not better.
I feel if we're going to do anything, we should put a unified description
of privileges and aclitem-reading into section 5.6, and take that material
out of the various places where it lives now.  Like the attached, in which
I failed to resist the temptation to wordsmith some stuff as well as move
it around.

			regards, tom lane

  15. Re: doc - improve description of default privileges

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2018-11-30T22:19:35Z

    On 2018-Nov-30, Tom Lane wrote:

> I feel if we're going to do anything, we should put a unified description
> of privileges and aclitem-reading into section 5.6, and take that material
> out of the various places where it lives now.  Like the attached, in which
> I failed to resist the temptation to wordsmith some stuff as well as move
> it around.

I looked at the psql manpage and the HTML rendering of section 5.6 and
it all looks good to me.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  16. Re: doc - improve description of default privileges

    Fabien COELHO <coelho@cri.ensmp.fr> — 2018-12-03T07:44:27Z

    
>> I feel if we're going to do anything, we should put a unified description
>> of privileges and aclitem-reading into section 5.6, and take that material
>> out of the various places where it lives now.  Like the attached, in which
>> I failed to resist the temptation to wordsmith some stuff as well as move
>> it around.
>
> I looked at the psql manpage and the HTML rendering of section 5.6 and
> it all looks good to me.

Indeed, this looks great, a precise and full description of privileges 
just in one place.

-- 
Fabien.

  17. Re: doc - improve description of default privileges

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-12-03T16:42:20Z

    Fabien COELHO <coelho@cri.ensmp.fr> writes:
>>> I feel if we're going to do anything, we should put a unified description
>>> of privileges and aclitem-reading into section 5.6, and take that material
>>> out of the various places where it lives now.  Like the attached, in which
>>> I failed to resist the temptation to wordsmith some stuff as well as move
>>> it around.

>> I looked at the psql manpage and the HTML rendering of section 5.6 and
>> it all looks good to me.

> Indeed, this looks great, a precise and full description of privileges 
> just in one place.

Pushed (with a little bit more tweaking).

			regards, tom lane

  18. Re: doc - improve description of default privileges

    Fabien COELHO <coelho@cri.ensmp.fr> — 2018-12-04T07:16:24Z

    >>> I looked at the psql manpage and the HTML rendering of section 5.6 and
>>> it all looks good to me.
>
>> Indeed, this looks great, a precise and full description of privileges
>> just in one place.
>
> Pushed (with a little bit more tweaking).

Thanks for the rewrite, extensions, improvements and final push.

-- 
Fabien.