pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
pgsql: Improve default and empty privilege outputs in psql.
4+ messages / 2 participants
[nested] [flat]

Commits from this thread:
  2024-09-24 1591b38d17 923a71584f  Fix psql describe commands' handling of ACL columns for old servers.
* pgsql: Improve default and empty privilege outputs in psql.
@ 2023-11-13 20:41  Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 4+ messages in thread

From: Tom Lane @ 2023-11-13 20:41 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org

Improve default and empty privilege outputs in psql.

Default privileges are represented as NULL::aclitem[] in catalog ACL
columns, while revoking all privileges leaves an empty aclitem[].
These two cases used to produce identical output in psql meta-commands
like \dp.  Using something like "\pset null '(default)'" as a
workaround for spotting the difference did not work, because null
values were always displayed as empty strings by describe.c's
meta-commands.

This patch improves that with two changes:

1. Print "(none)" for empty privileges so that the user is able to
   distinguish them from default privileges, even without special
   workarounds.

2. Remove the special handling of null values in describe.c,
   so that "\pset null" is honored like everywhere else.
   (This affects all output from these commands, not only ACLs.)

The privileges shown by \dconfig+ and \ddp as well as the column
privileges shown by \dp are not affected by change #1, because the
respective aclitem[] is reset to NULL or deleted from the catalog
instead of leaving an empty array.

Erik Wienhold and Laurenz Albe

Discussion: https://postgr.es/m/1966228777.127452.1694979110595@office.mailbox.org

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/d1379ebf4c2d3d399e739966dbfa34e92a53b727

Modified Files
--------------
doc/src/sgml/ddl.sgml              | 16 ++++++++++-
src/bin/psql/describe.c            | 57 +++++++++-----------------------------
src/test/regress/expected/psql.out | 55 ++++++++++++++++++++++++++++++++++++
src/test/regress/sql/psql.sql      | 32 +++++++++++++++++++++
4 files changed, 115 insertions(+), 45 deletions(-)



^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: pgsql: Improve default and empty privilege outputs in psql.
@ 2024-09-24 15:14  Christoph Berg <myon@debian.org>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 4+ messages in thread

From: Christoph Berg @ 2024-09-24 15:14 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers@lists.postgresql.org

Re: Tom Lane
> Improve default and empty privilege outputs in psql.

I'm sorry to report this when 17.0 has already been wrapped, but this
change is breaking `psql -l` against 9.3-or-earlier servers:

$ /usr/lib/postgresql/17/bin/psql
psql (17rc1 (Debian 17~rc1-1.pgdg+2), Server 9.3.25)
Geben Sie »help« für Hilfe ein.

postgres =# \set ECHO_HIDDEN on
postgres =# \l
/******* ANFRAGE ********/
SELECT
  d.datname as "Name",
  pg_catalog.pg_get_userbyid(d.datdba) as "Owner",
  pg_catalog.pg_encoding_to_char(d.encoding) as "Encoding",
  'libc' AS "Locale Provider",
  d.datcollate as "Collate",
  d.datctype as "Ctype",
  NULL as "Locale",
  NULL as "ICU Rules",
  CASE WHEN pg_catalog.cardinality(d.datacl) = 0 THEN '(none)' ELSE pg_catalog.array_to_string(d.datacl, E'\n') END AS "Access privileges"
FROM pg_catalog.pg_database d
ORDER BY 1;
/************************/

FEHLER:  42883: Funktion pg_catalog.cardinality(aclitem[]) existiert nicht
ZEILE 10:   CASE WHEN pg_catalog.cardinality(d.datacl) = 0 THEN '(none...
                      ^
TIP:  Keine Funktion stimmt mit dem angegebenen Namen und den Argumenttypen überein. Sie müssen möglicherweise ausdrückliche Typumwandlungen hinzufügen.
ORT:  ParseFuncOrColumn, parse_func.c:299


The psql docs aren't really explicit about which old versions are
still supported, but there's some mentioning that \d should work back
to 9.2:

      <para><application>psql</application> works best with servers of the same
       or an older major version.  Backslash commands are particularly likely
       to fail if the server is of a newer version than <application>psql</application>
       itself.  However, backslash commands of the <literal>\d</literal> family should
       work with servers of versions back to 9.2, though not necessarily with
       servers newer than <application>psql</application> itself.

\l seems a tad more important even.

Christoph





^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: pgsql: Improve default and empty privilege outputs in psql.
@ 2024-09-24 16:27  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Christoph Berg <myon@debian.org>
  0 siblings, 1 reply; 4+ messages in thread

From: Tom Lane @ 2024-09-24 16:27 UTC (permalink / raw)
  To: Christoph Berg <myon@debian.org>; +Cc: pgsql-hackers@lists.postgresql.org

Christoph Berg <myon@debian.org> writes:
> Re: Tom Lane
>> Improve default and empty privilege outputs in psql.

> I'm sorry to report this when 17.0 has already been wrapped, but this
> change is breaking `psql -l` against 9.3-or-earlier servers:
> FEHLER:  42883: Funktion pg_catalog.cardinality(aclitem[]) existiert nicht

Grumble.  Well, if that's the worst bug in 17.0 we should all be
very pleased indeed ;-).  I'll see about fixing it after the
release freeze lifts.

> The psql docs aren't really explicit about which old versions are
> still supported, but there's some mentioning that \d should work back
> to 9.2:

Yes, that's the expectation.  I'm sure we can think of a more
backwards-compatible way to test for empty datacl, though.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: pgsql: Improve default and empty privilege outputs in psql.
@ 2024-09-24 17:57  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 4+ messages in thread

From: Tom Lane @ 2024-09-24 17:57 UTC (permalink / raw)
  To: Christoph Berg <myon@debian.org>; +Cc: pgsql-hackers@lists.postgresql.org

I wrote:
> Yes, that's the expectation.  I'm sure we can think of a more
> backwards-compatible way to test for empty datacl, though.

Looks like the attached should be sufficient.  As penance, I tried
all the commands in describe.c against 9.2 (or the oldest supported
server version), and none of them fail now.

			regards, tom lane

Attachments:

fix-ACL-printing-for-old-servers.patch (text/x-diff, 516B) download diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index faabecbc76..6a36c91083 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -6678,7 +6678,7 @@ printACLColumn(PQExpBuffer buf, const char *colname) { appendPQExpBuffer(buf, "CASE" - " WHEN pg_catalog.cardinality(%s) = 0 THEN '%s'" + " WHEN pg_catalog.array_length(%s, 1) = 0 THEN '%s'" " ELSE pg_catalog.array_to_string(%s, E'\\n')" " END AS \"%s\"", colname, gettext_noop("(none)"),
^ permalink raw reply [nested|flat] 4+ messages in thread


end of thread, other threads:[~2024-09-24 17:57 UTC | newest]

Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-11-13 20:41 pgsql: Improve default and empty privilege outputs in psql. Tom Lane <tgl@sss.pgh.pa.us>
2024-09-24 15:14 ` Christoph Berg <myon@debian.org>
2024-09-24 16:27   ` Tom Lane <tgl@sss.pgh.pa.us>
2024-09-24 17:57     ` Tom Lane <tgl@sss.pgh.pa.us>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox