Re: Show INHERIT in \du

Hello hackers,

psql's \du command currently does not list the INHERIT role attribute.
It does show the other privilege attributes (superuser, create role,
create db), and INHERIT seems like the kind of thing a user
executing\du would want to know.

I'd like to add it to \du. The downside is that it would add width to
an already rather wide output, but I see that as a worthwhile
tradeoff. If I'm in the minority there, perhaps I could just add it
to \du+?

TIA for your comments.

Cheers,
BJ

--On Donnerstag, Februar 14, 2008 15:52:31 +1100 Brendan Jurd
<direvus(at)gmail(dot)com> wrote:

> Hello hackers,
>
> psql's \du command currently does not list the INHERIT role attribute.
> It does show the other privilege attributes (superuser, create role,
> create db), and INHERIT seems like the kind of thing a user
> executing\du would want to know.
>
> I'd like to add it to \du. The downside is that it would add width to
> an already rather wide output, but I see that as a worthwhile
> tradeoff. If I'm in the minority there, perhaps I could just add it
> to \du+?
>

There's also a tiny patch adding the LOGIN privilege to \du pending:

<http://archives.postgresql.org/pgsql-patches/2007-11/msg00014.php>

I don't see it on the unapplied patches lists however....

--
Thanks

Bernd

Brendan Jurd escribió:
> Hello hackers,
>
> psql's \du command currently does not list the INHERIT role attribute.
> It does show the other privilege attributes (superuser, create role,
> create db), and INHERIT seems like the kind of thing a user
> executing\du would want to know.
>
> I'd like to add it to \du. The downside is that it would add width to
> an already rather wide output, but I see that as a worthwhile
> tradeoff. If I'm in the minority there, perhaps I could just add it
> to \du+?

I wonder if it's possible to create a more compact output -- say, a
fixed-width column, with a letter for each enabled attribute, or a space
in the respective position when the attribute is disabled. Legend of
letter at the bottom of the output, or some such. So instead of

alvherre=# \du
List of roles
Role name | Superuser | Create role | Create DB | Connections | Member of
-----------+-----------+-------------+-----------+-------------+-----------
alvherre | yes | yes | yes | no limit | {}
foo | no | no | yes | no limit | {}
(2 rows)

We would have something like

alvherre=# \du
List of roles
Role name | Attributes | Connections | Member of
-----------+------------+-------------+-----------
alvherre | SRDI | no limit | {}
foo | D | no limit | {}
(2 rows)
Attributes: S -- superuser
R -- create role
D -- create database
I -- inherit

For those that think that the \du output is currently OK because it
doesn't wrap, consider the french translation which is quite a bit wider
than the original:

alvherre=# \du
Liste des rôles
Nom du rôle | Superutilisateur | Créer un rôle | Créer une base | Connexions | Membre de
-------------+------------------+---------------+----------------+-------------+-----------
alvherre | oui | oui | oui | sans limite | {}
(1 ligne)

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

--On Donnerstag, Februar 14, 2008 10:27:37 -0300 Alvaro Herrera
<alvherre(at)commandprompt(dot)com> wrote:

> I wonder if it's possible to create a more compact output -- say, a
> fixed-width column, with a letter for each enabled attribute, or a space
> in the respective position when the attribute is disabled. Legend of
> letter at the bottom of the output, or some such. So instead of
>
> alvherre=# \du
> List of roles
> Role name | Superuser | Create role | Create DB | Connections | Member
> of
> -----------+-----------+-------------+-----------+-------------+---------
> -- alvherre | yes | yes | yes | no limit | {}
> foo | no | no | yes | no limit | {} (2
> rows)
>
> We would have something like
>
> alvherre=# \du
> List of roles
> Role name | Attributes | Connections | Member of
> -----------+------------+-------------+-----------
> alvherre | SRDI | no limit | {}
> foo | D | no limit | {}
> (2 rows)
> Attributes: S -- superuser
> R -- create role
> D -- create database
> I -- inherit

Hmm, i don't think that makes it easier to read, especially when you get a
long list of role names and the legend at the bottom of it. And you are
always able to get a compact view with \x and \du <rolename>.

--
Thanks

Bernd

Bernd Helmle <mailings(at)oopsware(dot)de> writes:
> --On Donnerstag, Februar 14, 2008 10:27:37 -0300 Alvaro Herrera
> <alvherre(at)commandprompt(dot)com> wrote:
>> I wonder if it's possible to create a more compact output -- say, a
>> fixed-width column, with a letter for each enabled attribute, or a space
>> in the respective position when the attribute is disabled.

> Hmm, i don't think that makes it easier to read, especially when you get a
> long list of role names and the legend at the bottom of it.

Also, what about localization? In some languages you might need to
resort to ugly phrasings to avoid having the same prefix letter for
several attributes.

Now that psql prints multiline field values nicely, maybe it'd work
to fold all the boolean attributes into one column. I'm imagining
something like

Role name | Privileges | Member of
----------+-------------+-----------
postgres | superuser | {}
| create role |
| create db |
joeblow | | {users}
foobar | | {users}

regards, tom lane

Alvaro Herrera a écrit :
> [...]
> For those that think that the \du output is currently OK because it
> doesn't wrap, consider the french translation which is quite a bit wider
> than the original:
>
> alvherre=# \du
> Liste des rôles
> Nom du rôle | Superutilisateur | Créer un rôle | Créer une base | Connexions | Membre de
> -------------+------------------+---------------+----------------+-------------+-----------
> alvherre | oui | oui | oui | sans limite | {}
> (1 ligne)
>

+1

I'll be quite happy if we could decrease this.

--
Guillaume.
http://www.postgresqlfr.org
http://dalibo.com

Tom Lane escribió:

> Now that psql prints multiline field values nicely, maybe it'd work
> to fold all the boolean attributes into one column. I'm imagining
> something like
>
> Role name | Privileges | Member of
> ----------+-------------+-----------
> postgres | superuser | {}
> | create role |
> | create db |
> joeblow | | {users}
> foobar | | {users}

Hmm, this looks like a neat idea (provided we keep the list of possible
privileges short.)

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

On Fri, Feb 15, 2008 at 3:42 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Now that psql prints multiline field values nicely, maybe it'd work
> to fold all the boolean attributes into one column. I'm imagining
> something like
>
> Role name | Privileges | Member of
> ----------+-------------+-----------
> postgres | superuser | {}
> | create role |
> | create db |
> joeblow | | {users}
> foobar | | {users}
>

I like it -- very compact compared with what we currently have, but
still easy to read.

What about connection limit? I suppose we could combine it into the
privileges column, and refrain from displaying anything for "no
limit".

Role name | Privileges | Member of
----------+-------------+-----------
postgres | superuser | {}
| create role |
| create db |
joeblow | | {users}
foobar | 5 connections | {users}

Cheers
BJ

"Brendan Jurd" <direvus(at)gmail(dot)com> writes:
> What about connection limit? I suppose we could combine it into the
> privileges column, and refrain from displaying anything for "no
> limit".

"5 connections" seems a bit odd for a "privilege". I'd be happy with
leaving that as a separate column. Or I guess we could choose a
less specific word (like "attributes"?) for the column heading.

regards, tom lane

On Fri, Feb 15, 2008 at 10:50 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Brendan Jurd" <direvus(at)gmail(dot)com> writes:
> > What about connection limit? I suppose we could combine it into the
> > privileges column, and refrain from displaying anything for "no
> > limit".
>
> "5 connections" seems a bit odd for a "privilege". I'd be happy with
> leaving that as a separate column. Or I guess we could choose a
> less specific word (like "attributes"?) for the column heading.
>

"Attributes" works better IMO. The manual refers to INHERIT and LOGIN
as "attributes" rather than "privileges", so I would go for
"attributes" regardless of where we place connection limit.

Leaving connection limit in a separate column would be okay, but if
most installations seldom use connection limit (and I suspect this is
true), that's a chunky amount of underutilised space in the \du
output.

Cheers
BJ

On Feb 14, 2008, at 7:27 AM, Alvaro Herrera wrote:
> Attributes: S -- superuser
> R -- create role
> D -- create database
> I -- inherit

Can we add something similar to the bottom of \z?
--
Decibel!, aka Jim C. Nasby, Database Architect decibel(at)decibel(dot)org
Give your computer some brain candy! www.distributed.net Team #1828

On Feb 14, 2008, at 5:50 PM, Tom Lane wrote:
> "Brendan Jurd" <direvus(at)gmail(dot)com> writes:
>> What about connection limit? I suppose we could combine it into the
>> privileges column, and refrain from displaying anything for "no
>> limit".
>
> "5 connections" seems a bit odd for a "privilege". I'd be happy with
> leaving that as a separate column. Or I guess we could choose a
> less specific word (like "attributes"?) for the column heading.

It would also be nice if \du showed things that had been SET for each
user/role (ALTER USER ... SET search_path=blah).

FWIW, I prefer the single-letter suggestion over writing everything out.
--
Decibel!, aka Jim C. Nasby, Database Architect decibel(at)decibel(dot)org
Give your computer some brain candy! www.distributed.net Team #1828

On Fri, Feb 15, 2008 at 12:19 PM, Decibel! <decibel(at)decibel(dot)org> wrote:
> On Feb 14, 2008, at 7:27 AM, Alvaro Herrera wrote:
> > Attributes: S -- superuser
> > R -- create role
> > D -- create database
> > I -- inherit
>
>
> Can we add something similar to the bottom of \z?

Yeah, that thought occurred to me when I read Alvaro's suggestion too.
Especially since the most common single-letter codes given by \z
don't match up to the privilege names (r = SELECT, w = UPDATE, etc.)

I've done up a patch per Tom's idea of combining the binary role
attributes into a single column.

Each attribute which differs from the default is listed on a separate
line, like so:

List of roles
Role name | Attributes | Member of
-------------+----------------+-------------------
bob | | {readers,writers}
brendanjurd | Superuser | {}
: Create role
: Create DB
harry | No inherit | {}
jim | 10 connections | {readers}
readers | No login | {}
writers | No login | {}
(6 rows)

Notes:

* The patch relies on array_to_string's current treatment of NULL
values in the array; they are ignored. If that behaviour changes in
the future, the \du output will become very ugly indeed.
* I'm not sure whether "No login" and "No inherit" are the best
phrases to use. I took my cue from the SQL setting names NOLOGIN and
NOINHERIT, but maybe something more grammatically sensible with
"Cannot login" and "No inheritance" would be preferable.
* If accepted, this patch would supercede the earlier patch mentioned
by Bernd Helmle upthread, which adds LOGIN to the output as a new
column: http://archives.postgresql.org/pgsql-patches/2007-11/msg00014.php

Cheers,
BJ

Brendan Jurd escribió:
> I've done up a patch per Tom's idea of combining the binary role
> attributes into a single column.

This patch seems to be missing from the queue.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera wrote:
> Brendan Jurd escribi?:
> > I've done up a patch per Tom's idea of combining the binary role
> > attributes into a single column.
>
> This patch seems to be missing from the queue.

Yes because I am still processing email from two weeks ago. I was in
Europe for a week.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------

Brendan Jurd wrote:
> I've done up a patch per Tom's idea of combining the binary role
> attributes into a single column.
>
> Each attribute which differs from the default is listed on a separate
> line, like so:
>
> List of roles
> Role name | Attributes | Member of
> -------------+----------------+-------------------
> bob | | {readers,writers}
> brendanjurd | Superuser | {}
> : Create role
> : Create DB
> harry | No inherit | {}
> jim | 10 connections | {readers}
> readers | No login | {}
> writers | No login | {}
> (6 rows)
>
> Notes:
>
> * The patch relies on array_to_string's current treatment of NULL
> values in the array; they are ignored. If that behaviour changes in
> the future, the \du output will become very ugly indeed.
> * I'm not sure whether "No login" and "No inherit" are the best
> phrases to use. I took my cue from the SQL setting names NOLOGIN and
> NOINHERIT, but maybe something more grammatically sensible with
> "Cannot login" and "No inheritance" would be preferable.
> * If accepted, this patch would supercede the earlier patch mentioned
> by Bernd Helmle upthread, which adds LOGIN to the output as a new
> column: http://archives.postgresql.org/pgsql-patches/2007-11/msg00014.php
>
> Cheers,
> BJ

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/docs/faq

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

"Brendan Jurd" <direvus(at)gmail(dot)com> writes:
> I've done up a patch per Tom's idea of combining the binary role
> attributes into a single column.

I started to look at committing this and realized that there's a very
nasty problem: our current approach to localizing the strings won't
work. See this patch for background:
http://archives.postgresql.org/pgsql-committers/2007-12/msg00187.php

The code is now set up so that it can pass an entire field value
through gettext(), but if gettext recognizes the strings "foo" and
"bar" that doesn't mean it will do anything useful with "foo\nbar",
which is what this patch would require.

I suspect that to solve this in a non-kluge fashion we'd need to make
\du pull over the plain boolean and integer values, then build a new
PGresult data structure on its own. Ugh. (Actually, without any
support from libpq for building PGresults, it's hard to imagine doing
that in a way that wouldn't be a kluge itself.)

Or we could go back to the drawing board on what the output ought to
look like.

Thoughts?

regards, tom lane

Brendan Jurd escribió:
> I've done up a patch per Tom's idea of combining the binary role
> attributes into a single column.

Thanks -- this is nice. I even went to apply it, but found a problem:
gettext localizes the NULL string to the localization header :-( For
example:

alvherre=# \du
Liste des rôles
Nom du rôle | Attributes | Membre de
-------------+-------------------------------------------------------------+-----------
alvherre | Superuser | {}
: Create role
: Create DB
asd | No inherit | {}
: No login
bar | Project-Id-Version: psql | {}
: Report-Msgid-Bugs-To:
: POT-Creation-Date: 2007-12-17 11:14-0400
: PO-Revision-Date: 2007-12-18 10:44+0100
: Last-Translator: Guillaume Lelarge <guillaume(at)lelarge(dot)info>
: Language-Team: <fr(at)li(dot)org>
: MIME-Version: 1.0
: Content-Type: text/plain; charset=ISO-8859-15
: Content-Transfer-Encoding: 8bit
: X-Generator: KBabel 1.11.4
:
foo | No login | {asd}
(4 lignes)

Starting psql with LC_ALL=C shows the truth:

alvherre=# \du
List of roles
Role name | Attributes | Member of
-----------+-------------+-----------
alvherre | Superuser | {}
: Create role
: Create DB
asd | No inherit | {}
: No login
bar | | {}
foo | No login | {asd}
(4 rows)

I'm not sure how to fix this. Thoughts?

--
Alvaro Herrera http://www.advogato.org/person/alvherre
Management by consensus: I have decided; you concede.
(Leonard Liu)

Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> writes:
> gettext localizes the NULL string to the localization header :-( For
> example:

Oooh, that's even different from the one I thought of :-(. Yeah,
we've got a problem here.

We could fix that particular issue by changing print.c so that it
doesn't attempt to localize a zero-length string; but that won't
help localizing multiple strings that have been concatenated.

regards, tom lane

On 21/03/2008, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> The code is now set up so that it can pass an entire field value
> through gettext(), but if gettext recognizes the strings "foo" and
> "bar" that doesn't mean it will do anything useful with "foo\nbar",
> which is what this patch would require.
>

Ouch!

> I suspect that to solve this in a non-kluge fashion we'd need to make
> \du pull over the plain boolean and integer values, then build a new
> PGresult data structure on its own. Ugh. (Actually, without any
> support from libpq for building PGresults, it's hard to imagine doing
> that in a way that wouldn't be a kluge itself.)
>
> Or we could go back to the drawing board on what the output ought to
> look like.
>

We can't just build the output table by hand like
describeOneTableDetails does? Admittedly it's kludgy, but it's not an
unprecedented kludge.

"Brendan Jurd" <direvus(at)gmail(dot)com> writes:
> On 21/03/2008, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> The code is now set up so that it can pass an entire field value
>> through gettext(), but if gettext recognizes the strings "foo" and
>> "bar" that doesn't mean it will do anything useful with "foo\nbar",
>> which is what this patch would require.

> Ouch!

> We can't just build the output table by hand like
> describeOneTableDetails does? Admittedly it's kludgy, but it's not an
> unprecedented kludge.

Oh, I had forgotten the existence of that entry point in print.c. Yeah,
it might be workable --- want to have a go at it?

regards, tom lane

On 21/03/2008, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Brendan Jurd" <direvus(at)gmail(dot)com> writes:
>
> > We can't just build the output table by hand like
> > describeOneTableDetails does? Admittedly it's kludgy, but it's not an
> > unprecedented kludge.
>
> Oh, I had forgotten the existence of that entry point in print.c. Yeah,
> it might be workable --- want to have a go at it?
>

Sure. It will be at least a few days (Easter holidays) before I can
submit anything.

On 21/03/2008, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Brendan Jurd" <direvus(at)gmail(dot)com> writes:
> > We can't just build the output table by hand like
> > describeOneTableDetails does? Admittedly it's kludgy, but it's not an
> > unprecedented kludge.
>
>
> Oh, I had forgotten the existence of that entry point in print.c. Yeah,
> it might be workable --- want to have a go at it?
>

I've had a chance to look at this now, and although it certainly does
seem workable, there's a lot of duplication of code that I feel uneasy
about. describeOneTableDetails essentially already duplicates the
table buildling code in printQuery, so I would be creating a third
copy of the same logic.

This makes me wonder whether print.c could offer something a bit more
helpful to callers wishing to DIY a table; we could have a
table-building struct with methods like addHeader and addCell.

What do you think? Overkill, or worthy pursuit?

"Brendan Jurd" <direvus(at)gmail(dot)com> writes:
> I've had a chance to look at this now, and although it certainly does
> seem workable, there's a lot of duplication of code that I feel uneasy
> about. describeOneTableDetails essentially already duplicates the
> table buildling code in printQuery, so I would be creating a third
> copy of the same logic.

> This makes me wonder whether print.c could offer something a bit more
> helpful to callers wishing to DIY a table; we could have a
> table-building struct with methods like addHeader and addCell.

> What do you think? Overkill, or worthy pursuit?

Once you have two occurrences of a pattern, it's reasonable to assume
there will be more later. +1 for building a little bit of infrastructure.

regards, tom lane

On Tue, Mar 25, 2008 at 2:41 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Brendan Jurd" <direvus(at)gmail(dot)com> writes:
> > This makes me wonder whether print.c could offer something a bit more
> > helpful to callers wishing to DIY a table; we could have a
> > table-building struct with methods like addHeader and addCell.
>
> > What do you think? Overkill, or worthy pursuit?
>
> Once you have two occurrences of a pattern, it's reasonable to assume
> there will be more later. +1 for building a little bit of infrastructure.
>

I've written a patch which implements the same \du behaviour as my
previous patch, but using the new printTable API I submitted in [1].

If the printTable API patch is rejected or substantially changed, we
will need to revisit this patch.

The new patch constructs a table manually, in the same manner as
describeOneTableDetails, so that we get the same outputs as the
original patch but without any of the localisation issues identified
by Tom and Alvaro.

I have attached a patch against my printTable code, containing only
the changes I made to describeRoles() (du-attributes_1.diff.bz2), and
a combined patch against HEAD containing the full printTable API
changes as well as the changes to describeRoles()
(du-attributes-print-table_1.diff.bz2).

No memory problems detected by valgrind, and all regression tests
passed on x86_64 gentoo.

I've added this item to the May CommitFest wiki page.

Cheers,
BJ

[1[ http://archives.postgresql.org/message-id/37ed240d0804120112w60c80089obd3a8c8ac72505f8@mail.gmail.com

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Sun, Apr 13, 2008 at 6:11 PM, Brendan Jurd wrote:
> I've written a patch which implements the same \du behaviour as my
> previous patch, but using the new printTable API

New versions of this patch, including changes made to the printTable API in [1]

As before, I've included a patch against HEAD which includes the
printTable changes (du-attributes-print-table_2.diff.bz2), and a patch
against my printTable branch which shows just the changes to
describeRoles (du-attributes_2.diff.bz2).

Cheers,
BJ

[1]
http://archives.postgresql.org/message-id/37ed240d0804150111l14f623bx6fd594dd516bf4a4@mail.gmail.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.7 (GNU/Linux)
Comment: http://getfiregpg.org

iD8DBQFIBGZP5YBsbHkuyV0RAsb9AKDrFJ/8V00t2XCwIihzEZYcPQZKiQCg3q6L
RkiMfjqLay/JLk8phnniYLs=
=oW6S
-----END PGP SIGNATURE-----