Re: 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]. 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].

> 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'

[2] For example, the command "droplang --list" only prints out "Name"
and "Trusted?" columns.

On Sun, Nov 21, 2010 at 8:18 PM, Josh Kupershmidt <schmiddy(at)gmail(dot)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(at)gmail(dot)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(at)gmail(dot)com> wrote:
> On Sun, Nov 21, 2010 at 8:39 PM, Robert Haas <robertmhaas(at)gmail(dot)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

On Sun, Jan 16, 2011 at 02:26, Andreas Karlsson <andreas(at)proxel(dot)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(at)hagander(dot)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(at)proxel(dot)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(at)gmail(dot)com> wrote:
> On Sun, Jan 16, 2011 at 7:04 AM, Magnus Hagander <magnus(at)hagander(dot)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(at)gmail(dot)com> wrote:
> On Sun, Jan 16, 2011 at 8:52 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> On Sun, Jan 16, 2011 at 7:04 AM, Magnus Hagander <magnus(at)hagander(dot)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(at)gmail(dot)com> wrote:
> On Sun, Jan 16, 2011 at 10:40 PM, Josh Kupershmidt <schmiddy(at)gmail(dot)com> wrote:
>> On Sun, Jan 16, 2011 at 8:52 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>> On Sun, Jan 16, 2011 at 7:04 AM, Magnus Hagander <magnus(at)hagander(dot)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(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)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(at)momjian(dot)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(at)proxel(dot)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(at)proxel(dot)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(at)proxel(dot)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

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(at)proxel(dot)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

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(at)proxel(dot)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(at)gmail(dot)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

On Wed, Jan 19, 2011 at 11:19 PM, Josh Kupershmidt <schmiddy(at)gmail(dot)com> wrote:
> On Wed, Jan 19, 2011 at 9:09 PM, Robert Haas <robertmhaas(at)gmail(dot)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