Justin, Tom,

Thanks for the review and the help in splitting the patch into two parts.

I wonder if we shouldn't put these new tests in largeobject.sql instead.
(That would mean there are two expected-files to change, which is a bit
tedious, but it's not very hard.)

As suggested, I moved the tests from psql.sql to largeobject.sql.
The tests are added to the beginning of the file because I need one large object with a known id and a known owner.  This guarantees the same output of \dl+ as expected.

I made the same changes to two files in the 'expected' directory: largeobject.out and largeobject_1.out. Although I must say that I still can't understand why more than one file with expected result is used for some tests.

Also, I decided to delete following line in the listLargeObjects function because all the other commands in describe.c do not contain it:
    myopt.topt.tuples_only = false;

This changed the existing behavior, but is consistent with the other commands.

Second version (after splitting) is attached.
v2-0001-move-and-rename-do_lo_list.patch - no changes,
v2-0002-print-large-object-acls.patch - tests moved to largeobject.sql

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

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index fb3bab9494..0d64757899 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -811,7 +811,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();
 				break;
 			case 'L':
 				success = listLanguages(pattern, show_verbose, show_system);
@@ -1963,7 +1963,7 @@ 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();
 
 		else if (strcmp(cmd + 3, "unlink") == 0)
 		{
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index c28788e84f..0d92f3a80b 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -6455,3 +6455,39 @@ listOpFamilyFunctions(const char *access_method_pattern,
 	PQclear(res);
 	return true;
 }
+
+/*
+ * \dl or \lo_list
+ * Lists large objects
+ */
+bool
+listLargeObjects(void)
+{
+	PGresult   *res;
+	char		buf[1024];
+	printQueryOpt myopt = pset.popt;
+
+	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"));
+
+	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/describe.h b/src/bin/psql/describe.h
index 71b320f1fc..3a55e1e4ed 100644
--- a/src/bin/psql/describe.h
+++ b/src/bin/psql/describe.h
@@ -139,5 +139,7 @@ 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(void);
 
 #endif							/* DESCRIBE_H */
diff --git a/src/bin/psql/large_obj.c b/src/bin/psql/large_obj.c
index 10e47c87ac..243875be83 100644
--- a/src/bin/psql/large_obj.c
+++ b/src/bin/psql/large_obj.c
@@ -262,42 +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;
-
-	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"));
-
-	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/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 64d9030652..22f6c5c7ab 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -2146,7 +2146,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 ae38d3dcc3..1ab200a4ad 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1681,11 +1681,14 @@ 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.
+        If <literal>+</literal> is appended to the command name,
+        each large object is listed with its associated permissions,
+        if any.
         </para>
         </listitem>
       </varlistentry>
@@ -2610,12 +2613,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 0d64757899..ceedcc860c 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -811,7 +811,16 @@ exec_command_d(PsqlScanState scan_state, bool active_branch, const char *cmd)
 				success = describeRoles(pattern, show_verbose, show_system);
 				break;
 			case 'l':
-				success = listLargeObjects();
+				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);
@@ -1928,11 +1937,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)
 		{
@@ -1962,8 +1973,8 @@ exec_command_lo(PsqlScanState scan_state, bool active_branch, const char *cmd)
 			}
 		}
 
-		else if (strcmp(cmd + 3, "list") == 0)
-			success = listLargeObjects();
+		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 aa68e9dc6b..cdb33719ff 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -6469,27 +6469,37 @@ listOpFamilyFunctions(const char *access_method_pattern,
  * Lists large objects
  */
 bool
-listLargeObjects(void)
+listLargeObjects(bool verbose)
 {
+	PQExpBufferData buf;
 	PGresult   *res;
-	char		buf[1024];
 	printQueryOpt myopt = pset.popt;
 
-	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"));
-
-	res = PSQLexec(buf);
+	initPQExpBuffer(&buf);
+
+	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"));
+
+	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;
diff --git a/src/bin/psql/describe.h b/src/bin/psql/describe.h
index 3a55e1e4ed..b57ba67bba 100644
--- a/src/bin/psql/describe.h
+++ b/src/bin/psql/describe.h
@@ -140,6 +140,6 @@ extern bool listOpFamilyFunctions(const char *access_method_pattern,
 								  const char *family_pattern, bool verbose);
 
 /* \dl or \lo_list */
-extern bool listLargeObjects(void);
+extern bool listLargeObjects(bool verbose);
 
 #endif							/* DESCRIBE_H */
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 8cadfbb103..5752a36ac8 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"));
@@ -325,7 +325,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/test/regress/expected/largeobject.out b/src/test/regress/expected/largeobject.out
index f461ca3b9f..e1f469b1bd 100644
--- a/src/test/regress/expected/largeobject.out
+++ b/src/test/regress/expected/largeobject.out
@@ -4,33 +4,77 @@
 -- directory paths are passed to us in environment variables
 \getenv abs_srcdir PG_ABS_SRCDIR
 \getenv abs_builddir PG_ABS_BUILDDIR
--- ensure consistent test output regardless of the default bytea format
-SET bytea_output TO escape;
--- Load a file
-CREATE TABLE lotest_stash_values (loid oid, fd integer);
--- lo_creat(mode integer) returns oid
--- The mode arg to lo_creat is unused, some vestigal holdover from ancient times
--- returns the large object id
-INSERT INTO lotest_stash_values (loid) SELECT lo_creat(42);
+-- Test printing large object acls.
+-- The expected output should contain only one large object
+-- with a known id (42) and a known owner (regress_lo_user).
+SELECT lo_create(42);
+ lo_create 
+-----------
+        42
+(1 row)
+
 -- Test ALTER LARGE OBJECT
 CREATE ROLE regress_lo_user;
-DO $$
-  BEGIN
-    EXECUTE 'ALTER LARGE OBJECT ' || (select loid from lotest_stash_values)
-		|| ' OWNER TO regress_lo_user';
-  END
-$$;
+ALTER LARGE OBJECT 42 OWNER TO regress_lo_user;
 SELECT
 	rol.rolname
 FROM
-	lotest_stash_values s
-	JOIN pg_largeobject_metadata lo ON s.loid = lo.oid
-	JOIN pg_authid rol ON lo.lomowner = rol.oid;
+	pg_largeobject_metadata lo
+	JOIN pg_authid rol ON lo.lomowner = rol.oid
+WHERE
+    lo.oid = 42;
      rolname     
 -----------------
  regress_lo_user
 (1 row)
 
+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 | regress_lo_user | the answer to the ultimate question of life
+(1 row)
+
+\dl+
+                                              Large objects
+ ID |      Owner      |         Access privileges          |                 Description                 
+----+-----------------+------------------------------------+---------------------------------------------
+ 42 | regress_lo_user | regress_lo_user=rw/regress_lo_user+| the answer to the ultimate question of life
+    |                 | =r/regress_lo_user                 | 
+(1 row)
+
+\lo_list
+                           Large objects
+ ID |      Owner      |                 Description                 
+----+-----------------+---------------------------------------------
+ 42 | regress_lo_user | the answer to the ultimate question of life
+(1 row)
+
+\lo_list+
+                                              Large objects
+ ID |      Owner      |         Access privileges          |                 Description                 
+----+-----------------+------------------------------------+---------------------------------------------
+ 42 | regress_lo_user | regress_lo_user=rw/regress_lo_user+| the answer to the ultimate question of life
+    |                 | =r/regress_lo_user                 | 
+(1 row)
+
+-- Invalid commands
+\dl-
+invalid command \dl-
+\lo_list-
+invalid command \lo_list-
+-- Cleanup
+\lo_unlink 42
+-- ensure consistent test output regardless of the default bytea format
+SET bytea_output TO escape;
+-- Load a file
+CREATE TABLE lotest_stash_values (loid oid, fd integer);
+-- lo_creat(mode integer) returns oid
+-- The mode arg to lo_creat is unused, some vestigal holdover from ancient times
+-- returns the large object id
+INSERT INTO lotest_stash_values (loid) SELECT lo_creat(42);
 -- NOTE: large objects require transactions
 BEGIN;
 -- lo_open(lobjId oid, mode integer) returns integer
diff --git a/src/test/regress/expected/largeobject_1.out b/src/test/regress/expected/largeobject_1.out
index a9725c375d..39e27b9609 100644
--- a/src/test/regress/expected/largeobject_1.out
+++ b/src/test/regress/expected/largeobject_1.out
@@ -4,33 +4,77 @@
 -- directory paths are passed to us in environment variables
 \getenv abs_srcdir PG_ABS_SRCDIR
 \getenv abs_builddir PG_ABS_BUILDDIR
--- ensure consistent test output regardless of the default bytea format
-SET bytea_output TO escape;
--- Load a file
-CREATE TABLE lotest_stash_values (loid oid, fd integer);
--- lo_creat(mode integer) returns oid
--- The mode arg to lo_creat is unused, some vestigal holdover from ancient times
--- returns the large object id
-INSERT INTO lotest_stash_values (loid) SELECT lo_creat(42);
+-- Test printing large object acls.
+-- The expected output should contain only one large object
+-- with a known id (42) and a known owner (regress_lo_user).
+SELECT lo_create(42);
+ lo_create 
+-----------
+        42
+(1 row)
+
 -- Test ALTER LARGE OBJECT
 CREATE ROLE regress_lo_user;
-DO $$
-  BEGIN
-    EXECUTE 'ALTER LARGE OBJECT ' || (select loid from lotest_stash_values)
-		|| ' OWNER TO regress_lo_user';
-  END
-$$;
+ALTER LARGE OBJECT 42 OWNER TO regress_lo_user;
 SELECT
 	rol.rolname
 FROM
-	lotest_stash_values s
-	JOIN pg_largeobject_metadata lo ON s.loid = lo.oid
-	JOIN pg_authid rol ON lo.lomowner = rol.oid;
+	pg_largeobject_metadata lo
+	JOIN pg_authid rol ON lo.lomowner = rol.oid
+WHERE
+    lo.oid = 42;
      rolname     
 -----------------
  regress_lo_user
 (1 row)
 
+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 | regress_lo_user | the answer to the ultimate question of life
+(1 row)
+
+\dl+
+                                              Large objects
+ ID |      Owner      |         Access privileges          |                 Description                 
+----+-----------------+------------------------------------+---------------------------------------------
+ 42 | regress_lo_user | regress_lo_user=rw/regress_lo_user+| the answer to the ultimate question of life
+    |                 | =r/regress_lo_user                 | 
+(1 row)
+
+\lo_list
+                           Large objects
+ ID |      Owner      |                 Description                 
+----+-----------------+---------------------------------------------
+ 42 | regress_lo_user | the answer to the ultimate question of life
+(1 row)
+
+\lo_list+
+                                              Large objects
+ ID |      Owner      |         Access privileges          |                 Description                 
+----+-----------------+------------------------------------+---------------------------------------------
+ 42 | regress_lo_user | regress_lo_user=rw/regress_lo_user+| the answer to the ultimate question of life
+    |                 | =r/regress_lo_user                 | 
+(1 row)
+
+-- Invalid commands
+\dl-
+invalid command \dl-
+\lo_list-
+invalid command \lo_list-
+-- Cleanup
+\lo_unlink 42
+-- ensure consistent test output regardless of the default bytea format
+SET bytea_output TO escape;
+-- Load a file
+CREATE TABLE lotest_stash_values (loid oid, fd integer);
+-- lo_creat(mode integer) returns oid
+-- The mode arg to lo_creat is unused, some vestigal holdover from ancient times
+-- returns the large object id
+INSERT INTO lotest_stash_values (loid) SELECT lo_creat(42);
 -- NOTE: large objects require transactions
 BEGIN;
 -- lo_open(lobjId oid, mode integer) returns integer
diff --git a/src/test/regress/sql/largeobject.sql b/src/test/regress/sql/largeobject.sql
index 16da077f3a..a9c2f4cadc 100644
--- a/src/test/regress/sql/largeobject.sql
+++ b/src/test/regress/sql/largeobject.sql
@@ -6,6 +6,34 @@
 \getenv abs_srcdir PG_ABS_SRCDIR
 \getenv abs_builddir PG_ABS_BUILDDIR
 
+-- Test printing large object acls.
+-- The expected output should contain only one large object
+-- with a known id (42) and a known owner (regress_lo_user).
+SELECT lo_create(42);
+
+-- Test ALTER LARGE OBJECT
+CREATE ROLE regress_lo_user;
+ALTER LARGE OBJECT 42 OWNER TO regress_lo_user;
+SELECT
+	rol.rolname
+FROM
+	pg_largeobject_metadata lo
+	JOIN pg_authid rol ON lo.lomowner = rol.oid
+WHERE
+    lo.oid = 42;
+
+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+
+-- Invalid commands
+\dl-
+\lo_list-
+-- Cleanup
+\lo_unlink 42
+
 -- ensure consistent test output regardless of the default bytea format
 SET bytea_output TO escape;
 
@@ -16,21 +44,6 @@ CREATE TABLE lotest_stash_values (loid oid, fd integer);
 -- returns the large object id
 INSERT INTO lotest_stash_values (loid) SELECT lo_creat(42);
 
--- Test ALTER LARGE OBJECT
-CREATE ROLE regress_lo_user;
-DO $$
-  BEGIN
-    EXECUTE 'ALTER LARGE OBJECT ' || (select loid from lotest_stash_values)
-		|| ' OWNER TO regress_lo_user';
-  END
-$$;
-SELECT
-	rol.rolname
-FROM
-	lotest_stash_values s
-	JOIN pg_largeobject_metadata lo ON s.loid = lo.oid
-	JOIN pg_authid rol ON lo.lomowner = rol.oid;
-
 -- NOTE: large objects require transactions
 BEGIN;
 

Reply via email to