psql: Add \dL to show languages
Hi all,
I'd like to revive Fernando Ike's patch implementing the "\dL" command
for psql to list available languages, last version here:
http://archives.postgresql.org/pgsql-hackers/2009-07/msg01092.php
The original patch produced columns "Name", "Owner", "Procedural
Language", "Trusted", "Call Handler", and "Validator". I propose
simplifying the non-verbose output of \dL to look like this:
Name | Owner | Trusted
---------------------+-------+---------
plperl | josh | t
plpgsql | josh | t
plpythonu | josh | f
(3 rows)
since the rest of the columns in the original patch seem like they
would be distracting noise the majority of the time[2]For example, the command "droplang --list" only prints out "Name" and "Trusted?" columns.. I've kept most
of the original columns in the verbose output.
Tom Lane and Peter Eisentraut gave feedback on the original patch. I
think these concerns raised by Peter should now be addressed:
1) This is obviously wrong:
CASE WHEN l.lanispl = 't' THEN 'Trusted' WHEN l.lanispl = 'f' THEN 'Untrusted' END
I ripped out this "Procedural Language" column[1]I'm not sure what Fernando intended the original "Procedural Language" column to be, but that column displayed "Trusted" or "Untrusted" in addition to the "Trusted" column. Maybe this was a typo in the patch? In any event, I don't think it's useful to have a separate "Name" and "Procedural Language" column. If we did want to include a Procedural Language column in addition to the Name, I'm not sure offhand where to get this information, e.g. how to get the string "PL/pgSQL" given pg_language.lanname = 'plpgsql'.
2) It may be better to use lanispl to determine whether a language is a system
object or not. It's kind of obscure, but pg_dump does it that way, so it'd at
least be consistent.
I added a "System Object" column in the verbose output with this information.
3) Your code does processSQLNamePattern(), but neither the help nor the
documentation mention that \dL accepts a pattern. A pattern for listing
languages might be overkill, but at least the documentation needs to match
what the code attempts to do.
I added a note to the psql-ref.sgml documentation that \dL accepts a
pattern. I agree it's probably overkill to support pattern matching
when most folks will have maybe 1-3 additional languages installed,
but it's easy enough to add in, and similar psql functions support
patterns as well.
4) Instead of LEFT JOIN pg_catalog.pg_proc p on l.lanplcallfoid = p.oid etc,
just cast the oid field to regprocedure. See examples elsewhere in
describe.c.
Done, though I didn't see anything else in describe.c using casts to
regprocedure. Maybe there's a better way?
I've also fixed the tab-completion for \dL's pattern input. I haven't
yet test backwards compatibility with older server versions, though it
looks like this patch should work fine by not querying for "lanowner"
on 8.2 and earlier; I didn't see any other columns missing in
pg_language back to at least 8.1.
Josh
--
[1]: I'm not sure what Fernando intended the original "Procedural Language" column to be, but that column displayed "Trusted" or "Untrusted" in addition to the "Trusted" column. Maybe this was a typo in the patch? In any event, I don't think it's useful to have a separate "Name" and "Procedural Language" column. If we did want to include a Procedural Language column in addition to the Name, I'm not sure offhand where to get this information, e.g. how to get the string "PL/pgSQL" given pg_language.lanname = 'plpgsql'
Language" column to be, but that column displayed "Trusted" or
"Untrusted" in addition to the "Trusted" column. Maybe this was a typo
in the patch? In any event, I don't think it's useful to have a
separate "Name" and "Procedural Language" column. If we did want to
include a Procedural Language column in addition to the Name, I'm not
sure offhand where to get this information, e.g. how to get the string
"PL/pgSQL" given pg_language.lanname = 'plpgsql'
[2]: For example, the command "droplang --list" only prints out "Name" and "Trusted?" columns.
and "Trusted?" columns.
Attachments:
psql_languages.v5.patchapplication/octet-stream; name=psql_languages.v5.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index d44fc56..814a848 100644
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*************** testdb=>
*** 1230,1235 ****
--- 1230,1250 ----
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>\dL[S+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
+ <listitem>
+ <para>
+ Lists all procedural languages. If <replaceable
+ class="parameter">pattern</replaceable>
+ is specified, only languages whose names match the pattern are listed.
+ By default, only user-created languages
+ are shown; supply the <literal>S</literal> modifier to include system
+ objects. If <literal>+</literal> is appended to the command name, each
+ language is listed with its call handler, validator, access privileges,
+ and whether it is a system object.
+ </para>
+ </listitem>
+ </varlistentry>
<varlistentry>
<term><literal>\dn[S+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index c1edf44..eb7bb85 100644
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*************** exec_command(const char *cmd,
*** 416,421 ****
--- 416,424 ----
case 'l':
success = do_lo_list();
break;
+ case 'L':
+ success = listLanguages(pattern, show_verbose, show_system);
+ break;
case 'n':
success = listSchemas(pattern, show_verbose, show_system);
break;
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index c4370a1..2833711 100644
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
*************** listTables(const char *tabtypes, const c
*** 2492,2497 ****
--- 2492,2562 ----
}
+ /* \dL
+ *
+ * Describes Languages.
+ */
+ bool
+ listLanguages(const char *pattern, bool verbose, bool showSystem)
+ {
+ PQExpBufferData buf;
+ PGresult *res;
+ printQueryOpt myopt = pset.popt;
+
+ initPQExpBuffer(&buf);
+
+ printfPQExpBuffer(&buf,
+ "SELECT l.lanname AS \"%s\",\n",
+ gettext_noop("Procedural Language"));
+ if (pset.sversion >= 80300)
+ appendPQExpBuffer(&buf,
+ " pg_catalog.pg_get_userbyid(l.lanowner) as \"%s\",\n",
+ gettext_noop("Owner"));
+
+ appendPQExpBuffer(&buf,
+ " l.lanpltrusted AS \"%s\"",
+ gettext_noop("Trusted"));
+
+ if (verbose)
+ {
+ appendPQExpBuffer(&buf,
+ ",\n l.lanplcallfoid::regprocedure AS \"%s\",\n"
+ " l.lanvalidator::regprocedure AS \"%s\",\n"
+ " NOT l.lanispl AS \"%s\",\n",
+ gettext_noop("Call Handler"),
+ gettext_noop("Validator"),
+ gettext_noop("System Language"));
+ printACLColumn(&buf, "l.lanacl");
+ }
+
+ appendPQExpBuffer(&buf,
+ " FROM pg_catalog.pg_language l\n");
+
+ processSQLNamePattern(pset.db, &buf, pattern, false, false,
+ NULL, "l.lanname", NULL, NULL);
+
+ if (!showSystem && !pattern)
+ appendPQExpBuffer(&buf, "WHERE lanplcallfoid != 0");
+
+ appendPQExpBuffer(&buf, " ORDER BY 1;");
+
+ res = PSQLexec(buf.data, false);
+ termPQExpBuffer(&buf);
+ if (!res)
+ return false;
+
+ myopt.nullPrint = NULL;
+ myopt.title = _("List of languages");
+ myopt.translate_header = true;
+
+ printQuery(res, &myopt, pset.queryFout, pset.logfile);
+
+ PQclear(res);
+ return true;
+
+ }
+
+
/*
* \dD
*
diff --git a/src/bin/psql/describe.h b/src/bin/psql/describe.h
index 6a6abdb..9c1fe68 100644
*** a/src/bin/psql/describe.h
--- b/src/bin/psql/describe.h
*************** extern bool listForeignServers(const cha
*** 81,85 ****
--- 81,87 ----
/* \deu */
extern bool listUserMappings(const char *pattern, bool verbose);
+ /* \dL */
+ extern bool listLanguages(const char *pattern, bool verbose, bool showSystem);
#endif /* DESCRIBE_H */
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 4458d63..d078406 100644
*** a/src/bin/psql/help.c
--- b/src/bin/psql/help.c
*************** slashUsage(unsigned short int pager)
*** 210,215 ****
--- 210,216 ----
fprintf(output, _(" \\dg[+] [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[S+] [PATTERN] list (procedural) languages\n"));
fprintf(output, _(" \\dn[+] [PATTERN] list schemas\n"));
fprintf(output, _(" \\do[S] [PATTERN] list operators\n"));
fprintf(output, _(" \\dp [PATTERN] list table, view, and sequence access privileges\n"));
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 2c36850..e7a2b4f 100644
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** psql_completion(char *text, int start, i
*** 646,652 ****
static const char *const backslash_commands[] = {
"\\a", "\\connect", "\\conninfo", "\\C", "\\cd", "\\copy", "\\copyright",
"\\d", "\\da", "\\db", "\\dc", "\\dC", "\\dd", "\\dD", "\\des", "\\deu", "\\dew", "\\df",
! "\\dF", "\\dFd", "\\dFp", "\\dFt", "\\dg", "\\di", "\\dl",
"\\dn", "\\do", "\\dp", "\\drds", "\\ds", "\\dS", "\\dt", "\\dT", "\\dv", "\\du",
"\\e", "\\echo", "\\ef", "\\encoding",
"\\f", "\\g", "\\h", "\\help", "\\H", "\\i", "\\l",
--- 646,652 ----
static const char *const backslash_commands[] = {
"\\a", "\\connect", "\\conninfo", "\\C", "\\cd", "\\copy", "\\copyright",
"\\d", "\\da", "\\db", "\\dc", "\\dC", "\\dd", "\\dD", "\\des", "\\deu", "\\dew", "\\df",
! "\\dF", "\\dFd", "\\dFp", "\\dFt", "\\dg", "\\di", "\\dl", "\\dL",
"\\dn", "\\do", "\\dp", "\\drds", "\\ds", "\\dS", "\\dt", "\\dT", "\\dv", "\\du",
"\\e", "\\echo", "\\ef", "\\encoding",
"\\f", "\\g", "\\h", "\\help", "\\H", "\\i", "\\l",
*************** psql_completion(char *text, int start, i
*** 2551,2556 ****
--- 2551,2558 ----
else if (strncmp(prev_wd, "\\di", strlen("\\di")) == 0)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_indexes, NULL);
+ else if (strncmp(prev_wd, "\\dL", strlen("\\dL")) == 0)
+ COMPLETE_WITH_QUERY(Query_for_list_of_languages);
else if (strncmp(prev_wd, "\\dn", strlen("\\dn")) == 0)
COMPLETE_WITH_QUERY(Query_for_list_of_schemas);
else if (strncmp(prev_wd, "\\dp", strlen("\\dp")) == 0
On Sun, Nov 21, 2010 at 8:18 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote:
I'd like to revive Fernando Ike's patch implementing the "\dL" command
for psql to list available languages, last version here:
http://archives.postgresql.org/pgsql-hackers/2009-07/msg01092.php
Please add this patch to the currently open CommitFest:
https://commitfest.postgresql.org/action/commitfest_view/open
And please also help with review of patches from the current CommitFest:
https://commitfest.postgresql.org/action/commitfest_view/inprogress
Thanks,
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Sun, Nov 21, 2010 at 8:39 PM, Robert Haas <robertmhaas@gmail.com> wrote:
Please add this patch to the currently open CommitFest:
Added to 2011-01.
https://commitfest.postgresql.org/action/commitfest_view/open
And please also help with review of patches from the current CommitFest:
https://commitfest.postgresql.org/action/commitfest_view/inprogress
Yeah, I know I need to help out on reviews more. I signed on as an
additional reviewer for Thom Brown's "Aditional docs index entries and
table sorting". I'll try to at least take a look at one or two more
without a Reviewer listed (maybe "Tab completion in psql for triggers
on views" or "parallel pg_dump") as time permits, though I'm probably
not qualified to be the only reviewer for either of those.
Josh
On Sun, Nov 21, 2010 at 9:44 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote:
On Sun, Nov 21, 2010 at 8:39 PM, Robert Haas <robertmhaas@gmail.com> wrote:
Please add this patch to the currently open CommitFest:
Added to 2011-01.
https://commitfest.postgresql.org/action/commitfest_view/open
And please also help with review of patches from the current CommitFest:
https://commitfest.postgresql.org/action/commitfest_view/inprogress
Yeah, I know I need to help out on reviews more. I signed on as an
additional reviewer for Thom Brown's "Aditional docs index entries and
table sorting". I'll try to at least take a look at one or two more
without a Reviewer listed (maybe "Tab completion in psql for triggers
on views" or "parallel pg_dump") as time permits, though I'm probably
not qualified to be the only reviewer for either of those.
Anything you can do is great. We always seem to have more patches
than reviewers....
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Hi Josh,
Here is my review of this patch for the commitfest.
Review of https://commitfest.postgresql.org/action/patch_view?id=439
Contents and Purpose
====================
This patch adds the \dL command in psql to list the procedual languages.
To me this seems like a useful addition to the commands in psql and \dL
is also a quite sane name for it which follows the established
conventions.
Documentation of the new command is included and looks good.
Runing it
=========
Patch did not apply cleanly against HEAD so fixed it.
I tested the comamnds, \dL, \dLS, \dL+, \dLS+ and they wroked as I
expected. Support for patterns worked too and while not that useful it
was not much code either. The psql completion worked fine too.
Some things I noticed when using it though.
I do not like the use of parentheses in the usage description "list
(procedural) languages". Why not have it simply as "list procedural
languages"?
Should we include a column in \dL+ for the laninline function (DO
blocks)?
Performance
===========
Quite irrelevant to this patch. :)
Coding
======
In general the code looks good and follows conventions except for some
whitesapce errors that I cleaned up.
* Trailing whitespace in src/bin/psql/describe.c.
* Incorrect indenation, spaces vs tabs in psql/describe.c and
psql/tab-complete.c.
* Removed empty line after return in listLanguages to match other
functions.
The comment for the function is not that descriptive but it is enough
and fits with the other functions.
Another things is that generated SQL needs its formatting fixed up in my
oppinion. I recommend looking at the query built by \dL by using psql
-E. Here is an example how the query looks for \dL+
SELECT l.lanname AS "Procedural Language",
pg_catalog.pg_get_userbyid(l.lanowner) as "Owner",
l.lanpltrusted AS "Trusted",
l.lanplcallfoid::regprocedure AS "Call Handler",
l.lanvalidator::regprocedure AS "Validator",
NOT l.lanispl AS "System Language",
pg_catalog.array_to_string(l.lanacl, E'\n') AS "Access privileges" FROM pg_catalog.pg_language l
ORDER BY 1;
Conclusion
==========
The patch looks useful, worked, and there were no bugs obvious to me.
The code also looks good and in line with other functions doing similar
things. There are just some small issues that I think should be resolved
before committing it: laninline, format of the built query and the usage
string.
Attached is a version that applies to current HEAD and with whitespace
fixed.
Regards,
Andreas
Attachments:
psql_languages.v6.patchtext/x-patch; charset=UTF-8; name=psql_languages.v6.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 5f61561..30d4507 100644
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*************** testdb=>
*** 1249,1254 ****
--- 1249,1269 ----
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>\dL[S+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
+ <listitem>
+ <para>
+ Lists all procedural languages. If <replaceable
+ class="parameter">pattern</replaceable>
+ is specified, only languages whose names match the pattern are listed.
+ By default, only user-created languages
+ are shown; supply the <literal>S</literal> modifier to include system
+ objects. If <literal>+</literal> is appended to the command name, each
+ language is listed with its call handler, validator, access privileges,
+ and whether it is a system object.
+ </para>
+ </listitem>
+ </varlistentry>
<varlistentry>
<term><literal>\dn[S+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 962c13c..301dc11 100644
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*************** exec_command(const char *cmd,
*** 416,421 ****
--- 416,424 ----
case 'l':
success = do_lo_list();
break;
+ case 'L':
+ success = listLanguages(pattern, show_verbose, show_system);
+ break;
case 'n':
success = listSchemas(pattern, show_verbose, show_system);
break;
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 205190f..e41453b 100644
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
*************** listTables(const char *tabtypes, const c
*** 2566,2571 ****
--- 2566,2635 ----
}
+ /* \dL
+ *
+ * Describes Languages.
+ */
+ bool
+ listLanguages(const char *pattern, bool verbose, bool showSystem)
+ {
+ PQExpBufferData buf;
+ PGresult *res;
+ printQueryOpt myopt = pset.popt;
+
+ initPQExpBuffer(&buf);
+
+ printfPQExpBuffer(&buf,
+ "SELECT l.lanname AS \"%s\",\n",
+ gettext_noop("Procedural Language"));
+ if (pset.sversion >= 80300)
+ appendPQExpBuffer(&buf,
+ " pg_catalog.pg_get_userbyid(l.lanowner) as \"%s\",\n",
+ gettext_noop("Owner"));
+
+ appendPQExpBuffer(&buf,
+ " l.lanpltrusted AS \"%s\"",
+ gettext_noop("Trusted"));
+
+ if (verbose)
+ {
+ appendPQExpBuffer(&buf,
+ ",\n l.lanplcallfoid::regprocedure AS \"%s\",\n"
+ " l.lanvalidator::regprocedure AS \"%s\",\n"
+ " NOT l.lanispl AS \"%s\",\n",
+ gettext_noop("Call Handler"),
+ gettext_noop("Validator"),
+ gettext_noop("System Language"));
+ printACLColumn(&buf, "l.lanacl");
+ }
+
+ appendPQExpBuffer(&buf,
+ " FROM pg_catalog.pg_language l\n");
+
+ processSQLNamePattern(pset.db, &buf, pattern, false, false,
+ NULL, "l.lanname", NULL, NULL);
+
+ if (!showSystem && !pattern)
+ appendPQExpBuffer(&buf, "WHERE lanplcallfoid != 0");
+
+ appendPQExpBuffer(&buf, " ORDER BY 1;");
+
+ res = PSQLexec(buf.data, false);
+ termPQExpBuffer(&buf);
+ if (!res)
+ return false;
+
+ myopt.nullPrint = NULL;
+ myopt.title = _("List of languages");
+ myopt.translate_header = true;
+
+ printQuery(res, &myopt, pset.queryFout, pset.logfile);
+
+ PQclear(res);
+ return true;
+ }
+
+
/*
* \dD
*
diff --git a/src/bin/psql/describe.h b/src/bin/psql/describe.h
index 2029ef8..4e80bcf 100644
*** a/src/bin/psql/describe.h
--- b/src/bin/psql/describe.h
*************** extern bool listUserMappings(const char
*** 84,88 ****
--- 84,90 ----
/* \det */
extern bool listForeignTables(const char *pattern, bool verbose);
+ /* \dL */
+ extern bool listLanguages(const char *pattern, bool verbose, bool showSystem);
#endif /* DESCRIBE_H */
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 96c85a2..55986f1 100644
*** a/src/bin/psql/help.c
--- b/src/bin/psql/help.c
*************** slashUsage(unsigned short int pager)
*** 211,216 ****
--- 211,217 ----
fprintf(output, _(" \\dg[+] [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[S+] [PATTERN] list (procedural) languages\n"));
fprintf(output, _(" \\dn[+] [PATTERN] list schemas\n"));
fprintf(output, _(" \\do[S] [PATTERN] list operators\n"));
fprintf(output, _(" \\dp [PATTERN] list table, view, and sequence access privileges\n"));
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 8c15838..84c68a7 100644
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** psql_completion(char *text, int start, i
*** 713,719 ****
static const char *const backslash_commands[] = {
"\\a", "\\connect", "\\conninfo", "\\C", "\\cd", "\\copy", "\\copyright",
"\\d", "\\da", "\\db", "\\dc", "\\dC", "\\dd", "\\dD", "\\des", "\\det", "\\deu", "\\dew", "\\df",
! "\\dF", "\\dFd", "\\dFp", "\\dFt", "\\dg", "\\di", "\\dl",
"\\dn", "\\do", "\\dp", "\\drds", "\\ds", "\\dS", "\\dt", "\\dT", "\\dv", "\\du",
"\\e", "\\echo", "\\ef", "\\encoding",
"\\f", "\\g", "\\h", "\\help", "\\H", "\\i", "\\l",
--- 713,719 ----
static const char *const backslash_commands[] = {
"\\a", "\\connect", "\\conninfo", "\\C", "\\cd", "\\copy", "\\copyright",
"\\d", "\\da", "\\db", "\\dc", "\\dC", "\\dd", "\\dD", "\\des", "\\det", "\\deu", "\\dew", "\\df",
! "\\dF", "\\dFd", "\\dFp", "\\dFt", "\\dg", "\\di", "\\dl", "\\dL",
"\\dn", "\\do", "\\dp", "\\drds", "\\ds", "\\dS", "\\dt", "\\dT", "\\dv", "\\du",
"\\e", "\\echo", "\\ef", "\\encoding",
"\\f", "\\g", "\\h", "\\help", "\\H", "\\i", "\\l",
*************** psql_completion(char *text, int start, i
*** 2680,2685 ****
--- 2680,2687 ----
else if (strncmp(prev_wd, "\\di", strlen("\\di")) == 0)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_indexes, NULL);
+ else if (strncmp(prev_wd, "\\dL", strlen("\\dL")) == 0)
+ COMPLETE_WITH_QUERY(Query_for_list_of_languages);
else if (strncmp(prev_wd, "\\dn", strlen("\\dn")) == 0)
COMPLETE_WITH_QUERY(Query_for_list_of_schemas);
else if (strncmp(prev_wd, "\\dp", strlen("\\dp")) == 0
On Sun, Jan 16, 2011 at 02:26, Andreas Karlsson <andreas@proxel.se> wrote:
Hi Josh,
Contents and Purpose
====================This patch adds the \dL command in psql to list the procedual languages.
<snip>
Some things I noticed when using it though.
I do not like the use of parentheses in the usage description "list
(procedural) languages". Why not have it simply as "list procedural
languages"?
Because it lists non-procedural langauges as well? (I didn't check it,
that's just a guess)
Should we include a column in \dL+ for the laninline function (DO
blocks)?
+1 for doing that. Remember it has to be version-dependent though.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
On Sun, Jan 16, 2011 at 7:04 AM, Magnus Hagander <magnus@hagander.net> wrote:
I do not like the use of parentheses in the usage description "list
(procedural) languages". Why not have it simply as "list procedural
languages"?Because it lists non-procedural langauges as well? (I didn't check it,
that's just a guess)
There are many places in our code and documentation where "procedural
language" or "language" are treated as synonyms. There's no semantic
difference; procedural is simply a noise word.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Sat, Jan 15, 2011 at 8:26 PM, Andreas Karlsson <andreas@proxel.se> wrote:
Hi Josh,
Here is my review of this patch for the commitfest.
Review of https://commitfest.postgresql.org/action/patch_view?id=439
Thanks a lot for the review!
Contents and Purpose
====================This patch adds the \dL command in psql to list the procedual languages.
To me this seems like a useful addition to the commands in psql and \dL
is also a quite sane name for it which follows the established
conventions.Documentation of the new command is included and looks good.
Runing it
=========Patch did not apply cleanly against HEAD so fixed it.
I tested the comamnds, \dL, \dLS, \dL+, \dLS+ and they wroked as I
expected. Support for patterns worked too and while not that useful it
was not much code either. The psql completion worked fine too.
Yeah, IIRC Fernando included pattern-completion in the original patch,
and a reviewer said roughly the same thing -- it's probably overkill,
but since it's just a small amount of code and it's already in there,
no big deal.
Some things I noticed when using it though.
I do not like the use of parentheses in the usage description "list
(procedural) languages". Why not have it simply as "list procedural
languages"?
I agree.
Should we include a column in \dL+ for the laninline function (DO
blocks)?
Hrm, I guess that could be useful for the verbose output at least.
Performance
===========Quite irrelevant to this patch. :)
Coding
======In general the code looks good and follows conventions except for some
whitesapce errors that I cleaned up.* Trailing whitespace in src/bin/psql/describe.c.
* Incorrect indenation, spaces vs tabs in psql/describe.c and
psql/tab-complete.c.
* Removed empty line after return in listLanguages to match other
functions.The comment for the function is not that descriptive but it is enough
and fits with the other functions.Another things is that generated SQL needs its formatting fixed up in my
oppinion. I recommend looking at the query built by \dL by using psql
-E. Here is an example how the query looks for \dL+SELECT l.lanname AS "Procedural Language",
pg_catalog.pg_get_userbyid(l.lanowner) as "Owner",
l.lanpltrusted AS "Trusted",
l.lanplcallfoid::regprocedure AS "Call Handler",
l.lanvalidator::regprocedure AS "Validator",
NOT l.lanispl AS "System Language",
pg_catalog.array_to_string(l.lanacl, E'\n') AS "Access privileges" FROM pg_catalog.pg_language l
ORDER BY 1;
Sorry, I don't understand.. what's wrong with the formatting of this
query? In terms of whitespace, it looks pretty similar to
listDomains() directly below listLanguages() in describe.c.
Conclusion
==========The patch looks useful, worked, and there were no bugs obvious to me.
The code also looks good and in line with other functions doing similar
things. There are just some small issues that I think should be resolved
before committing it: laninline, format of the built query and the usage
string.Attached is a version that applies to current HEAD and with whitespace
fixed.Regards,
Andreas
Thanks for the cleaned up patch.
Josh
On Sun, Jan 16, 2011 at 8:52 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Sun, Jan 16, 2011 at 7:04 AM, Magnus Hagander <magnus@hagander.net> wrote:
I do not like the use of parentheses in the usage description "list
(procedural) languages". Why not have it simply as "list procedural
languages"?Because it lists non-procedural langauges as well? (I didn't check it,
that's just a guess)There are many places in our code and documentation where "procedural
language" or "language" are treated as synonyms. There's no semantic
difference; procedural is simply a noise word.
[bikeshedding]
I agree with Andreas' suggestion that the help string be "list
procedural languages", even though the \dLS output looks something
like this:
List of languages
Procedural Language | Owner | Trusted
---------------------+-------+---------
c | josh | f
internal | josh | f
plpgsql | josh | t
sql | josh | t
(4 rows)
which, as Magnus points out, includes non-procedural languages (SQL).
I think that "list languages" could be confusing to newcomers -- the
very people who might be reading through the help output of psql for
the first time -- who might suppose that "languages" has something to
do with the character sets supported by PostgreSQL, and might not even
be aware that a variety of procedural languages can be used inside the
database.
Josh
On Sun, Jan 16, 2011 at 10:40 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote:
On Sun, Jan 16, 2011 at 8:52 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Sun, Jan 16, 2011 at 7:04 AM, Magnus Hagander <magnus@hagander.net> wrote:
I do not like the use of parentheses in the usage description "list
(procedural) languages". Why not have it simply as "list procedural
languages"?Because it lists non-procedural langauges as well? (I didn't check it,
that's just a guess)There are many places in our code and documentation where "procedural
language" or "language" are treated as synonyms. There's no semantic
difference; procedural is simply a noise word.[bikeshedding]
I agree with Andreas' suggestion that the help string be "list
procedural languages", even though the \dLS output looks something
like this:List of languages
Procedural Language | Owner | Trusted
---------------------+-------+---------
c | josh | f
internal | josh | f
plpgsql | josh | t
sql | josh | t
(4 rows)
By the by, in the output of \df, \dt, \db, etc., that first column is
called simply "Name".
which, as Magnus points out, includes non-procedural languages (SQL).
I think that "list languages" could be confusing to newcomers -- the
very people who might be reading through the help output of psql for
the first time -- who might suppose that "languages" has something to
do with the character sets supported by PostgreSQL, and might not even
be aware that a variety of procedural languages can be used inside the
database.
Fair point.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Mon, Jan 17, 2011 at 05:22, Robert Haas <robertmhaas@gmail.com> wrote:
On Sun, Jan 16, 2011 at 10:40 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote:
On Sun, Jan 16, 2011 at 8:52 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Sun, Jan 16, 2011 at 7:04 AM, Magnus Hagander <magnus@hagander.net> wrote:
I do not like the use of parentheses in the usage description "list
(procedural) languages". Why not have it simply as "list procedural
languages"?Because it lists non-procedural langauges as well? (I didn't check it,
that's just a guess)There are many places in our code and documentation where "procedural
language" or "language" are treated as synonyms. There's no semantic
difference; procedural is simply a noise word.[bikeshedding]
I agree with Andreas' suggestion that the help string be "list
procedural languages", even though the \dLS output looks something
like this:List of languages
Procedural Language | Owner | Trusted
---------------------+-------+---------
c | josh | f
internal | josh | f
plpgsql | josh | t
sql | josh | t
(4 rows)By the by, in the output of \df, \dt, \db, etc., that first column is
called simply "Name".
+1 for just using "name"
which, as Magnus points out, includes non-procedural languages (SQL).
I think that "list languages" could be confusing to newcomers -- the
very people who might be reading through the help output of psql for
the first time -- who might suppose that "languages" has something to
do with the character sets supported by PostgreSQL, and might not even
be aware that a variety of procedural languages can be used inside the
database.Fair point.
Yeah. Procedural langauges may strictly be wrong, but people aren't
likely to misunderstand it.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
On mån, 2011-01-17 at 07:37 +0100, Magnus Hagander wrote:
which, as Magnus points out, includes non-procedural languages (SQL).
I think that "list languages" could be confusing to newcomers -- the
very people who might be reading through the help output of psql for
the first time -- who might suppose that "languages" has something to
do with the character sets supported by PostgreSQL, and might not even
be aware that a variety of procedural languages can be used inside the
database.Fair point.
Yeah. Procedural langauges may strictly be wrong, but people aren't
likely to misunderstand it.
The term "procedural" in this context originated with Oracle's PL/SQL,
which is a procedural language extension to the non-procedural SQL
language. From this came PostgreSQL's PL/pgSQL, and that naming was
then continued with PL/Tcl, at which point "PL/$X" lost its original
meaning of "procedural extension to the non-procedural language $X" and
meant more or less "handler for writing PostgreSQL functions in language
$X".
Otherwise PL/Scheme will blow your mind. :)
Think of "procedural language" as "language for writing [PostgreSQL]
procedures". As was pointed out, it's also a useful convention for
distinguishing this from other "languages", such as message
translations.
On Mon, Jan 17, 2011 at 02:48:43PM +0200, Peter Eisentraut wrote:
On m�n, 2011-01-17 at 07:37 +0100, Magnus Hagander wrote:
which, as Magnus points out, includes non-procedural languages
(SQL).I think that "list languages" could be confusing to newcomers
-- the very people who might be reading through the help output
of psql for the first time -- who might suppose that
"languages" has something to do with the character sets
supported by PostgreSQL, and might not even be aware that a
variety of procedural languages can be used inside the
database.Fair point.
Yeah. Procedural langauges may strictly be wrong, but people
aren't likely to misunderstand it.The term "procedural" in this context originated with Oracle's
PL/SQL, which is a procedural language extension to the
non-procedural SQL language.
We can repurpose 'P' to mean, Programming or PostgreSQL, and have done :)
Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Peter Eisentraut wrote:
On m?n, 2011-01-17 at 07:37 +0100, Magnus Hagander wrote:
which, as Magnus points out, includes non-procedural languages (SQL).
I think that "list languages" could be confusing to newcomers -- the
very people who might be reading through the help output of psql for
the first time -- who might suppose that "languages" has something to
do with the character sets supported by PostgreSQL, and might not even
be aware that a variety of procedural languages can be used inside the
database.Fair point.
Yeah. Procedural langauges may strictly be wrong, but people aren't
likely to misunderstand it.The term "procedural" in this context originated with Oracle's PL/SQL,
which is a procedural language extension to the non-procedural SQL
language. From this came PostgreSQL's PL/pgSQL, and that naming was
then continued with PL/Tcl, at which point "PL/$X" lost its original
meaning of "procedural extension to the non-procedural language $X" and
meant more or less "handler for writing PostgreSQL functions in language
$X".Otherwise PL/Scheme will blow your mind. :)
Think of "procedural language" as "language for writing [PostgreSQL]
procedures". As was pointed out, it's also a useful convention for
distinguishing this from other "languages", such as message
translations.
FYI, I always refer to them as server-side language, to distinguish them
from client-side languages.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
On Mon, 2011-01-17 at 07:37 +0100, Magnus Hagander wrote:
Yeah. Procedural langauges may strictly be wrong, but people aren't
likely to misunderstand it.
That was idea when suggesting we call it "procedural languages". It is
short and I do not think it can be misunderstood.
Regards,
Andreas
On Sun, 2011-01-16 at 22:32 -0500, Josh Kupershmidt wrote:
On Sat, Jan 15, 2011 at 8:26 PM, Andreas Karlsson <andreas@proxel.se> wrote:
Should we include a column in \dL+ for the laninline function (DO
blocks)?Hrm, I guess that could be useful for the verbose output at least.
Magnus Hagander agreed with that idea and added that for that we need to
check the version. The column was added in 9.0 if I recall.
SELECT l.lanname AS "Procedural Language",
pg_catalog.pg_get_userbyid(l.lanowner) as "Owner",
l.lanpltrusted AS "Trusted",
l.lanplcallfoid::regprocedure AS "Call Handler",
l.lanvalidator::regprocedure AS "Validator",
NOT l.lanispl AS "System Language",
pg_catalog.array_to_string(l.lanacl, E'\n') AS "Access privileges" FROM pg_catalog.pg_language l
ORDER BY 1;Sorry, I don't understand.. what's wrong with the formatting of this
query? In terms of whitespace, it looks pretty similar to
listDomains() directly below listLanguages() in describe.c.
It is quite similar, so the general style is correct. Just some errors
in the details. Here are the ones I see in the example above, but there
could be others in other code paths.
* Missing indentation before ACL column, the other functions have it.
* One space before FROM instead of one newline like the other queries.
* The space before ORDER BY.
That's enough nitpickery for now. :)
Regards,
Andreas
Hi all,
I've updated the patch to address the following points:
* help string now says "list procedural languages" (no parentheses now)
* the language name column is now titled "Name"
* added another column in verbose mode for 9.0+ showing whether DO
blocks are possible with the language. I named this column "DO
Blocks?", though am open to suggestions
* fixed the whitespace problems Andreas noticed with the SELECT query
Looking at the verbose output, which looks something like this:
List of languages
Name | Owner | Trusted | Call Handler |
Validator | System Language | DO Blocks?
| Access privileges
-----------+-------+---------+-------------------------+------------------------+-----------------+-----------
-+-------------------
plpgsql | josh | t | plpgsql_call_handler() |
plpgsql_validator(oid) | f | t
|
plpythonu | josh | f | plpython_call_handler() | -
| f | t
|
(2 rows)
I have a hard time imagining users who would find "Call Handler" or
"Validator" useful. This was in Fernando's original patch, and I just
didn't bother to take it out. If others feel the same way, I'd be
happy to rip those columns out.
Few more comments below:
On Mon, Jan 17, 2011 at 3:51 PM, Andreas Karlsson <andreas@proxel.se> wrote:
On Sun, 2011-01-16 at 22:32 -0500, Josh Kupershmidt wrote:
On Sat, Jan 15, 2011 at 8:26 PM, Andreas Karlsson <andreas@proxel.se> wrote:
Should we include a column in \dL+ for the laninline function (DO
blocks)?Hrm, I guess that could be useful for the verbose output at least.
Magnus Hagander agreed with that idea and added that for that we need to
check the version. The column was added in 9.0 if I recall.
Added.
[snip]
* Missing indentation before ACL column, the other functions have it.
* One space before FROM instead of one newline like the other queries.
* The space before ORDER BY.
These should be fixed now.
That's enough nitpickery for now. :)
I spend enough of my time nitpicking others. Turnabout is fair play :)
Thanks for all the review and feedback from everyone.
Josh
Attachments:
psql_languages.v7.patchtext/x-patch; charset=US-ASCII; name=psql_languages.v7.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 5f61561..30d4507 100644
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*************** testdb=>
*** 1249,1254 ****
--- 1249,1269 ----
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>\dL[S+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
+ <listitem>
+ <para>
+ Lists all procedural languages. If <replaceable
+ class="parameter">pattern</replaceable>
+ is specified, only languages whose names match the pattern are listed.
+ By default, only user-created languages
+ are shown; supply the <literal>S</literal> modifier to include system
+ objects. If <literal>+</literal> is appended to the command name, each
+ language is listed with its call handler, validator, access privileges,
+ and whether it is a system object.
+ </para>
+ </listitem>
+ </varlistentry>
<varlistentry>
<term><literal>\dn[S+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 962c13c..301dc11 100644
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*************** exec_command(const char *cmd,
*** 416,421 ****
--- 416,424 ----
case 'l':
success = do_lo_list();
break;
+ case 'L':
+ success = listLanguages(pattern, show_verbose, show_system);
+ break;
case 'n':
success = listSchemas(pattern, show_verbose, show_system);
break;
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 205190f..5984748 100644
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
*************** listTables(const char *tabtypes, const c
*** 2566,2571 ****
--- 2566,2638 ----
}
+ /* \dL
+ *
+ * Describes Languages.
+ */
+ bool
+ listLanguages(const char *pattern, bool verbose, bool showSystem)
+ {
+ PQExpBufferData buf;
+ PGresult *res;
+ printQueryOpt myopt = pset.popt;
+
+ initPQExpBuffer(&buf);
+
+ printfPQExpBuffer(&buf,
+ "SELECT l.lanname AS \"%s\",\n",
+ gettext_noop("Name"));
+ if (pset.sversion >= 80300)
+ appendPQExpBuffer(&buf,
+ " pg_catalog.pg_get_userbyid(l.lanowner) as \"%s\",\n",
+ gettext_noop("Owner"));
+
+ appendPQExpBuffer(&buf,
+ " l.lanpltrusted AS \"%s\"",
+ gettext_noop("Trusted"));
+
+ if (verbose)
+ {
+ appendPQExpBuffer(&buf,
+ ",\n l.lanplcallfoid::regprocedure AS \"%s\",\n"
+ " l.lanvalidator::regprocedure AS \"%s\",\n"
+ " NOT l.lanispl AS \"%s\",\n ",
+ gettext_noop("Call Handler"),
+ gettext_noop("Validator"),
+ gettext_noop("System Language"));
+ if (pset.sversion >= 90000)
+ appendPQExpBuffer(&buf, "l.laninline != 0 AS \"%s\",\n ",
+ gettext_noop("DO Blocks?"));
+ printACLColumn(&buf, "l.lanacl");
+ }
+
+ appendPQExpBuffer(&buf,
+ "\nFROM pg_catalog.pg_language l");
+
+ processSQLNamePattern(pset.db, &buf, pattern, false, false,
+ NULL, "l.lanname", NULL, NULL);
+
+ if (!showSystem && !pattern)
+ appendPQExpBuffer(&buf, "\nWHERE lanplcallfoid != 0");
+
+ appendPQExpBuffer(&buf, "\nORDER BY 1;");
+
+ res = PSQLexec(buf.data, false);
+ termPQExpBuffer(&buf);
+ if (!res)
+ return false;
+
+ myopt.nullPrint = NULL;
+ myopt.title = _("List of languages");
+ myopt.translate_header = true;
+
+ printQuery(res, &myopt, pset.queryFout, pset.logfile);
+
+ PQclear(res);
+ return true;
+ }
+
+
/*
* \dD
*
diff --git a/src/bin/psql/describe.h b/src/bin/psql/describe.h
index 2029ef8..4e80bcf 100644
*** a/src/bin/psql/describe.h
--- b/src/bin/psql/describe.h
*************** extern bool listUserMappings(const char
*** 84,88 ****
--- 84,90 ----
/* \det */
extern bool listForeignTables(const char *pattern, bool verbose);
+ /* \dL */
+ extern bool listLanguages(const char *pattern, bool verbose, bool showSystem);
#endif /* DESCRIBE_H */
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 96c85a2..bd5c4b7 100644
*** a/src/bin/psql/help.c
--- b/src/bin/psql/help.c
*************** slashUsage(unsigned short int pager)
*** 211,216 ****
--- 211,217 ----
fprintf(output, _(" \\dg[+] [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[S+] [PATTERN] list procedural languages\n"));
fprintf(output, _(" \\dn[+] [PATTERN] list schemas\n"));
fprintf(output, _(" \\do[S] [PATTERN] list operators\n"));
fprintf(output, _(" \\dp [PATTERN] list table, view, and sequence access privileges\n"));
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 8c15838..84c68a7 100644
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** psql_completion(char *text, int start, i
*** 713,719 ****
static const char *const backslash_commands[] = {
"\\a", "\\connect", "\\conninfo", "\\C", "\\cd", "\\copy", "\\copyright",
"\\d", "\\da", "\\db", "\\dc", "\\dC", "\\dd", "\\dD", "\\des", "\\det", "\\deu", "\\dew", "\\df",
! "\\dF", "\\dFd", "\\dFp", "\\dFt", "\\dg", "\\di", "\\dl",
"\\dn", "\\do", "\\dp", "\\drds", "\\ds", "\\dS", "\\dt", "\\dT", "\\dv", "\\du",
"\\e", "\\echo", "\\ef", "\\encoding",
"\\f", "\\g", "\\h", "\\help", "\\H", "\\i", "\\l",
--- 713,719 ----
static const char *const backslash_commands[] = {
"\\a", "\\connect", "\\conninfo", "\\C", "\\cd", "\\copy", "\\copyright",
"\\d", "\\da", "\\db", "\\dc", "\\dC", "\\dd", "\\dD", "\\des", "\\det", "\\deu", "\\dew", "\\df",
! "\\dF", "\\dFd", "\\dFp", "\\dFt", "\\dg", "\\di", "\\dl", "\\dL",
"\\dn", "\\do", "\\dp", "\\drds", "\\ds", "\\dS", "\\dt", "\\dT", "\\dv", "\\du",
"\\e", "\\echo", "\\ef", "\\encoding",
"\\f", "\\g", "\\h", "\\help", "\\H", "\\i", "\\l",
*************** psql_completion(char *text, int start, i
*** 2680,2685 ****
--- 2680,2687 ----
else if (strncmp(prev_wd, "\\di", strlen("\\di")) == 0)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_indexes, NULL);
+ else if (strncmp(prev_wd, "\\dL", strlen("\\dL")) == 0)
+ COMPLETE_WITH_QUERY(Query_for_list_of_languages);
else if (strncmp(prev_wd, "\\dn", strlen("\\dn")) == 0)
COMPLETE_WITH_QUERY(Query_for_list_of_schemas);
else if (strncmp(prev_wd, "\\dp", strlen("\\dp")) == 0
Hi Josh,
Nope, I do not have any better ideas than "DO Blocks?".
Everything looks good with the exception one bug now.
\dL foo
********* QUERY **********
SELECT l.lanname AS "Name",
pg_catalog.pg_get_userbyid(l.lanowner) as "Owner",
l.lanpltrusted AS "Trusted"
FROM pg_catalog.pg_language lWHERE l.lanname ~ '^(foo)$'
ORDER BY 1;
**************************
ERROR: syntax error at or near "l"
LINE 4: FROM pg_catalog.pg_language lWHERE l.lanname ~ '^(foo)$'
I believe the fix is to move \n from before the WHERE clause to after
the FROM, and from before ORDER BY to after WHERE.
Fix this bug and I believe this patch is ready for a committer.
PS. You added some trailing withspace after printACLColumn, A
recommendation if you want to avoid it is to either have a git commit
hook which checks for that and/or have colouring of git diffs so you can
see it marked in red. I use both. :)
Andreas
On Tue, Jan 18, 2011 at 1:35 PM, Andreas Karlsson <andreas@proxel.se> wrote:
Hi Josh,
Nope, I do not have any better ideas than "DO Blocks?".
Everything looks good with the exception one bug now.
\dL foo
********* QUERY **********
SELECT l.lanname AS "Name",
pg_catalog.pg_get_userbyid(l.lanowner) as "\Owner",
l.lanpltrusted AS "Trusted"
FROM pg_catalog.pg_language lWHERE l.lanname ~ '^(foo)$'ORDER BY 1;
**************************ERROR: syntax error at or near "l"
LINE 4: FROM pg_catalog.pg_language lWHERE l.lanname ~ '^(foo)$'I believe the fix is to move \n from before the WHERE clause to after
the FROM, and from before ORDER BY to after WHERE.
Whoops, good you caught that. Should be fixed now.
Fix this bug and I believe this patch is ready for a committer.
PS. You added some trailing withspace after printACLColumn, A
recommendation if you want to avoid it is to either have a git commit
hook which checks for that and/or have colouring of git diffs so you can
see it marked in red. I use both. :)
Got that now too. I lost my ~/.emacs file recently, which is mostly
why I'm making whitespace mistakes. Rebuilding slowly though;
(setq-default show-trailing-whitespace t) is what I needed.
I left the "Call Handler" and "Validator" columns in the verbose
output since I haven't heard otherwise.
Josh
Attachments:
psql_languages.v8.patchtext/x-patch; charset=US-ASCII; name=psql_languages.v8.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 5f61561..30d4507 100644
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*************** testdb=>
*** 1249,1254 ****
--- 1249,1269 ----
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>\dL[S+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
+ <listitem>
+ <para>
+ Lists all procedural languages. If <replaceable
+ class="parameter">pattern</replaceable>
+ is specified, only languages whose names match the pattern are listed.
+ By default, only user-created languages
+ are shown; supply the <literal>S</literal> modifier to include system
+ objects. If <literal>+</literal> is appended to the command name, each
+ language is listed with its call handler, validator, access privileges,
+ and whether it is a system object.
+ </para>
+ </listitem>
+ </varlistentry>
<varlistentry>
<term><literal>\dn[S+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 962c13c..301dc11 100644
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*************** exec_command(const char *cmd,
*** 416,421 ****
--- 416,424 ----
case 'l':
success = do_lo_list();
break;
+ case 'L':
+ success = listLanguages(pattern, show_verbose, show_system);
+ break;
case 'n':
success = listSchemas(pattern, show_verbose, show_system);
break;
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 205190f..abfd854 100644
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
*************** listTables(const char *tabtypes, const c
*** 2566,2571 ****
--- 2566,2638 ----
}
+ /* \dL
+ *
+ * Describes Languages.
+ */
+ bool
+ listLanguages(const char *pattern, bool verbose, bool showSystem)
+ {
+ PQExpBufferData buf;
+ PGresult *res;
+ printQueryOpt myopt = pset.popt;
+
+ initPQExpBuffer(&buf);
+
+ printfPQExpBuffer(&buf,
+ "SELECT l.lanname AS \"%s\",\n",
+ gettext_noop("Name"));
+ if (pset.sversion >= 80300)
+ appendPQExpBuffer(&buf,
+ " pg_catalog.pg_get_userbyid(l.lanowner) as \"%s\",\n",
+ gettext_noop("Owner"));
+
+ appendPQExpBuffer(&buf,
+ " l.lanpltrusted AS \"%s\"",
+ gettext_noop("Trusted"));
+
+ if (verbose)
+ {
+ appendPQExpBuffer(&buf,
+ ",\n l.lanplcallfoid::regprocedure AS \"%s\",\n"
+ " l.lanvalidator::regprocedure AS \"%s\",\n"
+ " NOT l.lanispl AS \"%s\",\n ",
+ gettext_noop("Call Handler"),
+ gettext_noop("Validator"),
+ gettext_noop("System Language"));
+ if (pset.sversion >= 90000)
+ appendPQExpBuffer(&buf, "l.laninline != 0 AS \"%s\",\n ",
+ gettext_noop("DO Blocks?"));
+ printACLColumn(&buf, "l.lanacl");
+ }
+
+ appendPQExpBuffer(&buf,
+ "\nFROM pg_catalog.pg_language l\n");
+
+ processSQLNamePattern(pset.db, &buf, pattern, false, false,
+ NULL, "l.lanname", NULL, NULL);
+
+ if (!showSystem && !pattern)
+ appendPQExpBuffer(&buf, "WHERE lanplcallfoid != 0\n");
+
+ appendPQExpBuffer(&buf, "ORDER BY 1;");
+
+ res = PSQLexec(buf.data, false);
+ termPQExpBuffer(&buf);
+ if (!res)
+ return false;
+
+ myopt.nullPrint = NULL;
+ myopt.title = _("List of languages");
+ myopt.translate_header = true;
+
+ printQuery(res, &myopt, pset.queryFout, pset.logfile);
+
+ PQclear(res);
+ return true;
+ }
+
+
/*
* \dD
*
diff --git a/src/bin/psql/describe.h b/src/bin/psql/describe.h
index 2029ef8..4e80bcf 100644
*** a/src/bin/psql/describe.h
--- b/src/bin/psql/describe.h
*************** extern bool listUserMappings(const char
*** 84,88 ****
--- 84,90 ----
/* \det */
extern bool listForeignTables(const char *pattern, bool verbose);
+ /* \dL */
+ extern bool listLanguages(const char *pattern, bool verbose, bool showSystem);
#endif /* DESCRIBE_H */
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 96c85a2..bd5c4b7 100644
*** a/src/bin/psql/help.c
--- b/src/bin/psql/help.c
*************** slashUsage(unsigned short int pager)
*** 211,216 ****
--- 211,217 ----
fprintf(output, _(" \\dg[+] [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[S+] [PATTERN] list procedural languages\n"));
fprintf(output, _(" \\dn[+] [PATTERN] list schemas\n"));
fprintf(output, _(" \\do[S] [PATTERN] list operators\n"));
fprintf(output, _(" \\dp [PATTERN] list table, view, and sequence access privileges\n"));
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 8c15838..84c68a7 100644
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** psql_completion(char *text, int start, i
*** 713,719 ****
static const char *const backslash_commands[] = {
"\\a", "\\connect", "\\conninfo", "\\C", "\\cd", "\\copy", "\\copyright",
"\\d", "\\da", "\\db", "\\dc", "\\dC", "\\dd", "\\dD", "\\des", "\\det", "\\deu", "\\dew", "\\df",
! "\\dF", "\\dFd", "\\dFp", "\\dFt", "\\dg", "\\di", "\\dl",
"\\dn", "\\do", "\\dp", "\\drds", "\\ds", "\\dS", "\\dt", "\\dT", "\\dv", "\\du",
"\\e", "\\echo", "\\ef", "\\encoding",
"\\f", "\\g", "\\h", "\\help", "\\H", "\\i", "\\l",
--- 713,719 ----
static const char *const backslash_commands[] = {
"\\a", "\\connect", "\\conninfo", "\\C", "\\cd", "\\copy", "\\copyright",
"\\d", "\\da", "\\db", "\\dc", "\\dC", "\\dd", "\\dD", "\\des", "\\det", "\\deu", "\\dew", "\\df",
! "\\dF", "\\dFd", "\\dFp", "\\dFt", "\\dg", "\\di", "\\dl", "\\dL",
"\\dn", "\\do", "\\dp", "\\drds", "\\ds", "\\dS", "\\dt", "\\dT", "\\dv", "\\du",
"\\e", "\\echo", "\\ef", "\\encoding",
"\\f", "\\g", "\\h", "\\help", "\\H", "\\i", "\\l",
*************** psql_completion(char *text, int start, i
*** 2680,2685 ****
--- 2680,2687 ----
else if (strncmp(prev_wd, "\\di", strlen("\\di")) == 0)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_indexes, NULL);
+ else if (strncmp(prev_wd, "\\dL", strlen("\\dL")) == 0)
+ COMPLETE_WITH_QUERY(Query_for_list_of_languages);
else if (strncmp(prev_wd, "\\dn", strlen("\\dn")) == 0)
COMPLETE_WITH_QUERY(Query_for_list_of_schemas);
else if (strncmp(prev_wd, "\\dp", strlen("\\dp")) == 0
On Tue, 2011-01-18 at 19:34 -0500, Josh Kupershmidt wrote:
Got that now too. I lost my ~/.emacs file recently, which is mostly
why I'm making whitespace mistakes. Rebuilding slowly though;
(setq-default show-trailing-whitespace t) is what I needed.
Aha, I see.
I left the "Call Handler" and "Validator" columns in the verbose
output since I haven't heard otherwise.Josh
I do not really care either way. The columns are not terribly useful but
not pointless either.
The patch looks alright now so I will mark it as ready for committer
now.
Andreas
On Wed, Jan 19, 2011 at 5:47 PM, Andreas Karlsson <andreas@proxel.se> wrote:
The patch looks alright now so I will mark it as ready for committer
now.
This patch doesn't seem terribly consistent to me - we show the name
of the call handler and the name of the validator, but for the inline
handler we just indicate whether there is one or not. That seems like
something that we should make consistent, and my vote is to show the
name in all cases.
Also, I'm wondering whether the System Language column be renamed to
Internal Language, for consistency with the terminology used here:
http://www.postgresql.org/docs/current/static/catalog-pg-language.html
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Wed, Jan 19, 2011 at 9:09 PM, Robert Haas <robertmhaas@gmail.com> wrote:
This patch doesn't seem terribly consistent to me - we show the name
of the call handler and the name of the validator, but for the inline
handler we just indicate whether there is one or not. That seems like
something that we should make consistent, and my vote is to show the
name in all cases.
OK, changed. I've shuffled the columns slightly so that handlers and
Validator columns are next to each other.
Also, I'm wondering whether the System Language column be renamed to
Internal Language, for consistency with the terminology used here:http://www.postgresql.org/docs/current/static/catalog-pg-language.html
Fixed, updated patch attached. Though, reading the description of
lanispl on that page, I wonder if "user-defined language" correctly
describes plpgsql these days, which comes installed by default.
Josh
Attachments:
psql_languages.v9.patchtext/x-patch; charset=US-ASCII; name=psql_languages.v9.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 5f61561..30d4507 100644
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*************** testdb=>
*** 1249,1254 ****
--- 1249,1269 ----
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>\dL[S+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
+ <listitem>
+ <para>
+ Lists all procedural languages. If <replaceable
+ class="parameter">pattern</replaceable>
+ is specified, only languages whose names match the pattern are listed.
+ By default, only user-created languages
+ are shown; supply the <literal>S</literal> modifier to include system
+ objects. If <literal>+</literal> is appended to the command name, each
+ language is listed with its call handler, validator, access privileges,
+ and whether it is a system object.
+ </para>
+ </listitem>
+ </varlistentry>
<varlistentry>
<term><literal>\dn[S+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 962c13c..301dc11 100644
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*************** exec_command(const char *cmd,
*** 416,421 ****
--- 416,424 ----
case 'l':
success = do_lo_list();
break;
+ case 'L':
+ success = listLanguages(pattern, show_verbose, show_system);
+ break;
case 'n':
success = listSchemas(pattern, show_verbose, show_system);
break;
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 205190f..e1db4c0 100644
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
*************** listTables(const char *tabtypes, const c
*** 2566,2571 ****
--- 2566,2638 ----
}
+ /* \dL
+ *
+ * Describes Languages.
+ */
+ bool
+ listLanguages(const char *pattern, bool verbose, bool showSystem)
+ {
+ PQExpBufferData buf;
+ PGresult *res;
+ printQueryOpt myopt = pset.popt;
+
+ initPQExpBuffer(&buf);
+
+ printfPQExpBuffer(&buf,
+ "SELECT l.lanname AS \"%s\",\n",
+ gettext_noop("Name"));
+ if (pset.sversion >= 80300)
+ appendPQExpBuffer(&buf,
+ " pg_catalog.pg_get_userbyid(l.lanowner) as \"%s\",\n",
+ gettext_noop("Owner"));
+
+ appendPQExpBuffer(&buf,
+ " l.lanpltrusted AS \"%s\"",
+ gettext_noop("Trusted"));
+
+ if (verbose)
+ {
+ appendPQExpBuffer(&buf,
+ ",\n NOT l.lanispl AS \"%s\",\n"
+ " l.lanplcallfoid::regprocedure AS \"%s\",\n"
+ " l.lanvalidator::regprocedure AS \"%s\",\n ",
+ gettext_noop("Internal Language"),
+ gettext_noop("Call Handler"),
+ gettext_noop("Validator"));
+ if (pset.sversion >= 90000)
+ appendPQExpBuffer(&buf, "l.laninline::regprocedure AS \"%s\",\n ",
+ gettext_noop("Inline Handler"));
+ printACLColumn(&buf, "l.lanacl");
+ }
+
+ appendPQExpBuffer(&buf,
+ "\nFROM pg_catalog.pg_language l\n");
+
+ processSQLNamePattern(pset.db, &buf, pattern, false, false,
+ NULL, "l.lanname", NULL, NULL);
+
+ if (!showSystem && !pattern)
+ appendPQExpBuffer(&buf, "WHERE lanplcallfoid != 0\n");
+
+ appendPQExpBuffer(&buf, "ORDER BY 1;");
+
+ res = PSQLexec(buf.data, false);
+ termPQExpBuffer(&buf);
+ if (!res)
+ return false;
+
+ myopt.nullPrint = NULL;
+ myopt.title = _("List of languages");
+ myopt.translate_header = true;
+
+ printQuery(res, &myopt, pset.queryFout, pset.logfile);
+
+ PQclear(res);
+ return true;
+ }
+
+
/*
* \dD
*
diff --git a/src/bin/psql/describe.h b/src/bin/psql/describe.h
index 2029ef8..4e80bcf 100644
*** a/src/bin/psql/describe.h
--- b/src/bin/psql/describe.h
*************** extern bool listUserMappings(const char
*** 84,88 ****
--- 84,90 ----
/* \det */
extern bool listForeignTables(const char *pattern, bool verbose);
+ /* \dL */
+ extern bool listLanguages(const char *pattern, bool verbose, bool showSystem);
#endif /* DESCRIBE_H */
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 96c85a2..bd5c4b7 100644
*** a/src/bin/psql/help.c
--- b/src/bin/psql/help.c
*************** slashUsage(unsigned short int pager)
*** 211,216 ****
--- 211,217 ----
fprintf(output, _(" \\dg[+] [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[S+] [PATTERN] list procedural languages\n"));
fprintf(output, _(" \\dn[+] [PATTERN] list schemas\n"));
fprintf(output, _(" \\do[S] [PATTERN] list operators\n"));
fprintf(output, _(" \\dp [PATTERN] list table, view, and sequence access privileges\n"));
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 8c15838..84c68a7 100644
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** psql_completion(char *text, int start, i
*** 713,719 ****
static const char *const backslash_commands[] = {
"\\a", "\\connect", "\\conninfo", "\\C", "\\cd", "\\copy", "\\copyright",
"\\d", "\\da", "\\db", "\\dc", "\\dC", "\\dd", "\\dD", "\\des", "\\det", "\\deu", "\\dew", "\\df",
! "\\dF", "\\dFd", "\\dFp", "\\dFt", "\\dg", "\\di", "\\dl",
"\\dn", "\\do", "\\dp", "\\drds", "\\ds", "\\dS", "\\dt", "\\dT", "\\dv", "\\du",
"\\e", "\\echo", "\\ef", "\\encoding",
"\\f", "\\g", "\\h", "\\help", "\\H", "\\i", "\\l",
--- 713,719 ----
static const char *const backslash_commands[] = {
"\\a", "\\connect", "\\conninfo", "\\C", "\\cd", "\\copy", "\\copyright",
"\\d", "\\da", "\\db", "\\dc", "\\dC", "\\dd", "\\dD", "\\des", "\\det", "\\deu", "\\dew", "\\df",
! "\\dF", "\\dFd", "\\dFp", "\\dFt", "\\dg", "\\di", "\\dl", "\\dL",
"\\dn", "\\do", "\\dp", "\\drds", "\\ds", "\\dS", "\\dt", "\\dT", "\\dv", "\\du",
"\\e", "\\echo", "\\ef", "\\encoding",
"\\f", "\\g", "\\h", "\\help", "\\H", "\\i", "\\l",
*************** psql_completion(char *text, int start, i
*** 2680,2685 ****
--- 2680,2687 ----
else if (strncmp(prev_wd, "\\di", strlen("\\di")) == 0)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_indexes, NULL);
+ else if (strncmp(prev_wd, "\\dL", strlen("\\dL")) == 0)
+ COMPLETE_WITH_QUERY(Query_for_list_of_languages);
else if (strncmp(prev_wd, "\\dn", strlen("\\dn")) == 0)
COMPLETE_WITH_QUERY(Query_for_list_of_schemas);
else if (strncmp(prev_wd, "\\dp", strlen("\\dp")) == 0
On Wed, Jan 19, 2011 at 11:19 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote:
On Wed, Jan 19, 2011 at 9:09 PM, Robert Haas <robertmhaas@gmail.com> wrote:
This patch doesn't seem terribly consistent to me - we show the name
of the call handler and the name of the validator, but for the inline
handler we just indicate whether there is one or not. That seems like
something that we should make consistent, and my vote is to show the
name in all cases.OK, changed. I've shuffled the columns slightly so that handlers and
Validator columns are next to each other.Also, I'm wondering whether the System Language column be renamed to
Internal Language, for consistency with the terminology used here:http://www.postgresql.org/docs/current/static/catalog-pg-language.html
Fixed, updated patch attached. Though, reading the description of
lanispl on that page, I wonder if "user-defined language" correctly
describes plpgsql these days, which comes installed by default.
OK, committed.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company