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
From 2afe3cbf674e163c146ea29582f7aa3839bd184d Mon Sep 17 00:00:00 2001 From: Laurenz Albe <laurenz.a...@cybertec.at> Date: Mon, 9 Oct 2023 10:27:58 +0200 Subject: [PATCH] psql: honor "\pset null" in backslash commands In the output of backslash commands, NULL was always displayed as empty string, rather than honoring the values set with "\pset null". This was surprising, and in some cases it was downright confusing: for example, default privileges (stored as NULL) were displayed just like an empty aclitem[], making these cases undistinguishable in the output. Add this missing detail to the documentation. In passing, add entry for "privileges, default" to the index. Discussion: https://postgr.es/m/96d6885a-5e25-9ae8-4a1a-d7e557a5fe9c%40mtneva.com --- doc/src/sgml/ddl.sgml | 14 ++++++++--- doc/src/sgml/ref/psql-ref.sgml | 4 +++- src/bin/psql/describe.c | 43 ---------------------------------- 3 files changed, 14 insertions(+), 47 deletions(-) diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 075ff32991..76e62250e4 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -1737,6 +1737,11 @@ ALTER TABLE products RENAME TO items; <primary>ACL</primary> </indexterm> + <indexterm zone="ddl-priv-default"> + <primary>privilege</primary> + <secondary>default</secondary> + </indexterm> + <para> When an object is created, it is assigned an owner. The owner is normally the role that executed the creation statement. @@ -2049,7 +2054,7 @@ REVOKE ALL ON accounts FROM PUBLIC; reference page of the respective command. </para> - <para> + <para id="ddl-priv-default"> PostgreSQL grants privileges on some types of objects to <literal>PUBLIC</literal> by default when the objects are created. No privileges are granted to <literal>PUBLIC</literal> by default on @@ -2353,8 +2358,11 @@ GRANT SELECT (col1), UPDATE (col1) ON mytable TO miriam_rw; <para> If the <quote>Access privileges</quote> column is empty for a given object, it means the object has default privileges (that is, its - privileges entry in the relevant system catalog is null). Default - privileges always include all privileges for the owner, and can include + privileges entry in the relevant system catalog is null) or empty privileges + (that is, no privileges at all, even for the object owner — a rare + occurrence). One way to distinguish default privileges from empty privileges + is to set <userinput>\pset null '(default)'</userinput>. + Default privileges always include all privileges for the owner, and can include some privileges for <literal>PUBLIC</literal> depending on the object type, as explained above. The first <command>GRANT</command> or <command>REVOKE</command> on an object will instantiate the default diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index d94e3cacfc..7cd12eb867 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -3079,7 +3079,9 @@ lo_import 152801 Sets the string to be printed in place of a null value. The default is to print nothing, which can easily be mistaken for an empty string. For example, one might prefer <literal>\pset null - '(null)'</literal>. + '(null)'</literal>. Setting a non-empty null display string will + help to distinguish between default and empty privileges, as + explained in <xref linkend="ddl-priv"/>. </para> </listitem> </varlistentry> diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index bac94a338c..224aa22575 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -124,7 +124,6 @@ describeAggregates(const char *pattern, bool verbose, bool showSystem) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of aggregate functions"); myopt.translate_header = true; @@ -197,7 +196,6 @@ describeAccessMethods(const char *pattern, bool verbose) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of access methods"); myopt.translate_header = true; myopt.translate_columns = translate_columns; @@ -262,7 +260,6 @@ describeTablespaces(const char *pattern, bool verbose) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of tablespaces"); myopt.translate_header = true; @@ -585,7 +582,6 @@ describeFunctions(const char *functypes, const char *func_pattern, if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of functions"); myopt.translate_header = true; if (pset.sversion >= 90600) @@ -702,7 +698,6 @@ describeTypes(const char *pattern, bool verbose, bool showSystem) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of data types"); myopt.translate_header = true; @@ -893,7 +888,6 @@ describeOperators(const char *oper_pattern, if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of operators"); myopt.translate_header = true; @@ -995,7 +989,6 @@ listAllDbs(const char *pattern, bool verbose) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of databases"); myopt.translate_header = true; @@ -1146,7 +1139,6 @@ permissionsList(const char *pattern, bool showSystem) if (!res) goto error_return; - myopt.nullPrint = NULL; printfPQExpBuffer(&buf, _("Access privileges")); myopt.title = buf.data; myopt.translate_header = true; @@ -1218,7 +1210,6 @@ listDefaultACLs(const char *pattern) if (!res) goto error_return; - myopt.nullPrint = NULL; printfPQExpBuffer(&buf, _("Default access privileges")); myopt.title = buf.data; myopt.translate_header = true; @@ -1417,7 +1408,6 @@ objectDescription(const char *pattern, bool showSystem) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("Object descriptions"); myopt.translate_header = true; myopt.translate_columns = translate_columns; @@ -3852,7 +3842,6 @@ listDbRoleSettings(const char *pattern, const char *pattern2) } else { - myopt.nullPrint = NULL; myopt.title = _("List of settings"); myopt.translate_header = true; @@ -3926,7 +3915,6 @@ describeRoleGrants(const char *pattern, bool showSystem) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of role grants"); myopt.translate_header = true; @@ -4122,7 +4110,6 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys } else { - myopt.nullPrint = NULL; myopt.title = _("List of relations"); myopt.translate_header = true; myopt.translate_columns = translate_columns; @@ -4332,7 +4319,6 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose) initPQExpBuffer(&title); appendPQExpBufferStr(&title, tabletitle); - myopt.nullPrint = NULL; myopt.title = title.data; myopt.translate_header = true; myopt.translate_columns = translate_columns; @@ -4412,7 +4398,6 @@ listLanguages(const char *pattern, bool verbose, bool showSystem) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of languages"); myopt.translate_header = true; @@ -4497,7 +4482,6 @@ listDomains(const char *pattern, bool verbose, bool showSystem) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of domains"); myopt.translate_header = true; @@ -4576,7 +4560,6 @@ listConversions(const char *pattern, bool verbose, bool showSystem) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of conversions"); myopt.translate_header = true; myopt.translate_columns = translate_columns; @@ -4644,7 +4627,6 @@ describeConfigurationParameters(const char *pattern, bool verbose, if (!res) return false; - myopt.nullPrint = NULL; if (pattern) myopt.title = _("List of configuration parameters"); else @@ -4726,7 +4708,6 @@ listEventTriggers(const char *pattern, bool verbose) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of event triggers"); myopt.translate_header = true; myopt.translate_columns = translate_columns; @@ -4825,7 +4806,6 @@ listExtendedStats(const char *pattern) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of extended statistics"); myopt.translate_header = true; @@ -4938,7 +4918,6 @@ listCasts(const char *pattern, bool verbose) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of casts"); myopt.translate_header = true; myopt.translate_columns = translate_columns; @@ -5057,7 +5036,6 @@ listCollations(const char *pattern, bool verbose, bool showSystem) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of collations"); myopt.translate_header = true; myopt.translate_columns = translate_columns; @@ -5119,7 +5097,6 @@ listSchemas(const char *pattern, bool verbose, bool showSystem) if (!res) goto error_return; - myopt.nullPrint = NULL; myopt.title = _("List of schemas"); myopt.translate_header = true; @@ -5236,7 +5213,6 @@ listTSParsers(const char *pattern, bool verbose) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of text search parsers"); myopt.translate_header = true; @@ -5384,7 +5360,6 @@ describeOneTSParser(const char *oid, const char *nspname, const char *prsname) if (!res) return false; - myopt.nullPrint = NULL; initPQExpBuffer(&title); if (nspname) printfPQExpBuffer(&title, _("Text search parser \"%s.%s\""), @@ -5421,7 +5396,6 @@ describeOneTSParser(const char *oid, const char *nspname, const char *prsname) return false; } - myopt.nullPrint = NULL; if (nspname) printfPQExpBuffer(&title, _("Token types for parser \"%s.%s\""), nspname, prsname); @@ -5497,7 +5471,6 @@ listTSDictionaries(const char *pattern, bool verbose) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of text search dictionaries"); myopt.translate_header = true; @@ -5563,7 +5536,6 @@ listTSTemplates(const char *pattern, bool verbose) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of text search templates"); myopt.translate_header = true; @@ -5618,7 +5590,6 @@ listTSConfigs(const char *pattern, bool verbose) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of text search configurations"); myopt.translate_header = true; @@ -5764,7 +5735,6 @@ describeOneTSConfig(const char *oid, const char *nspname, const char *cfgname, appendPQExpBuffer(&title, _("\nParser: \"%s\""), prsname); - myopt.nullPrint = NULL; myopt.title = title.data; myopt.footers = NULL; myopt.topt.default_footer = false; @@ -5841,7 +5811,6 @@ listForeignDataWrappers(const char *pattern, bool verbose) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of foreign-data wrappers"); myopt.translate_header = true; @@ -5918,7 +5887,6 @@ listForeignServers(const char *pattern, bool verbose) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of foreign servers"); myopt.translate_header = true; @@ -5974,7 +5942,6 @@ listUserMappings(const char *pattern, bool verbose) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of user mappings"); myopt.translate_header = true; @@ -6047,7 +6014,6 @@ listForeignTables(const char *pattern, bool verbose) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of foreign tables"); myopt.translate_header = true; @@ -6099,7 +6065,6 @@ listExtensions(const char *pattern) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of installed extensions"); myopt.translate_header = true; @@ -6203,7 +6168,6 @@ listOneExtensionContents(const char *extname, const char *oid) if (!res) return false; - myopt.nullPrint = NULL; initPQExpBuffer(&title); printfPQExpBuffer(&title, _("Objects in extension \"%s\""), extname); myopt.title = title.data; @@ -6340,7 +6304,6 @@ listPublications(const char *pattern) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of publications"); myopt.translate_header = true; myopt.translate_columns = translate_columns; @@ -6695,7 +6658,6 @@ describeSubscriptions(const char *pattern, bool verbose) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of subscriptions"); myopt.translate_header = true; myopt.translate_columns = translate_columns; @@ -6808,7 +6770,6 @@ listOperatorClasses(const char *access_method_pattern, if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of operator classes"); myopt.translate_header = true; myopt.translate_columns = translate_columns; @@ -6897,7 +6858,6 @@ listOperatorFamilies(const char *access_method_pattern, if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of operator families"); myopt.translate_header = true; myopt.translate_columns = translate_columns; @@ -6996,7 +6956,6 @@ listOpFamilyOperators(const char *access_method_pattern, if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of operators of operator families"); myopt.translate_header = true; myopt.translate_columns = translate_columns; @@ -7089,7 +7048,6 @@ listOpFamilyFunctions(const char *access_method_pattern, if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("List of support functions of operator families"); myopt.translate_header = true; myopt.translate_columns = translate_columns; @@ -7141,7 +7099,6 @@ listLargeObjects(bool verbose) if (!res) return false; - myopt.nullPrint = NULL; myopt.title = _("Large objects"); myopt.translate_header = true; -- 2.41.0
