Subject: psql: \dl+ to list large objects privileges

From: Pavel Luzanov <[email-masked]>
Date: Tue, 31 Aug 2021 17:14:12 +0300

Hello,

While working through the documentation I found an empty cell in the 
table for the large objects privilege display with the psql command [1]. 
And indeed the \dl command does not show privileges. And there is no 
modifier + for it.

This patch adds a + modifier to the \dl command and also to the \lo_list 
command to display privilege information on large objects.

I decided to move the do_lo_list function to describe.c in order to use 
the printACLColumn helper function. And at the same time I renamed 
do_lo_list to listLargeObjects to unify with the names of other similar 
functions.

I don't like how I handled the + modifier in the \lo_list command. But I 
don't know how to do better now. This is the second time I've programmed 
in C. The first time was the 'Hello World' program. So maybe something 
is done wrong.

If it's interesting, I can add the patch to commitfest.

1. 
https://www.postgresql.org/docs/devel/ddl-priv.html#PRIVILEGES-SUMMARY-TABLE

-- 
Pavel Luzanov
Postgres Professional: https://postgrespro.com
The Russian Postgres Company

diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index e0ffb020bf..374515bcb2 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -2094,7 +2094,7 @@ REVOKE ALL ON accounts FROM PUBLIC;
       <entry><literal>LARGE OBJECT</literal></entry>
       <entry><literal>rw</literal></entry>
       <entry>none</entry>
-      <entry></entry>
+      <entry><literal>\dl+</literal></entry>
      </row>
      <row>
       <entry><literal>SCHEMA</literal></entry>
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index fcab5c0d51..de47ef528e 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1681,11 +1681,13 @@ testdb=&gt;

       <varlistentry>
-        <term><literal>\dl</literal></term>
+        <term><literal>\dl[+]</literal></term>
         <listitem>
         <para>
         This is an alias for <command>\lo_list</command>, which shows a
-        list of large objects.
+        list of large objects. If <literal>+</literal> is appended 
+        to the command name, each large object is listed with its 
+        associated permissions, if any.
         </para>
         </listitem>
       </varlistentry>
@@ -2588,12 +2590,15 @@ lo_import 152801
       </varlistentry>
 
       <varlistentry>
-        <term><literal>\lo_list</literal></term>
+        <term><literal>\lo_list[+]</literal></term>
         <listitem>
         <para>
         Shows a list of all <productname>PostgreSQL</productname>
         large objects currently stored in the database,
         along with any comments provided for them.
+        If <literal>+</literal> is appended to the command name, 
+        each large object is listed with its associated permissions,
+        if any. 
         </para>
         </listitem>
       </varlistentry>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 49d4c0e3ce..5251b0bab8 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -807,7 +807,7 @@ exec_command_d(PsqlScanState scan_state, bool active_branch, const char *cmd)
 				success = describeRoles(pattern, show_verbose, show_system);
 				break;
 			case 'l':
-				success = do_lo_list();
+				success = listLargeObjects(show_verbose);
 				break;
 			case 'L':
 				success = listLanguages(pattern, show_verbose, show_system);
@@ -1936,7 +1936,10 @@ exec_command_lo(PsqlScanState scan_state, bool active_branch, const char *cmd)
 		}
 
 		else if (strcmp(cmd + 3, "list") == 0)
-			success = do_lo_list();
+			success = listLargeObjects(false);
+
+		else if (strcmp(cmd + 3, "list+") == 0)
+			success = listLargeObjects(true);
 
 		else if (strcmp(cmd + 3, "unlink") == 0)
 		{
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 90ff649be7..bb04e2d5f7 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -6828,3 +6828,65 @@ listOpFamilyFunctions(const char *access_method_pattern,
 	PQclear(res);
 	return true;
 }
+
+/*
+ * \dl or \lo_list
+ * Lists large objects in database
+ */
+bool
+listLargeObjects(bool verbose)
+{
+        PQExpBufferData buf;
+        PGresult   *res;
+        printQueryOpt myopt = pset.popt;
+
+        initPQExpBuffer(&buf);
+
+        if (pset.sversion >= 90000)
+        {
+                printfPQExpBuffer(&buf,
+                                 "SELECT oid as \"%s\",\n"
+                                 "  pg_catalog.pg_get_userbyid(lomowner) as \"%s\",\n  ",
+                                 gettext_noop("ID"),
+                                 gettext_noop("Owner"));
+
+                if (verbose)
+                {
+                        printACLColumn(&buf, "lomacl");
+                        appendPQExpBufferStr(&buf, ",\n  ");
+                }
+
+                appendPQExpBuffer(&buf,
+                                 "pg_catalog.obj_description(oid, 'pg_largeobject') as \"%s\"\n"
+                                 "FROM pg_catalog.pg_largeobject_metadata\n"
+                                 "ORDER BY oid",
+                                 gettext_noop("Description"));
+
+        }
+        else
+        {
+                printfPQExpBuffer(&buf,
+                                 "SELECT loid as \"%s\",\n"
+                                 "  pg_catalog.obj_description(loid, 'pg_largeobject') as \"%s\"\n"
+                                 "FROM (SELECT DISTINCT loid FROM pg_catalog.pg_largeobject) x\n"
+                                 "ORDER BY 1",
+                                 gettext_noop("ID"),
+                                 gettext_noop("Description"));
+        }
+
+        res = PSQLexec(buf.data);
+        termPQExpBuffer(&buf);
+        if (!res)
+                return false;
+
+        myopt.topt.tuples_only = false;
+        myopt.nullPrint = NULL;
+        myopt.title = _("Large objects");
+        myopt.translate_header = true;
+
+        printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+
+        PQclear(res);
+        return true;
+}
+
diff --git a/src/bin/psql/describe.h b/src/bin/psql/describe.h
index 71b320f1fc..53738cc0cb 100644
--- a/src/bin/psql/describe.h
+++ b/src/bin/psql/describe.h
@@ -139,5 +139,8 @@ extern bool listOpFamilyOperators(const char *accessMethod_pattern,
 extern bool listOpFamilyFunctions(const char *access_method_pattern,
 								  const char *family_pattern, bool verbose);
 
+/* \dl or \lo_list */
+extern bool listLargeObjects(bool verbose);
+
 
 #endif							/* DESCRIBE_H */
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index d3fda67edd..7f1c9bc912 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -248,7 +248,7 @@ slashUsage(unsigned short int pager)
 	fprintf(output, _("  \\dFt[+] [PATTERN]      list text search templates\n"));
 	fprintf(output, _("  \\dg[S+] [PATTERN]      list roles\n"));
 	fprintf(output, _("  \\di[S+] [PATTERN]      list indexes\n"));
-	fprintf(output, _("  \\dl                    list large objects, same as \\lo_list\n"));
+	fprintf(output, _("  \\dl[+]                 list large objects, same as \\lo_list\n"));
 	fprintf(output, _("  \\dL[S+] [PATTERN]      list procedural languages\n"));
 	fprintf(output, _("  \\dm[S+] [PATTERN]      list materialized views\n"));
 	fprintf(output, _("  \\dn[S+] [PATTERN]      list schemas\n"));
@@ -324,7 +324,7 @@ slashUsage(unsigned short int pager)
 	fprintf(output, _("Large Objects\n"));
 	fprintf(output, _("  \\lo_export LOBOID FILE\n"
 					  "  \\lo_import FILE [COMMENT]\n"
-					  "  \\lo_list\n"
+					  "  \\lo_list[+]\n"
 					  "  \\lo_unlink LOBOID      large object operations\n"));
 
 	ClosePager(output);
diff --git a/src/bin/psql/large_obj.c b/src/bin/psql/large_obj.c
index c15fcc0885..243875be83 100644
--- a/src/bin/psql/large_obj.c
+++ b/src/bin/psql/large_obj.c
@@ -262,55 +262,3 @@ do_lo_unlink(const char *loid_arg)
 
 	return true;
 }
-
-
-
-/*
- * do_lo_list()
- *
- * Show all large objects in database with comments
- */
-bool
-do_lo_list(void)
-{
-	PGresult   *res;
-	char		buf[1024];
-	printQueryOpt myopt = pset.popt;
-
-	if (pset.sversion >= 90000)
-	{
-		snprintf(buf, sizeof(buf),
-				 "SELECT oid as \"%s\",\n"
-				 "  pg_catalog.pg_get_userbyid(lomowner) as \"%s\",\n"
-				 "  pg_catalog.obj_description(oid, 'pg_largeobject') as \"%s\"\n"
-				 "  FROM pg_catalog.pg_largeobject_metadata "
-				 "  ORDER BY oid",
-				 gettext_noop("ID"),
-				 gettext_noop("Owner"),
-				 gettext_noop("Description"));
-	}
-	else
-	{
-		snprintf(buf, sizeof(buf),
-				 "SELECT loid as \"%s\",\n"
-				 "  pg_catalog.obj_description(loid, 'pg_largeobject') as \"%s\"\n"
-				 "FROM (SELECT DISTINCT loid FROM pg_catalog.pg_largeobject) x\n"
-				 "ORDER BY 1",
-				 gettext_noop("ID"),
-				 gettext_noop("Description"));
-	}
-
-	res = PSQLexec(buf);
-	if (!res)
-		return false;
-
-	myopt.topt.tuples_only = false;
-	myopt.nullPrint = NULL;
-	myopt.title = _("Large objects");
-	myopt.translate_header = true;
-
-	printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
-
-	PQclear(res);
-	return true;
-}
diff --git a/src/bin/psql/large_obj.h b/src/bin/psql/large_obj.h
index 003acbf52c..3172a7704d 100644
--- a/src/bin/psql/large_obj.h
+++ b/src/bin/psql/large_obj.h
@@ -11,6 +11,5 @@
 bool		do_lo_export(const char *loid_arg, const char *filename_arg);
 bool		do_lo_import(const char *filename_arg, const char *comment_arg);
 bool		do_lo_unlink(const char *loid_arg);
-bool		do_lo_list(void);
 
 #endif							/* LARGE_OBJ_H */

---

From: Daniel Gustafsson <[email-masked]>
Date: Tue, 31 Aug 2021 16:35:30 +0200

> On 31 Aug 2021, at 16:14, Pavel Luzanov <[email-masked]> wrote:

> If it's interesting, I can add the patch to commitfest.

Please do, if it was interesting enough for you to write it, it’s interesting
enough to be in the commitfest.

--
Daniel Gustafsson		https://vmware.com/

---

From: Pavel Luzanov <[email-masked]>
Date: Tue, 31 Aug 2021 20:13:55 +0300

On 31.08.2021 17:35, Daniel Gustafsson wrote:
> Please do, if it was interesting enough for you to write it, it’s 
> interesting enough to be in the commitfest.

Thanks, added to the commitfest.

Pavel Luzanov

---

From: Georgios Kokolatos <[email-masked]>
Date: Fri, 03 Sep 2021 12:25:44 +0000

The following review has been posted through the commitfest application:
make installcheck-world:  not tested
Implements feature:       not tested
Spec compliant:           not tested
Documentation:            not tested

Hi,

thank you for the patch, I personally think it is a useful addition and thus it
gets my vote. However, I also think that the proposed code will need some
changes.

On a high level I will recommend the addition of tests. There are similar tests
present in:
    ./src/test/regress/sql/psql.sql

I will also inquire as to the need for renaming the function `do_lo_list` to
`listLargeObjects` and its move to describe.c. from large_obj.c. In itself it is
not necessarily a blocking point, though it will require some strong arguments
for doing so.

Applying the patch, generates several whitespace warnings. It will be helpful
if those warnings are removed.

The patch contains:

                        case 'l':

It might be of some interest to consider in the above to check the value of the
next character in command or emit an error if not valid. Such a pattern can be
found in the same switch block as for example:

                    switch (cmd[2])
                    {
                        case '\0':
                        case '+':
                <snip>
                        success = ...
                </snip>
                            break;
                        default:
                            status = PSQL_CMD_UNKNOWN;
                            break;
                    }

The patch contains:

In a fashion similar to `exec_command_list`, it might be interesting to consider
expressing the above as:

        show_verbose = strchr(cmd, '+') ? true : false;
        <snip>
        else if (strcmp(cmd + 3, "list") == 0
                success = do_lo_list(show_verbose);

Thoughts?

Cheers,
//Georgios

---

From: Georgios Kokolatos <[email-masked]>
Date: Fri, 03 Sep 2021 12:45:14 +0000

Hi,

There is something I forgot to mention in my previous review.

Typically when describing a thing in psql, it is the column "Description" that
belongs in the verbose version. For example listing indexes produces:

               List of relations
 Schema |   Name    | Type  |  Owner   | Table

and the verbose version:
                                           List of relations
 Schema |   Name    | Type  |  Owner   | Table | Persistence | Access method |    Size    | Description

Since '\dl' already contains description, it might be worthwhile to consider to add the column `Access privileges`
without introducing a verbose version.

Thoughts?

Cheers,
//Georgios

---

From: Pavel Luzanov <[email-masked]>
Date: Fri, 3 Sep 2021 16:20:09 +0300

Hello,

Thank you very much for review.

> without introducing a verbose version.

I thought about it.
The reason why I decided to add the verbose version is because of 
backward compatibility. Perhaps the appearance of a new column in an 
existing command may become undesirable to someone.

If we don't worry about backward compatibility, the patch will be 
easier. But I'm not sure this is the right approach.

Pavel Luzanov

---

From: Pavel Luzanov <[email-masked]>
Date: Fri, 3 Sep 2021 16:43:43 +0300

Hello,

Thank you very mush for review.

I will prepare a new version of the patch according to your comments. 

> not necessarily a blocking point, though it will require some strong arguments

I understand that I needed a good reason for such actions.

On the one hand all the commands for working with large objects are in 
large_obj.c. On the other hand, all commands for displaying the contents 
of system catalogs are in describe.c. The function do_lo_list belongs to 
both groups.

The main reason for moving the function to describe.c is that I wanted 
to use the printACLColumn function to display lomacl column. 
printACLColumn function is used in all the other commands to display 
privileges and this function is locally defined in describe.c and there 
is no reason to make in public.

Another option is to duplicate the printACLColumn function (or its 
contents) in large_obj.c. This seemed wrong to me.
Is it any other way?

Pavel Luzanov

---

From: Pavel Luzanov <[email-masked]>
Date: Sun, 5 Sep 2021 22:47:27 +0300

Hi,

> On a high level I will recommend the addition of tests. There are similar tests

Tests added.

> if those warnings are removed.

I know this is a silly mistake, and after reading this article[1] I 
tried to remove the extra spaces.

Can you tell me, please, how can you get such warnings?

>                              status = PSQL_CMD_UNKNOWN;

Check added.

>                  success = do_lo_list(show_verbose);

I rewrote this part.

New version attached.

[1] https://wiki.postgresql.org/wiki/Creating_Clean_Patches

       <varlistentry>
 
       <varlistentry>
-        <term><literal>\lo_list</literal></term>
+        <term><literal>\lo_list[+]</literal></term>
         <listitem>
         <para>
         Shows a list of all <productname>PostgreSQL</productname>
         large objects currently stored in the database,
         along with any comments provided for them.
+        If <literal>+</literal> is appended to the command name, 
+        each large object is listed with its associated permissions,
+        if any. 
         </para>
         </listitem>
       </varlistentry>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 49d4c0e3ce..f3645cab96 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -807,7 +807,16 @@ exec_command_d(PsqlScanState scan_state, bool active_branch, const char *cmd)
 				success = describeRoles(pattern, show_verbose, show_system);
 				break;
 			case 'l':
-				success = do_lo_list();
+				switch (cmd[2])
+				{
+					case '\0':
+					case '+':
+						success = listLargeObjects(show_verbose);
+						break;
+					default:
+						status = PSQL_CMD_UNKNOWN;
+						break;
+				}
 				break;
 			case 'L':
 				success = listLanguages(pattern, show_verbose, show_system);
@@ -1901,11 +1910,13 @@ exec_command_lo(PsqlScanState scan_state, bool active_branch, const char *cmd)
 	{
 		char	   *opt1,
 				   *opt2;
+		bool		show_verbose;
 
 		opt1 = psql_scan_slash_option(scan_state,
 									  OT_NORMAL, NULL, true);
 		opt2 = psql_scan_slash_option(scan_state,
 									  OT_NORMAL, NULL, true);
+		show_verbose = strchr(cmd, '+') ? true : false;
 
 		if (strcmp(cmd + 3, "export") == 0)
 		{
@@ -1935,8 +1946,8 @@ exec_command_lo(PsqlScanState scan_state, bool active_branch, const char *cmd)
 			}
 		}
 
-		else if (strcmp(cmd + 3, "list") == 0)
-			success = do_lo_list();
+		else if (strcmp(cmd + 3, "list") == 0 || strcmp(cmd + 3, "list+") == 0)
+			success = listLargeObjects(show_verbose);
 
index 90ff649be7..a079ce9e36 100644
@@ -6828,3 +6828,64 @@ listOpFamilyFunctions(const char *access_method_pattern,
 
+/* \dl or \lo_list */
 
 #endif							/* DESCRIBE_H */
 
 	ClosePager(output);
 
 	return true;
 }
-
-
-
-/*
- * do_lo_list()
- *
 
 #endif							/* LARGE_OBJ_H */
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 1b2f6bc418..62002e4ec3 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -5179,3 +5179,49 @@ List of access methods
  pg_catalog | &&   | anyarray      | anyarray       | boolean     | overlaps
 (1 row)
 
+-- check printing info about large objects
+CREATE ROLE lo_test;
+SELECT lo_create(42);
+ lo_create 
+-----------
+        42
+(1 row)
+
+ALTER LARGE OBJECT 42 OWNER TO lo_test;
+GRANT SELECT ON LARGE OBJECT 42 TO public;
+\dl
+       Large objects
+ ID |  Owner  | Description 
+----+---------+-------------
+ 42 | lo_test | 
+(1 row)
+
+\dl+
+                  Large objects
+ ID |  Owner  | Access privileges  | Description 
+----+---------+--------------------+-------------
+ 42 | lo_test | lo_test=rw/lo_test+| 
+    |         | =r/lo_test         | 
+(1 row)
+
+\lo_list
+       Large objects
+ ID |  Owner  | Description 
+----+---------+-------------
+ 42 | lo_test | 
+(1 row)
+
+\lo_list+
+                  Large objects
+ ID |  Owner  | Access privileges  | Description 
+----+---------+--------------------+-------------
+ 42 | lo_test | lo_test=rw/lo_test+| 
+    |         | =r/lo_test         | 
+(1 row)
+
+\dl-
+invalid command \dl-
+\lo_list-
+invalid command \lo_list-
+\lo_unlink 42
+DROP ROLE lo_test;
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 68121d171c..43b38213f6 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -1241,3 +1241,18 @@ drop role regress_partitioning_role;
 \dfa bit* small*
 \do - pg_catalog.int4
 \do && anyarray *
+
+-- check printing info about large objects
+CREATE ROLE lo_test;
+SELECT lo_create(42);
+ALTER LARGE OBJECT 42 OWNER TO lo_test;
+GRANT SELECT ON LARGE OBJECT 42 TO public;
+\dl
+\dl+
+\lo_list
+\lo_list+
+\dl-
+\lo_list-
+\lo_unlink 42
+DROP ROLE lo_test;
+

---

From: [email-masked]
Date: Mon, 06 Sep 2021 11:39:51 +0000

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Sunday, September 5th, 2021 at 21:47, Pavel Luzanov <[email-masked]> wrote:

Hi,

> > On a high level I will recommend the addition of tests. There are similar tests

Thanks! The tests look good. A minor nitpick would be to also add a comment on the
large object to verify that it is picked up correctly.

Also:

   +\lo_unlink 42
   +DROP ROLE lo_test;
   +

That last empty line can be removed.

> I know this is a silly mistake, and after reading this article[1] I tried to remove the extra spaces.

I only mentioned it because I thought you might find it useful.
I apply patches by `git apply` and executing the command on the latest version
of your patch, produces:

  $ git apply lo-list-acl-v2.patch
  lo-list-acl-v2.patch:349: new blank line at EOF.
  +
  warning: 1 line adds whitespace errors

The same can be observed highlighted by executing `git diff --color` as
recommended in the the article you linked.

> >                             status = PSQL_CMD_UNKNOWN;

Thanks.

> >                 success = do_lo_list(show_verbose);

Thank you. It looks good to me.

> New version attached.

The new version looks good to me.

I did spend a bit of time considering the addition of the verbose version of
the command. I understand your reasoning explained upstream in the same thread.
However, I am not entirely convinced. The columns in consideration are,
"Description" and "Access Privileges". Within the describe commands there are
four different options, listed and explained bellow:

   commands where description is found in verbose
\d \dA \dc \dd \des \df \dFd \dFt \di \dL \dn \dO \dP \dPt \dt \du \dx \dy \da
\db \dC \dD \det \dew \dFp \dg \dl \dm \do \dPi \dS \dT

   commands where description is not found in verbose (including object
   description)
\dd \dFd \dFt \dL \dx \da \dF \dFp \dl \do \dT

   commands where access privileges is found in verbose
\def \dD \des

   commands where access privileges is not found in verbose (including access
   privilege describing)
\ddp \dp \des \df \dL \dn \db \dD \dew \dl \dT

With the above list, I would argue that it feels more natural to include
the "Access Privileges" column in the non verbose version and be done with
the verbose version all together.

My apologies for the back and forth on this detail.

Thoughts?

Cheers,
//Georgios

---

From: Pavel Luzanov <[email-masked]>
Date: Mon, 6 Sep 2021 17:10:51 +0300

Hi,

>    warning: 1 line adds whitespace errors

Thanks, this is what I was looking for. The patch command doesn't show these warnings
(or I don't know the right way for use it).

> the verbose version all together.

My thoughts.
For most object types, the Description column is shown only in the verbose
version of the commands. But there are several object types,
including Large Objects, for which the description is shown in the normal version.
Both are valid options, so the Description column for large objects stays
in the normal version of the command.

Regarding the display of access privileges.
Instances of object types for which you can manage the access privileges
are listed in Table 5.2 [1].

For clarity, I will only show the first and last columns:

Table 5.2. Summary of Access Privileges

Object Type                     psql Command
------------------------------  ------------
DATABASE                        \l
DOMAIN                          \dD+
FUNCTION or PROCEDURE           \df+
FOREIGN DATA WRAPPER            \dew+
FOREIGN SERVER                  \des+
LANGUAGE                        \dL+
LARGE OBJECT
SCHEMA                          \dn+
SEQUENCE                        \dp
TABLE (and table-like objects)  \dp
Table column                    \dp
TABLESPACE                      \db+
TYPE                            \dT+

By the way, after seeing an empty cell for Large Objects in this table,
I decided to make a patch.

Note that the \dp command is specially designed to display access privileges,
so you don't need to pay attention to the lack of a + sign for it.

It turns out that all commands use the verbose version (or special command)
to display access privileges. Except the \l command for the databases.

Now the question.
Should we add a second exception and display access privileges
for large objects with the \dl command or do the verbose version
like most other commands: \dl+
?

If you still think that there is no need for a verbose version,
I will drop it and add an 'Access privileges' column to the normal command.

[1] https://www.postgresql.org/docs/devel/ddl-priv.html#PRIVILEGES-SUMMARY-TABLE

Pavel Luzanov

---

From: Pavel Luzanov <[email-masked]>
Date: Tue, 7 Sep 2021 14:28:38 +0300

Hi,

> That last empty line can be removed.

The new version adds a comment to a large object and removes the last empty line.

       <varlistentry>
 
       <varlistentry>

 

+/* \dl or \lo_list */
 
 #endif							/* DESCRIBE_H */
 
 	ClosePager(output);
 
 	return true;
 }
-
-
-
-/*
- * do_lo_list()
- *
 
 #endif							/* LARGE_OBJ_H */
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 1b2f6bc418..00ec76d728 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -5179,3 +5179,51 @@ List of access methods
  pg_catalog | &&   | anyarray      | anyarray       | boolean     | overlaps
 (1 row)
 
+-- check printing info about large objects
+-- one large object with OID=42 and owner lo_test is expected in the output
+CREATE ROLE lo_test;
+SELECT lo_create(42);
+ lo_create 
+-----------
+        42
+(1 row)
+
+ALTER LARGE OBJECT 42 OWNER TO lo_test;
+COMMENT ON LARGE OBJECT 42 IS 'the answer to the ultimate question of life';
+GRANT SELECT ON LARGE OBJECT 42 TO public;
+\dl
+                       Large objects
+ ID |  Owner  |                 Description                 
+----+---------+---------------------------------------------
+ 42 | lo_test | the answer to the ultimate question of life
+(1 row)
+
+\dl+
+                                  Large objects
+ ID |  Owner  | Access privileges  |                 Description                 
+----+---------+--------------------+---------------------------------------------
+ 42 | lo_test | lo_test=rw/lo_test+| the answer to the ultimate question of life
+    |         | =r/lo_test         | 
+(1 row)
+
+\lo_list
+                       Large objects
+ ID |  Owner  |                 Description                 
+----+---------+---------------------------------------------
+ 42 | lo_test | the answer to the ultimate question of life
+(1 row)
+
+\lo_list+
+                                  Large objects
+ ID |  Owner  | Access privileges  |                 Description                 
+----+---------+--------------------+---------------------------------------------
+ 42 | lo_test | lo_test=rw/lo_test+| the answer to the ultimate question of life
+    |         | =r/lo_test         | 
+(1 row)
+
+\dl-
+invalid command \dl-
+\lo_list-
+invalid command \lo_list-
+\lo_unlink 42
+DROP ROLE lo_test;
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 68121d171c..ceb1555170 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -1241,3 +1241,19 @@ drop role regress_partitioning_role;
 \dfa bit* small*
 \do - pg_catalog.int4
 \do && anyarray *
+
+-- check printing info about large objects
+-- one large object with OID=42 and owner lo_test is expected in the output
+CREATE ROLE lo_test;
+SELECT lo_create(42);
+ALTER LARGE OBJECT 42 OWNER TO lo_test;
+COMMENT ON LARGE OBJECT 42 IS 'the answer to the ultimate question of life';
+GRANT SELECT ON LARGE OBJECT 42 TO public;
+\dl
+\dl+
+\lo_list
+\lo_list+
+\dl-
+\lo_list-
+\lo_unlink 42
+DROP ROLE lo_test;

---

From: Neil Chen <[email-masked]>
Date: Sat, 18 Sep 2021 02:41:54 +0000

The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, passed
Spec compliant:           tested, passed
Documentation:            tested, passed

Hi, I think this is an interesting patch. +1
I tested it for the latest version, and it works well.

---

From: Pavel Luzanov <[email-masked]>
Date: Mon, 20 Sep 2021 11:47:02 +0300

Hi,

Thank you for testing.
As far as I understand, for the patch to move forward, someone has to become a reviewer
and change the status in the commitfest app.

Pavel Luzanov