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
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=> <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..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; +