Thread: 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.
Attachment
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
Attachment
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 languagesProcedural Language | Owner | Trusted
---------------------+-------+---------c | josh | finternal | josh | fplpgsql
| josh | tsql | 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
Attachment
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
Attachment
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
Attachment
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