Re: Fix output of zero privileges in psql - Mailing list pgsql-hackers

On 2023-10-07 14:29 +0200, Laurenz Albe write:
> On Sat, 2023-10-07 at 05:07 +0200, Erik Wienhold wrote:
> > On 2023-10-06 22:32 +0200, Laurenz Albe write:
> > > On Sun, 2023-09-17 at 21:31 +0200, Erik Wienhold wrote:
> > > > I wrote a patch to change psql's display of zero privileges after a user's
> > > > reported confusion with the psql output for zero vs. default privileges [1].
> > > > Admittedly, zero privileges is a rare use case [2] but I think psql should not
> > > > confuse the user in the off chance that this happens.
> > > > 
> > > > [1] https://www.postgresql.org/message-id/efdd465d-a795-6188-7f71-7cdb4b2be031%40mtneva.com
> > > > [2] https://www.postgresql.org/message-id/31246.1693337238%40sss.pgh.pa.us
> > > 
> > > Reading that thread, I had the impression that there was more support for
> > > honoring "\pset null" rather than unconditionally displaying "(none)".
> > 
> > For example with your patch applied:
> > 
> >         create table t1 (a int);
> >         create table t2 (a int);
> >         create table t3 (a int);
> > 
> >         revoke all on t2 from :USER;
> > 
> >         \pset null <NULL>
> >         \dp t1|t2|t3
> >                                     Access privileges
> >          Schema | Name | Type  | Access privileges | Column privileges | Policies
> >         --------+------+-------+-------------------+-------------------+----------
> >          public | t1   | table | <NULL>            |                   |
> >          public | t2   | table |                   |                   |
> >          public | t3   | table | <NULL>            |                   |
> >         (3 rows)
> > 
> > Instead of only displaying the zero privileges with my patch and default
> > \pset null:
> > 
> >         \pset null ''
> >         \dp t1|t2|t3
> >                                     Access privileges
> >          Schema | Name | Type  | Access privileges | Column privileges | Policies
> >         --------+------+-------+-------------------+-------------------+----------
> >          public | t1   | table |                   |                   |
> >          public | t2   | table | (none)            |                   |
> >          public | t3   | table |                   |                   |
> >         (3 rows)
> > 
> > I guess if most tables have any non-default privileges then both
> > solutions are equally good.
> 
> It is a tough call.
> 
> For somebody who knows PostgreSQL well enough to know that default
> privileges are represented by NULL values, my solution is probably
> more appealing.
> 
> It seems that we both had the goal of distinguishing the cases of
> default and zero privileges, but for a beginner, both versions are
> confusing. better would probably be
> 
>                              Access privileges
>   Schema | Name | Type  | Access privileges | Column privileges | Policies
>  --------+------+-------+-------------------+-------------------+----------
>   public | t1   | table | default           | default           |
>   public | t2   | table |                   | default           |
>   public | t3   | table | default           | default           |

Ah yes.  The problem seems to be more with default privileges producing
no output right now.  I was just focusing on the zero privs edge case.

> The disadvantage of this (and the advantage of my proposal) is that it
> might confuse experienced users (and perhaps automated tools) if the
> output changes too much.

I agree that your patch is less invasive under default settings.  But is
the output of meta commands considered part of the interface where we
need to be cautious about not breaking clients?

I've written quite a few scripts that parse results from psql's stdout,
but always with simple queries to have control over columns and the
formatting of values.  I always expect meta command output to change
with the next release because to me they look more like a human-readable
interface, e.g. the localizable header which of course one can still
hide with --tuples-only.

> > > The simple attached patch does it like that.  What do you think?
> > 
> > LGTM.
> 
> If you are happy enough with my patch, shall we mark it as ready for
> committer?

I amended your patch to also document the effect of \pset null in this
case.  See the attached v2.

> Or do you want to have a stab at something like I suggested above?

Not right now if the user can just use \pset null 'default' with your
patch.

-- 
Erik