Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mMyFj-0006d2-1I for pgsql-hackers@arkaria.postgresql.org; Sun, 05 Sep 2021 19:56:31 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1mMy72-0008QB-Km for pgsql-hackers@arkaria.postgresql.org; Sun, 05 Sep 2021 19:47:32 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mMy72-0008Q2-9Y for pgsql-hackers@lists.postgresql.org; Sun, 05 Sep 2021 19:47:32 +0000 Received: from mail.postgrespro.ru ([93.174.131.139]) by magus.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1mMy6z-0003U3-Lx for pgsql-hackers@lists.postgresql.org; Sun, 05 Sep 2021 19:47:31 +0000 Received: from [10.0.1.8] (unknown [188.123.231.167]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mail.postgrespro.ru (Postfix) with ESMTPSA id 11CF321C8C3F; Sun, 5 Sep 2021 22:47:28 +0300 (MSK) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=postgrespro.ru; s=mail; t=1630871248; bh=b1H76rQZaOBiCcJdNl/7LiDCWfW0WXN8JPRwjFIPUCo=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=FI5a0A27wzFpqJ2BujcfOHyvLPySuCkfJSCf4SxDCQCk2+rZAmpwP9zthhAc6rzpl lSlVRrBt6iuBGl7np3B/6zzdRH9GnmcOtQpGfDvRgYdwHyT/rAtPfP4H9C99STl67f PiEmG/PHz6yn0QJ7SD6D8J2ja1Z9fbU30LrtT5Pw= Subject: Re: psql: \dl+ to list large objects privileges To: Georgios Kokolatos , pgsql-hackers@lists.postgresql.org Cc: Pavel Luzanov References: <6d722115-6297-bc53-bb7f-5f150e765299@postgrespro.ru> <90da33e5-5886-11f0-d22f-b2ff8b2d4b99@postgrespro.ru> <163067194414.1167.8140856874411625212.pgcf@coridan.postgresql.org> From: Pavel Luzanov Message-ID: <933f0fb3-5405-7160-cc8e-12f1a439c803@postgrespro.ru> Date: Sun, 5 Sep 2021 22:47:27 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: <163067194414.1167.8140856874411625212.pgcf@coridan.postgresql.org> Content-Type: multipart/mixed; boundary="------------7BEF82609BD4B920FD6F2FE7" Content-Language: ru List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk This is a multi-part message in MIME format. --------------7BEF82609BD4B920FD6F2FE7 Content-Type: multipart/alternative; boundary="------------2D804926471C624D4EDE731C" --------------2D804926471C624D4EDE731C Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Hi, On 03.09.2021 15:25, Georgios Kokolatos wrote: > On a high level I will recommend the addition of tests. There are similar tests Tests added. > Applying the patch, generates several whitespace warnings. It will be helpful > 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? > The patch contains: > > case 'l': > - success = do_lo_list(); > + success = listLargeObjects(show_verbose); > > > 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 '+': > > success = ... > > break; > default: > status = PSQL_CMD_UNKNOWN; > break; > } Check added. > The patch contains: > > else if (strcmp(cmd + 3, "list") == 0) > - success = do_lo_list(); > + success = listLargeObjects(false); > + > + else if (strcmp(cmd + 3, "list+") == 0) > + success = listLargeObjects(true); > > > In a fashion similar to `exec_command_list`, it might be interesting to consider > expressing the above as: > > show_verbose = strchr(cmd, '+') ? true : false; > > else if (strcmp(cmd + 3, "list") == 0 > success = do_lo_list(show_verbose); I rewrote this part. New version attached. [1] https://wiki.postgresql.org/wiki/Creating_Clean_Patches -- Pavel Luzanov Postgres Professional: https://postgrespro.com The Russian Postgres Company --------------2D804926471C624D4EDE731C Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 7bit 
Hi,



    
On 03.09.2021
        15:25, Georgios Kokolatos wrote:
    

    

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

    

    

    

    
Tests added.

    



    

      
Applying the patch, generates several whitespace warnings. It will be helpful
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?

    

    

      
The patch contains:

                        case 'l':
-                               success = do_lo_list();
+                               success = listLargeObjects(show_verbose);


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;
                    }

    

    

    
    
Check added.



    

      
The patch contains:

                else if (strcmp(cmd + 3, "list") == 0)
-                       success = do_lo_list();
+                       success = listLargeObjects(false);
+
+               else if (strcmp(cmd + 3, "list+") == 0)
+                       success = listLargeObjects(true);


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);

    

    

    
I rewrote this part.

    
New version attached.

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

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

  


--------------2D804926471C624D4EDE731C--

--------------7BEF82609BD4B920FD6F2FE7
Content-Type: text/x-patch; charset=UTF-8;
 name="lo-list-acl-v2.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="lo-list-acl-v2.patch"

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;
       LARGE OBJECT
       rw
       none
-      
+      \dl+
      
      
       SCHEMA
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=>
 
 
       
-        \dl
+        \dl[+]
         
         
         This is an alias for \lo_list, which shows a
-        list of large objects.
+        list of large objects. If + is appended 
+        to the command name, each large object is listed with its 
+        associated permissions, if any.
         
         
       
@@ -2588,12 +2590,15 @@ lo_import 152801
       
 
       
-        \lo_list
+        \lo_list[+]
         
         
         Shows a list of all PostgreSQL
         large objects currently stored in the database,
         along with any comments provided for them.
+        If + is appended to the command name, 
+        each large object is listed with its associated permissions,
+        if any. 
         
         
       
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);
 
 		else if (strcmp(cmd + 3, "unlink") == 0)
 		{
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 90ff649be7..a079ce9e36 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -6828,3 +6828,64 @@ 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 */
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;
+

--------------7BEF82609BD4B920FD6F2FE7--