Fix output of zero privileges in psql

On 17/09/2023 21:31 CEST Erik Wienhold <ewie@ewie.name> wrote:

This affects the following meta commands:

\db+ \dD+ \df+ \dL+ \dl+ \dn+ \dp \dT+ \l

also \des+ and \dew+

--
Erik

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.

With this change psql now prints "(none)" for zero privileges instead of
nothing.  This affects the following meta commands:

    \db+ \dD+ \df+ \dL+ \dl+ \dn+ \dp \dT+ \l

Default privileges start as NULL::aclitem[] in various catalog columns but
revoking the default privileges leaves an empty aclitem array.  Using
\pset null '(null)' as a workaround to spot default privileges does not work
because the meta commands ignore this setting.

The privileges shown by \dconfig+ and \ddp as well as the column privileges
shown by \dp are not affected by this change because those privileges are reset
to NULL instead of leaving empty arrays.

Commands \des+ and \dew+ are not covered in src/test/regress because no foreign
data wrapper is available at this point to create a foreign server.

[1] /messages/by-id/efdd465d-a795-6188-7f71-7cdb4b2be031@mtneva.com
[2] /messages/by-id/31246.1693337238@sss.pgh.pa.us

Reading that thread, I had the impression that there was more support for
honoring "\pset null" rather than unconditionally displaying "(none)".

The simple attached patch does it like that. What do you think?

Yours,
Laurenz Albe

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.

With this change psql now prints "(none)" for zero privileges instead of
nothing.  This affects the following meta commands:

    \db+ \dD+ \df+ \dL+ \dl+ \dn+ \dp \dT+ \l

Default privileges start as NULL::aclitem[] in various catalog columns but
revoking the default privileges leaves an empty aclitem array.  Using
\pset null '(null)' as a workaround to spot default privileges does not work
because the meta commands ignore this setting.

The privileges shown by \dconfig+ and \ddp as well as the column privileges
shown by \dp are not affected by this change because those privileges are reset
to NULL instead of leaving empty arrays.

Commands \des+ and \dew+ are not covered in src/test/regress because no foreign
data wrapper is available at this point to create a foreign server.

[1] /messages/by-id/efdd465d-a795-6188-7f71-7cdb4b2be031@mtneva.com
[2] /messages/by-id/31246.1693337238@sss.pgh.pa.us

Reading that thread, I had the impression that there was more support for
honoring "\pset null" rather than unconditionally displaying "(none)".

I took Tom's response in the -general thread to mean that we could fix
\pset null also as a "nice to have" but not as a solution to the display
of zero privileges.

Only fixing \pset null has one drawback IMO because it only affects how
default privileges (more common) are printed. The edge case of zero
privileges (less common) gets lost in a bunch of NULL output. And I
assume most users change the default \pset null to some non-empty string
in their psqlrc (I do).

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.

The simple attached patch does it like that. What do you think?

LGTM.

--
Erik

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] /messages/by-id/efdd465d-a795-6188-7f71-7cdb4b2be031@mtneva.com
[2] /messages/by-id/31246.1693337238@sss.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 |

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.

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?
Or do you want to have a stab at something like I suggested above?

Yours,
Laurenz Albe

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] /messages/by-id/efdd465d-a795-6188-7f71-7cdb4b2be031@mtneva.com
[2] /messages/by-id/31246.1693337238@sss.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

On Sat, 2023-10-07 at 20:41 +0200, Erik Wienhold wrote:

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.

+1

If you mention in ddl.sgml that you can use "\pset null" to distinguish
default from no privileges, you should mention that this only works with
psql. Many people out there don't use psql.

Also, I'm not sure if "zero privileges" will be readily understood by
everybody. Perhaps "no privileges at all, even for the object owner"
would be a better wording.

Perhaps it would also be good to mention this in the psql documentation.

Yours,
Laurenz Albe

On 2023-10-08 06:14 +0200, Laurenz Albe write:

On Sat, 2023-10-07 at 20:41 +0200, Erik Wienhold wrote:

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.

+1

If you mention in ddl.sgml that you can use "\pset null" to distinguish
default from no privileges, you should mention that this only works with
psql. Many people out there don't use psql.

I don't think this is necessary because that section in ddl.sgml is
already about psql and \dp.

Also, I'm not sure if "zero privileges" will be readily understood by
everybody. Perhaps "no privileges at all, even for the object owner"
would be a better wording.

Changed in v3 to "empty privileges" with an explanation that this means
"no privileges at all, even for the object owner".

Perhaps it would also be good to mention this in the psql documentation.

Just once under \pset null with a reference to section 5.7? Something
like "This is also useful for distinguishing default privileges from
empty privileges as explained in Section 5.7."

Or instead under every command affected by this change? \dp and \ddp
already have such a reference ("The meaning of the privilege display is
explained in Section 5.7.")

I prefer the first one because it's less effort ;) Also done in v3.

--
Erik

On Sun, Oct 8, 2023 at 6:55 PM Erik Wienhold <ewie@ewie.name> wrote:

On 2023-10-08 06:14 +0200, Laurenz Albe write:

On Sat, 2023-10-07 at 20:41 +0200, Erik Wienhold wrote:

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.

+1

If you mention in ddl.sgml that you can use "\pset null" to distinguish
default from no privileges, you should mention that this only works with
psql. Many people out there don't use psql.

I don't think this is necessary because that section in ddl.sgml is
already about psql and \dp.

I agree that we are simply detailing how psql makes this information
available to the reader and leave users of other clients on their own to
figure out their own methods - which many clients probably have handled for
them anyway.

For us, I would suggest the following wording:

In addition to the situation of printing all acl items, the Access and
Column privileges columns report two other situations specially. In the
rare case where all privileges for an object have been explicitly removed,
including from the owner and PUBLIC, (i.e., has empty privileges) these
columns will display NULL. The other case is where the built-in default
privileges are in effect, in which case these columns will display the
empty string. (Note that by default psql will print NULL as an empty
string, so in order to visually distinguish these two cases you will need
to issue the \pset null meta-command and choose some other string to print
for NULLs). Built-in default privileges include all privileges for the
owner, as well as those granted to PUBLIC per for relevant object types as
described above. The built-in default privileges are only in effect if the
object has not been the target of a GRANT or REVOKE and also has not had
its default privileges modified using ALTER DEFAULT PRIVILEGES. (???: if it
is possible to revert back to the state of built-in privileges that would
need to be described here.)

The above removes the parenthetical regarding null in the catalogs, this is
intentional as it seems that the goal here is to use psql instead of the
catalogs and adding its use of null being printed as the empty string just
seems likely to add confusion.

Also, I'm not sure if "zero privileges" will be readily understood by
everybody. Perhaps "no privileges at all, even for the object owner"
would be a better wording.

Changed in v3 to "empty privileges" with an explanation that this means
"no privileges at all, even for the object owner".

+1

We probably should add the two terms to the glossary:

empty privileges: all privileges explicitly revoked from the owner and
PUBLIC (where applicable), and none otherwise granted.

built-in default privileges: owner having all privileges and no privileges
granted or removed via ALTER DEFAULT PRIVILEGES

Perhaps it would also be good to mention this in the psql documentation.

Just once under \pset null with a reference to section 5.7? Something
like "This is also useful for distinguishing default privileges from
empty privileges as explained in Section 5.7."

Or instead under every command affected by this change? \dp and \ddp
already have such a reference ("The meaning of the privilege display is
explained in Section 5.7.")

I prefer the first one because it's less effort ;) Also done in v3.

We've chosen a poor default and I'd rather inform the user of specific
meta-commands to be wary of this poor default and change it at the point
they will be learning about the meta-commands adversely affected.

That said, I'd be willing to document that these commands, because they are
affected by empty string versus null, require a non-empty-string value for
\pset null and will choose the string '<null>' for the duration of the
meta-command's execution if the user's setting is incompatible.

David J.

On Mon, 2023-10-09 at 03:53 +0200, Erik Wienhold wrote:

On 2023-10-08 06:14 +0200, Laurenz Albe write:

On Sat, 2023-10-07 at 20:41 +0200, Erik Wienhold wrote:

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.

+1

If you mention in ddl.sgml that you can use "\pset null" to distinguish
default from no privileges, you should mention that this only works with
psql.  Many people out there don't use psql.

I don't think this is necessary because that section in ddl.sgml is
already about psql and \dp.

You are right.

Also, I'm not sure if "zero privileges" will be readily understood by
everybody.  Perhaps "no privileges at all, even for the object owner"
would be a better wording.

Changed in v3 to "empty privileges" with an explanation that this means
"no privileges at all, even for the object owner".

Looks good.

Perhaps it would also be good to mention this in the psql documentation.

Just once under  \pset null  with a reference to section 5.7?  Something
like "This is also useful for distinguishing default privileges from
empty privileges as explained in Section 5.7."

Or instead under every command affected by this change?  \dp and \ddp
already have such a reference ("The meaning of the privilege display is
explained in Section 5.7.")

I prefer the first one because it's less effort ;)  Also done in v3.

I think that sufficient.

I tinkered a bit with your documentation. For example, the suggestion to
"\pset null" seemed to be in an inappropriate place. Tell me what you think.

Yours,
Laurenz Albe

On Sun, 2023-10-08 at 19:58 -0700, David G. Johnston wrote:

For us, I would suggest the following wording:

In addition to the situation of printing all acl items, the Access and Column
privileges columns report two other situations specially.  In the rare case
where all privileges for an object have been explicitly removed, including
from the owner and PUBLIC, (i.e., has empty privileges) these columns will
display NULL.  The other case is where the built-in default privileges are
in effect, in which case these columns will display the empty string.
(Note that by default psql will print NULL as an empty string, so in order
to visually distinguish these two cases you will need to issue the \pset null
meta-command and choose some other string to print for NULLs).  Built-in
default privileges include all privileges for the owner, as well as those
granted to PUBLIC per for relevant object types as described above.

That doesn't look like an improvement over the latest patches to me.

The built-in default privileges are only in effect if the object has not been
the target of a GRANT or REVOKE and also has not had its default privileges
modified using ALTER DEFAULT PRIVILEGES. (???: if it is possible to revert
back to the state of built-in privileges that would need to be described here.)

I don't think that we need to mention ALTER DEFAULT PRIVILEGES there. If
the default privileges have been altered, the ACL will not be stored as
NULL in the catalogs.

The above removes the parenthetical regarding null in the catalogs, this is
intentional as it seems that the goal here is to use psql instead of the
catalogs and adding its use of null being printed as the empty string just
seems likely to add confusion.

To me, mentioning the default privileges are stored as NULLs in the catalogs
is not an invitation to view the privileges with catalog queries, but
information about implementation details that explains why default privileges
are displayed the way they are.

We probably should add the two terms to the glossary:

empty privileges: all privileges explicitly revoked from the owner and PUBLIC
(where applicable), and none otherwise granted.

built-in default privileges: owner having all privileges and no privileges
granted or removed via ALTER DEFAULT PRIVILEGES

"Empty privileges" are too unimportant to warrant an index entry.

I can see the value of an index entry

<indexterm>
<primary>privilege</primary>
<secondary>default</secondary>
</indexterm>

Done in the attached v5 of the patch, even though this is not really
the business of this patch.

Perhaps it would also be good to mention this in the psql documentation.

We've chosen a poor default and I'd rather inform the user of specific meta-commands
to be wary of this poor default and change it at the point they will be learning
about the meta-commands adversely affected.

That said, I'd be willing to document that these commands, because they are affected
by empty string versus null, require a non-empty-string value for \pset null and will
choose the string '<null>' for the duration of the meta-command's execution if the
user's setting is incompatible.

I am not certain I understood you correctly.
Are you advocating for adding a mention of "\pset null" to every backslash command
that displays privileges? That is excessive, in my opinion.

Yours,
Laurenz Albe

On Mon, Oct 9, 2023 at 1:29 AM Laurenz Albe <laurenz.albe@cybertec.at>
wrote:

On Sun, 2023-10-08 at 19:58 -0700, David G. Johnston wrote:

The built-in default privileges are only in effect if the object has not

been

the target of a GRANT or REVOKE and also has not had its default

privileges

modified using ALTER DEFAULT PRIVILEGES. (???: if it is possible to

revert

back to the state of built-in privileges that would need to be described

here.)

I don't think that we need to mention ALTER DEFAULT PRIVILEGES there. If
the default privileges have been altered, the ACL will not be stored as
NULL in the catalogs.

It's already mentioned there, I just felt bringing the mention of ADP and
grant/revoke both invalidating the built-in default privileges closer
together would be an improvement.

The above removes the parenthetical regarding null in the catalogs, this

is

intentional as it seems that the goal here is to use psql instead of the
catalogs and adding its use of null being printed as the empty string

just

seems likely to add confusion.

To me, mentioning the default privileges are stored as NULLs in the
catalogs
is not an invitation to view the privileges with catalog queries, but
information about implementation details that explains why default
privileges
are displayed the way they are.

Calling it an implementation detail leads me to conclude the point does not
belong in the user-facing administration documentation.

Perhaps it would also be good to mention this in the psql

documentation.

We've chosen a poor default and I'd rather inform the user of specific

meta-commands

to be wary of this poor default and change it at the point they will be

learning

about the meta-commands adversely affected.

That said, I'd be willing to document that these commands, because they

are affected

by empty string versus null, require a non-empty-string value for \pset

null and will

choose the string '<null>' for the duration of the meta-command's

execution if the

user's setting is incompatible.

I am not certain I understood you correctly.
Are you advocating for adding a mention of "\pset null" to every backslash
command
that displays privileges? That is excessive, in my opinion.

Yes, I am. I suppose the argument that any user of those commands is
expected to have read the ddl/privileges chapter would suffice for me
though.

My point with the second paragraph is that we could, instead of documenting
the caveat about null printing as empty strings is to instead issue an
implicit "\pset null '<null>'" as part of these commands, and a "\pset
null" afterward, conditioned upon it not already being set to a non-empty
value. IOW, the special-casing we do today but actually do it in a way
that distinguishes the two cases instead of forcing them to be
indistinguishable.

David J.

On Mon, 2023-10-09 at 09:30 -0700, David G. Johnston wrote:

On Mon, Oct 9, 2023 at 1:29 AM Laurenz Albe <laurenz.albe@cybertec.at> wrote:

On Sun, 2023-10-08 at 19:58 -0700, David G. Johnston wrote:

The built-in default privileges are only in effect if the object has not been
the target of a GRANT or REVOKE and also has not had its default privileges
modified using ALTER DEFAULT PRIVILEGES. (???: if it is possible to revert
back to the state of built-in privileges that would need to be described here.)

I don't think that we need to mention ALTER DEFAULT PRIVILEGES there.  If
the default privileges have been altered, the ACL will not be stored as
NULL in the catalogs.

It's already mentioned there, I just felt bringing the mention of ADP and
grant/revoke both invalidating the built-in default privileges closer together
would be an improvement.

Ah, sorry, I misread your comment. You are right. But then, the effects of
ALTER DEFAULT PRIVILEGES are discussed later in the paragraph anyway, and we don't
have to repeat that here.

To me, mentioning the default privileges are stored as NULLs in the catalogs
is not an invitation to view the privileges with catalog queries, but
information about implementation details that explains why default privileges
are displayed the way they are.

Calling it an implementation detail leads me to conclude the point does not
belong in the user-facing administration documentation.

Again, you have a point there. However, I find that detail useful, as it explains
the user-facing behavior. Anyway, I don't think it is the job of this patch to
remove that pre-existing detail.

Are you advocating for adding a mention of "\pset null" to every backslash command
that displays privileges?  That is excessive, in my opinion.

Yes, I am.  I suppose the argument that any user of those commands is expected
to have read the ddl/privileges chapter would suffice for me though.

Thanks. Then let's leave it like that.

My point with the second paragraph is that we could, instead of documenting the
caveat about null printing as empty strings is to instead issue an implicit
"\pset null '<null>'" as part of these commands, and a "\pset null" afterward,
conditioned upon it not already being set to a non-empty value.  IOW, the
special-casing we do today but actually do it in a way that distinguishes the
two cases instead of forcing them to be indistinguishable.

-1

The whole point of this patch is to make psql behave consistently with respect to
NULLs in meta-commands. Your suggestion would subvert that idea.

Yours,
Laurenz Albe

Laurenz Albe <laurenz.albe@cybertec.at> writes:

On Mon, 2023-10-09 at 09:30 -0700, David G. Johnston wrote:

My point with the second paragraph is that we could, instead of documenting the
caveat about null printing as empty strings is to instead issue an implicit
"\pset null '<null>'" as part of these commands, and a "\pset null" afterward,
conditioned upon it not already being set to a non-empty value.  IOW, the
special-casing we do today but actually do it in a way that distinguishes the
two cases instead of forcing them to be indistinguishable.

-1

The whole point of this patch is to make psql behave consistently with respect to
NULLs in meta-commands. Your suggestion would subvert that idea.

Yeah. There is a lot of attraction in having \pset null affect these
displays just like all other ones. The fact that they act differently
now is a wart, not something we should replace with a different special
case behavior.

Also, I'm fairly concerned about not changing the default behavior here.
The fact that this behavior has stood for a couple dozen years without
many complaints indicates that there's not all that big a problem to be
solved. I doubt that a new default behavior will be well received,
even if it's arguably better.

regards, tom lane

On Mon, Oct 9, 2023 at 12:13 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Laurenz Albe <laurenz.albe@cybertec.at> writes:

On Mon, 2023-10-09 at 09:30 -0700, David G. Johnston wrote:

My point with the second paragraph is that we could, instead of

documenting the

caveat about null printing as empty strings is to instead issue an

implicit

"\pset null '<null>'" as part of these commands, and a "\pset null"

afterward,

conditioned upon it not already being set to a non-empty value. IOW,

the

special-casing we do today but actually do it in a way that

distinguishes the

two cases instead of forcing them to be indistinguishable.

-1

The whole point of this patch is to make psql behave consistently with

respect to

NULLs in meta-commands. Your suggestion would subvert that idea.

Yeah. There is a lot of attraction in having \pset null affect these
displays just like all other ones. The fact that they act differently
now is a wart, not something we should replace with a different special
case behavior.

Also, I'm fairly concerned about not changing the default behavior here.
The fact that this behavior has stood for a couple dozen years without
many complaints indicates that there's not all that big a problem to be
solved. I doubt that a new default behavior will be well received,
even if it's arguably better.

My position is that the default behavior should be changed such that the
distinction these reports need to make between empty privileges and default
privileges is impossible for the user to remove. I suppose the best direct
solution given that goal is for the query to simply do away with any
reliance on NULL being an indicator. Output an i18n'd "no permissions
present" line in the rare empty permissions situation and leave the empty
string for built-in default.

If the consensus is to not actually fix these views to make them
environment invariant in their accuracy then so be it. Having them obey
\pset null seems like a half-measure but it at least is an improvement over
the status quo such that users are capable of altering their system to make
the reports distinguish the two cases if they realize the need.

I do agree that my suggestion regarding the implicit \pset null, while
plausible, leaves the wart that NULL is being used to mean something
specific. Better is to just use a label for that specific thing.

David J.

On Mon, 2023-10-09 at 15:13 -0400, Tom Lane wrote:

Laurenz Albe <laurenz.albe@cybertec.at> writes:

The whole point of this patch is to make psql behave consistently with respect to
NULLs in meta-commands.

Yeah.  There is a lot of attraction in having \pset null affect these
displays just like all other ones.  The fact that they act differently
now is a wart, not something we should replace with a different special
case behavior.

Also, I'm fairly concerned about not changing the default behavior here.
The fact that this behavior has stood for a couple dozen years without
many complaints indicates that there's not all that big a problem to be
solved.  I doubt that a new default behavior will be well received,
even if it's arguably better.

I understand your concern. People who have "\pset null" in their .psqlrc
might be surprised if suddenly "<null>" starts appearing in the outout
of \l.

But then the people who have "\pset null" in their .psqlrc are typically
already somewhat experienced and might have less trouble dealing with that
change (but they are more likely to bleat on the mailing list, granted).

Users with little experience won't notice any difference, so they won't
be confused by the change.

Yours,
Laurenz Albe

On 2023-10-09 09:54 +0200, Laurenz Albe write:

I tinkered a bit with your documentation. For example, the suggestion to
"\pset null" seemed to be in an inappropriate place. Tell me what you think.

+1 You're right to put that sentence right after the explanation of
empty privileges.

--
Erik

On 2023-10-09 10:29 +0200, Laurenz Albe write:

On Sun, 2023-10-08 at 19:58 -0700, David G. Johnston wrote:

We probably should add the two terms to the glossary:

empty privileges: all privileges explicitly revoked from the owner and PUBLIC
(where applicable), and none otherwise granted.

built-in default privileges: owner having all privileges and no privileges
granted or removed via ALTER DEFAULT PRIVILEGES

"Empty privileges" are too unimportant to warrant an index entry.

I can see the value of an index entry

<indexterm>
<primary>privilege</primary>
<secondary>default</secondary>
</indexterm>

Done in the attached v5 of the patch, even though this is not really
the business of this patch.

+1

--
Erik

On 2023-10-09 22:34 +0200, David G. Johnston write:

On Mon, Oct 9, 2023 at 12:13 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Yeah. There is a lot of attraction in having \pset null affect these
displays just like all other ones. The fact that they act differently
now is a wart, not something we should replace with a different special
case behavior.

Also, I'm fairly concerned about not changing the default behavior here.
The fact that this behavior has stood for a couple dozen years without
many complaints indicates that there's not all that big a problem to be
solved. I doubt that a new default behavior will be well received,
even if it's arguably better.

My position is that the default behavior should be changed such that the
distinction these reports need to make between empty privileges and default
privileges is impossible for the user to remove. I suppose the best direct
solution given that goal is for the query to simply do away with any
reliance on NULL being an indicator. Output an i18n'd "no permissions
present" line in the rare empty permissions situation and leave the empty
string for built-in default.

My initial patch does exactly that.

If the consensus is to not actually fix these views to make them
environment invariant in their accuracy then so be it. Having them obey
\pset null seems like a half-measure but it at least is an improvement over
the status quo such that users are capable of altering their system to make
the reports distinguish the two cases if they realize the need.

I agree.

--
Erik

On Sat, 2023-10-14 at 02:45 +0200, Erik Wienhold wrote:

On 2023-10-09 09:54 +0200, Laurenz Albe write:

I tinkered a bit with your documentation.  For example, the suggestion to
"\pset null" seemed to be in an inappropriate place.  Tell me what you think.

+1  You're right to put that sentence right after the explanation of
empty privileges.

Thanks for looking.

David, how do you feel about this? I am wondering whether to set this patch
"ready for committer" or not.

There is Tom wondering upthread whether changing psql's behavior like that
is too much of a compatibility break or not, but I guess it is alright to
leave that final verdict to a committer.

Yours,
Laurenz Albe