Re: Fix output of zero privileges in psql

From: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, Erik Wienhold <ewie(at)mailbox(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Fix output of zero privileges in psql
Date: 2023-10-20 20:35:06
Message-ID: CAKFQuwaMHa2kLGXbm9B=OAbCbKOqP38wH0xCK=WJqa0AS31Spg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Oct 20, 2023 at 12:57 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> "David G. Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com> writes:
> > On Fri, Oct 20, 2023 at 12:34 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> As near as I can tell, doing both things (the \pset null fix and
> >> substituting "(none)" for empty privileges) would be an acceptable
> >> answer to everyone who has commented. Let's proceed with both
> >> patches, or combine them into one if there are merge conflicts.
>
> > I'm under the impression that removing the null representation of empty
> > privileges by making them (none) removes all known \d commands that
> output
> > nulls and don't obey \pset null.
>
> How so? IIUC the proposal is to substitute "(none)" for empty-string
> ACLs, not null ACLs. The \pset change should be addressing an
> independent case.
>

Ok, I found my mis-understanding and better understand where you are all
coming from now; I apparently had the usage of NULL flip-flopped.

Taking pg_tablespace as an example. Its "spcacl" column produces NULL for
default privileges and '{}'::text[] for empty privileges.

Thus, today:
empty: array_to_string('{}'::text[], '\n') produces an empty string
default: array_to_string(null, '\n') produces null which then was printed
as a hard-coded empty string via forcibly changing \pset null

I was thinking the two cases were reversed.

My proposal for changing printACLColumn is thus:

case when spcacl is null then ''
when cardinality(spcacl) = 0 then '(none)'
else array_to_string(spcacl, E'\\n')
end as "Access privileges"

In short, I don't want default privileges to start to obey \pset null when
it never has before and is documented as displaying the empty string. I do
want the empty string produced by empty privileges to change to (none) so
that it no longer is indistinguishable from our choice of presentation for
the default privilege case.

Mechanically, we remove the existing \pset null for these metacommands and
move it into the query via the added CASE expression in the ‎printACLColumn
function.

This gets rid of NULL as an output value for this column and makes the
patch regarding \pset null discussion unnecessary. And it leaves the
existing well-established empty string for default privileges alone (and
changing this is what I believe Tom is against and I agree on that point).

David J.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2023-10-20 21:33:23 Re: Remove last traces of HPPA support
Previous Message Tom Lane 2023-10-20 19:59:42 Re: Remove last traces of HPPA support