Thread: psql with "Function Type" in \df
Folks, Here's a patch that adds a "Function Type" column to \df while removing the now-redundant \da. 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 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Attachment
David Fetter <david@fetter.org> writes:
> Here's a patch that adds a "Function Type" column to \df while
> removing the now-redundant \da.
Removing \da altogether was nowhere in the consensus, or even in the
discussion AFAIR. Also, what is the point of using single-letter
type codes when you've made the column header about as verbose as
it could get? I'd go for something like
Type
windowaggtriggernormal
Or we could spell out "aggregate", but that makes the column a
couple of characters wider ...
regards, tom lane
-----BEGIN PGP SIGNED MESSAGE----- Hash: RIPEMD160 > Here's a patch that adds a "Function Type" column to \df while > removing the now-redundant \da. 1. How does it make it redundant - is there a way to view all aggregates with \df now? 2. Even if the above is satisfied, I think we need a little more discussion before completely removing a now-functioning backslash command. Other than that, +1 ;) - -- Greg Sabino Mullane greg@turnstep.com End Point Corporation PGP Key: 0x14964AC8 200904131945 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -----BEGIN PGP SIGNATURE----- iEYEAREDAAYFAknjzrgACgkQvJuQZxSWSsjoWQCfSNxVbmL85Z6FDMQOu8rmsHxh wuwAn1t0pmQ8cqI/e3m+3eADi7cMTGOm =wKRy -----END PGP SIGNATURE-----
On Mon, Apr 13, 2009 at 07:24:31PM -0400, Tom Lane wrote: > David Fetter <david@fetter.org> writes: > > Here's a patch that adds a "Function Type" column to \df while > > removing the now-redundant \da. > > Removing \da altogether was nowhere in the consensus, or even in the > discussion AFAIR. It's back. > Also, what is the point of using single-letter type codes when > you've made the column header about as verbose as it could get? I'd > go for something like > > Type > > window > agg > trigger > normal > > Or we could spell out "aggregate", but that makes the column a > couple of characters wider ... Done. I've also added \df[antw], which lets people narrow their search. 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 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Attachment
David Fetter <david@fetter.org> writes:
> On Mon, Apr 13, 2009 at 07:24:31PM -0400, Tom Lane wrote:
>> I'd go for something like
>>
>> Type
>>
>> window
>> agg
>> trigger
>> normal
>>
>> Or we could spell out "aggregate", but that makes the column a
>> couple of characters wider ...
> Done.
I had a second thought about that: presumably we should make the
function type names translatable. If we do that, it might be better
to make the aggregate case be "aggregate" and take the width hit.
Otherwise translators are going to be puzzled when they come across
"agg" as a translatable phrase.
Or maybe I'm overthinking that problem. Comments from anyone who
actually does translations?
> I've also added \df[antw], which lets people narrow their search.
Why didn't you make it work like \d[tisv], ie allow more than one
letter? If you're going to be inventing new features at this late
date, they should at least work like the adjacent precedent.
regards, tom lane
On Tue, Apr 14, 2009 at 12:35:21PM -0400, Tom Lane wrote: > David Fetter <david@fetter.org> writes: > > On Mon, Apr 13, 2009 at 07:24:31PM -0400, Tom Lane wrote: > >> I'd go for something like > >> > >> Type > >> > >> window > >> agg > >> trigger > >> normal > >> > >> Or we could spell out "aggregate", but that makes the column a > >> couple of characters wider ... > > > Done. > > I had a second thought about that: presumably we should make the > function type names translatable. If we do that, it might be better > to make the aggregate case be "aggregate" and take the width hit. > Otherwise translators are going to be puzzled when they come across > "agg" as a translatable phrase. OK > Or maybe I'm overthinking that problem. Comments from anyone who > actually does translations? > > > I've also added \df[antw], which lets people narrow their search. > > Why didn't you make it work like \d[tisv], ie allow more than one > letter? If you're going to be inventing new features at this late > date, they should at least work like the adjacent precedent. Because I didn't think of it. Lemme see about that :) 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 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Tom Lane wrote: > I had a second thought about that: presumably we should make the > function type names translatable. If we do that, it might be better > to make the aggregate case be "aggregate" and take the width hit. > Otherwise translators are going to be puzzled when they come across > "agg" as a translatable phrase. I think it's good to have them translatable. As for using "aggregate" instead of "agg" I don't think it's that great an idea. If you need to notify translators that "agg" stands for "aggregate", add a /* translator: */ comment. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Tue, Apr 14, 2009 at 02:52:32PM -0400, Alvaro Herrera wrote: > Tom Lane wrote: > > > I had a second thought about that: presumably we should make the > > function type names translatable. If we do that, it might be > > better to make the aggregate case be "aggregate" and take the > > width hit. Otherwise translators are going to be puzzled when > > they come across "agg" as a translatable phrase. > > I think it's good to have them translatable. As for using > "aggregate" instead of "agg" I don't think it's that great an idea. > If you need to notify translators that "agg" stands for "aggregate", > add a /* translator: */ comment. Where would I add that? First mention, each time, or...? Cheers, David (reworking patch per suggestions) -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
David Fetter wrote: > On Tue, Apr 14, 2009 at 02:52:32PM -0400, Alvaro Herrera wrote: > > I think it's good to have them translatable. As for using > > "aggregate" instead of "agg" I don't think it's that great an idea. > > If you need to notify translators that "agg" stands for "aggregate", > > add a /* translator: */ comment. > > Where would I add that? First mention, each time, or...? Is there more than one mention of "agg"? -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
On Tue, Apr 14, 2009 at 03:04:55PM -0400, Alvaro Herrera wrote: > David Fetter wrote: > > On Tue, Apr 14, 2009 at 02:52:32PM -0400, Alvaro Herrera wrote: > > > > I think it's good to have them translatable. As for using > > > "aggregate" instead of "agg" I don't think it's that great an > > > idea. If you need to notify translators that "agg" stands for > > > "aggregate", add a /* translator: */ comment. > > > > Where would I add that? First mention, each time, or...? > > Is there more than one mention of "agg"? It's in 3 branches in describe.c. 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 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
On Tue, Apr 14, 2009 at 02:52:32PM -0400, Alvaro Herrera wrote: > Tom Lane wrote: > > > I had a second thought about that: presumably we should make the > > function type names translatable. If we do that, it might be better > > to make the aggregate case be "aggregate" and take the width hit. > > Otherwise translators are going to be puzzled when they come across > > "agg" as a translatable phrase. > > I think it's good to have them translatable. As for using "aggregate" > instead of "agg" I don't think it's that great an idea. If you need to > notify translators that "agg" stands for "aggregate", add a > /* translator: */ comment. Here's the next revision :) 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 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Attachment
On Tue, Apr 14, 2009 at 03:18:06PM -0700, David Fetter wrote: > On Tue, Apr 14, 2009 at 02:52:32PM -0400, Alvaro Herrera wrote: > > Tom Lane wrote: > > > > > I had a second thought about that: presumably we should make the > > > function type names translatable. If we do that, it might be better > > > to make the aggregate case be "aggregate" and take the width hit. > > > Otherwise translators are going to be puzzled when they come across > > > "agg" as a translatable phrase. > > > > I think it's good to have them translatable. As for using "aggregate" > > instead of "agg" I don't think it's that great an idea. If you need to > > notify translators that "agg" stands for "aggregate", add a > > /* translator: */ comment. > > Here's the next revision :) This time, with less-Byzantine logic, and no egregious whitespace changes. :) 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 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Attachment
David Fetter wrote: > > > I think it's good to have them translatable. As for using "aggregate" > > > instead of "agg" I don't think it's that great an idea. If you need to > > > notify translators that "agg" stands for "aggregate", add a > > > /* translator: */ comment. The "translator: " comment needs to be in the line just above the string. Right now the PO file looks like this: #: describe.c:243 describe.c:288 describe.c:304 msgid "agg" msgstr "" #. translator: "agg" is short for "aggregate" #: describe.c:244 msgid "window" msgstr "" Note really what we want ... -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Wed, Apr 15, 2009 at 08:54:10PM -0400, Alvaro Herrera wrote: > David Fetter wrote: > > > > > I think it's good to have them translatable. As for using "aggregate" > > > > instead of "agg" I don't think it's that great an idea. If you need to > > > > notify translators that "agg" stands for "aggregate", add a > > > > /* translator: */ comment. > > The "translator: " comment needs to be in the line just above the > string. Right now the PO file looks like this: > > #: describe.c:243 describe.c:288 describe.c:304 > msgid "agg" > msgstr "" > > #. translator: "agg" is short for "aggregate" > #: describe.c:244 > msgid "window" > msgstr "" > > > Note really what we want ... Is this any better? 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 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Attachment
David Fetter wrote: > On Wed, Apr 15, 2009 at 08:54:10PM -0400, Alvaro Herrera wrote: > > The "translator: " comment needs to be in the line just above the > > string. > > Is this any better? Yeah, this one is good (as far as this very minor detail is concerned anyway) -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
David Fetter wrote:
> Is this any better?
So what happens if I do \dfaQ? It should throw an error, yes?
This help line:
> + fprintf(output, _(" \\df[S+] [PATTERN] list functions. Add a, n, t, w for aggregate, normal, trigger,
window\n"));
needs shortening to below 80 chars (or maybe split it in two lines.
Just make sure they are a single translation item).
It also seems like we're missing tab completion support for this.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Fri, Apr 17, 2009 at 04:42:31PM -0400, Alvaro Herrera wrote:
> David Fetter wrote:
>
> > Is this any better?
>
> So what happens if I do \dfaQ? It should throw an error, yes?
>
> This help line:
>
> > + fprintf(output, _(" \\df[S+] [PATTERN] list functions. Add a, n, t, w for aggregate, normal, trigger,
window\n"));
>
> needs shortening to below 80 chars (or maybe split it in two lines.
> Just make sure they are a single translation item).
OK.
> It also seems like we're missing tab completion support for this.
Oops. Working on that now.
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
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
On Fri, Apr 17, 2009 at 04:42:31PM -0400, Alvaro Herrera wrote:
> David Fetter wrote:
>
> > Is this any better?
>
> So what happens if I do \dfaQ? It should throw an error, yes?
Interesting question.
>
> This help line:
>
> > + fprintf(output, _(" \\df[S+] [PATTERN] list functions. Add a, n, t, w for aggregate, normal, trigger,
window\n"));
>
> needs shortening to below 80 chars (or maybe split it in two lines.
> Just make sure they are a single translation item).
>
> It also seems like we're missing tab completion support for this.
This is another interesting question. I notice that the tab
completion doesn't support things like \dit.
Should I add that as a separate patch?
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
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
David Fetter wrote: > On Wed, Apr 15, 2009 at 08:54:10PM -0400, Alvaro Herrera wrote: > > David Fetter wrote: > > > > > > > I think it's good to have them translatable. As for using "aggregate" > > > > > instead of "agg" I don't think it's that great an idea. If you need to > > > > > notify translators that "agg" stands for "aggregate", add a > > > > > /* translator: */ comment. > > > > The "translator: " comment needs to be in the line just above the > > string. Right now the PO file looks like this: > > > > #: describe.c:243 describe.c:288 describe.c:304 > > msgid "agg" > > msgstr "" > > > > #. translator: "agg" is short for "aggregate" > > #: describe.c:244 > > msgid "window" > > msgstr "" > > > > > > Note really what we want ... > > Is this any better? Modified patch applied. I modified the \? display to be two lines: \df[S+] [PATTERN] list functions \df[antwS+] [PATTERN] list only agg/normal/trigger/window functions and adjusted the code and documentation a bit. I removed the release.sgml patch because that part will be done when the release notes are updated and I was worried it might confuse things to add it now. Thanks, the output looks very good. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: doc/src/sgml/ref/psql-ref.sgml =================================================================== RCS file: /cvsroot/pgsql/doc/src/sgml/ref/psql-ref.sgml,v retrieving revision 1.223 diff -c -c -r1.223 psql-ref.sgml *** doc/src/sgml/ref/psql-ref.sgml 8 Apr 2009 22:29:30 -0000 1.223 --- doc/src/sgml/ref/psql-ref.sgml 21 Apr 2009 15:45:56 -0000 *************** *** 1039,1056 **** <varlistentry> <term><literal>\df[S+] [ <replaceable class="parameter">pattern</replaceable> ]</literal></term> <listitem> <para> ! Lists available functions, together with their argument and ! return types. If <replaceable ! class="parameter">pattern</replaceable> ! is specified, only functions whose names match the pattern are shown. ! If the form <literal>\df+</literal> is used, additional information about ! each function, including volatility, language, source code and description, is shown. ! By default, only user-created objects are shown; supply a ! pattern or the <literal>S</literal> modifier to include system ! objects. </para> <note> --- 1039,1060 ---- <varlistentry> <term><literal>\df[S+] [ <replaceable class="parameter">pattern</replaceable> ]</literal></term> + <term><literal>\df[antw][S+] [ <replaceable class="parameter">pattern</replaceable> ]</literal></term> <listitem> <para> ! Lists available functions, together with their arguments, ! return types, and their function types: 'agg' (aggregate), ! 'normal', 'trigger', and 'window'. To display only functions ! of a specific type, use the corresponding letters <literal>a</>, ! <literal>n</>, <literal>t</>, or <literal>w</>. If <replaceable ! class="parameter">pattern</replaceable> is specified, only ! functions whose names match the pattern are shown. If the ! form <literal>\df+</literal> is used, additional information ! about each function, including volatility, language, source ! code and description, is shown. By default, only user-created ! objects are shown; supply a pattern or the <literal>S</literal> ! modifier to include system objects. </para> <note> *************** *** 1064,1070 **** </listitem> </varlistentry> - <varlistentry> <term><literal>\dF[+] [ <replaceable class="parameter">pattern</replaceable> ]</literal></term> <listitem> --- 1068,1073 ---- Index: src/bin/psql/command.c =================================================================== RCS file: /cvsroot/pgsql/src/bin/psql/command.c,v retrieving revision 1.204 diff -c -c -r1.204 command.c *** src/bin/psql/command.c 25 Mar 2009 13:07:26 -0000 1.204 --- src/bin/psql/command.c 21 Apr 2009 15:45:56 -0000 *************** *** 365,372 **** case 'D': success = listDomains(pattern, show_system); break; ! case 'f': ! success = describeFunctions(pattern, show_verbose, show_system); break; case 'g': /* no longer distinct from \du */ --- 365,386 ---- case 'D': success = listDomains(pattern, show_system); break; ! case 'f': /* function subsystem */ ! switch (cmd[2]) ! { ! case '\0': ! case '+': ! case 'S': ! case 'a': ! case 'n': ! case 't': ! case 'w': ! success = describeFunctions(&cmd[2], pattern, show_verbose, show_system); ! break; ! default: ! status = PSQL_CMD_UNKNOWN; ! break; ! } break; case 'g': /* no longer distinct from \du */ Index: src/bin/psql/describe.c =================================================================== RCS file: /cvsroot/pgsql/src/bin/psql/describe.c,v retrieving revision 1.208 diff -c -c -r1.208 describe.c *** src/bin/psql/describe.c 8 Apr 2009 22:29:30 -0000 1.208 --- src/bin/psql/describe.c 21 Apr 2009 15:45:56 -0000 *************** *** 183,197 **** /* \df ! * Takes an optional regexp to select particular functions */ bool ! describeFunctions(const char *pattern, bool verbose, bool showSystem) { PQExpBufferData buf; PGresult *res; printQueryOpt myopt = pset.popt; initPQExpBuffer(&buf); printfPQExpBuffer(&buf, --- 183,225 ---- /* \df ! * Takes an optional regexp to select particular functions. ! * ! * As with \d, you can specify the kinds of functions you want: ! * ! * a for aggregates ! * n for normal ! * t for trigger ! * w for window ! * ! * and you can mix and match these in any order. */ bool ! describeFunctions(const char *functypes, const char *pattern, bool verbose, bool showSystem) { + bool showAggregate = strchr(functypes, 'a') != NULL; + bool showNormal = strchr(functypes, 'n') != NULL; + bool showTrigger = strchr(functypes, 't') != NULL; + bool showWindow = strchr(functypes, 'w') != NULL; + PQExpBufferData buf; PGresult *res; printQueryOpt myopt = pset.popt; + if (showWindow && pset.sversion < 80400) + { + fprintf(stderr, _("\\df does not take a \"w\" decorator in %d.%d.\n"), + pset.sversion / 10000, (pset.sversion / 100) % 100); + return true; + } + + if (!showAggregate && !showNormal && !showTrigger && !showWindow) + { + showAggregate = showNormal = showTrigger = true; + if (pset.sversion >= 80400) + showWindow = true; + } + initPQExpBuffer(&buf); printfPQExpBuffer(&buf, *************** *** 203,211 **** if (pset.sversion >= 80400) appendPQExpBuffer(&buf, " pg_catalog.pg_get_function_result(p.oid) as \"%s\",\n" ! " pg_catalog.pg_get_function_arguments(p.oid) as \"%s\"", gettext_noop("Result data type"), ! gettext_noop("Argument data types")); else if (pset.sversion >= 80100) appendPQExpBuffer(&buf, " CASE WHEN p.proretset THEN 'SETOF ' ELSE '' END ||\n" --- 231,251 ---- if (pset.sversion >= 80400) appendPQExpBuffer(&buf, " pg_catalog.pg_get_function_result(p.oid) as \"%s\",\n" ! " pg_catalog.pg_get_function_arguments(p.oid) as \"%s\",\n" ! " CASE\n" ! " WHEN p.proisagg THEN '%s'\n" ! " WHEN p.proiswindow THEN '%s'\n" ! " WHEN pg_catalog.pg_get_function_result(p.oid) = 'trigger' THEN '%s'\n" ! " ELSE '%s'\n" ! "END as \"%s\"", gettext_noop("Result data type"), ! gettext_noop("Argument data types"), ! /* translator: "agg" is short for "aggregate" */ ! gettext_noop("agg"), ! gettext_noop("window"), ! gettext_noop("trigger"), ! gettext_noop("normal"), ! gettext_noop("Type")); else if (pset.sversion >= 80100) appendPQExpBuffer(&buf, " CASE WHEN p.proretset THEN 'SETOF ' ELSE '' END ||\n" *************** *** 238,253 **** " FROM\n" " pg_catalog.generate_series(0, pg_catalog.array_upper(p.proargtypes, 1)) AS s(i)\n" " ), ', ')\n" " END AS \"%s\"", gettext_noop("Result data type"), ! gettext_noop("Argument data types")); else appendPQExpBuffer(&buf, " CASE WHEN p.proretset THEN 'SETOF ' ELSE '' END ||\n" " pg_catalog.format_type(p.prorettype, NULL) as \"%s\",\n" ! " pg_catalog.oidvectortypes(p.proargtypes) as \"%s\"", gettext_noop("Result data type"), ! gettext_noop("Argument data types")); if (verbose) appendPQExpBuffer(&buf, --- 278,313 ---- " FROM\n" " pg_catalog.generate_series(0, pg_catalog.array_upper(p.proargtypes, 1)) AS s(i)\n" " ), ', ')\n" + " END AS \"%s\",\n" + " CASE\n" + " WHEN p.proisagg THEN '%s'\n" + " WHEN 'trigger' = pg_catalog.format_type(p.prorettype, NULL) THEN '%s'\n" + " ELSE '%s'\n" " END AS \"%s\"", gettext_noop("Result data type"), ! gettext_noop("Argument data types"), ! /* translator: "agg" is short for "aggregate" */ ! gettext_noop("agg"), ! gettext_noop("trigger"), ! gettext_noop("normal"), ! gettext_noop("Type")); else appendPQExpBuffer(&buf, " CASE WHEN p.proretset THEN 'SETOF ' ELSE '' END ||\n" " pg_catalog.format_type(p.prorettype, NULL) as \"%s\",\n" ! " pg_catalog.oidvectortypes(p.proargtypes) as \"%s\",\n" ! " CASE\n" ! " WHEN p.proisagg THEN '%s'\n" ! " WHEN 'trigger' = pg_catalog.format_type(p.prorettype, NULL) THEN '%s'\n" ! " ELSE '%s'\n" ! " END AS \"%s\"", gettext_noop("Result data type"), ! gettext_noop("Argument data types"), ! /* translator: "agg" is short for "aggregate" */ ! gettext_noop("agg"), ! gettext_noop("trigger"), ! gettext_noop("normal"), ! gettext_noop("Type")); if (verbose) appendPQExpBuffer(&buf, *************** *** 274,289 **** appendPQExpBuffer(&buf, " LEFT JOIN pg_catalog.pg_language l ON l.oid = p.prolang\n"); ! appendPQExpBuffer(&buf, "WHERE NOT p.proisagg\n"); if (!showSystem && !pattern) appendPQExpBuffer(&buf, " AND n.nspname <> 'pg_catalog'\n" " AND n.nspname <> 'information_schema'\n"); - processSQLNamePattern(pset.db, &buf, pattern, true, false, - "n.nspname", "p.proname", NULL, - "pg_catalog.pg_function_is_visible(p.oid)"); - appendPQExpBuffer(&buf, "ORDER BY 1, 2, 4;"); res = PSQLexec(buf.data, false); --- 334,396 ---- appendPQExpBuffer(&buf, " LEFT JOIN pg_catalog.pg_language l ON l.oid = p.prolang\n"); ! processSQLNamePattern(pset.db, &buf, pattern, false, true, ! "n.nspname", "p.proname", NULL, ! "pg_catalog.pg_function_is_visible(p.oid)"); ! ! if (showNormal && showAggregate && showTrigger && showWindow) ! /* Do nothing */; ! else if (showNormal) ! { ! if (!showWindow && pset.sversion >= 80400) ! appendPQExpBuffer(&buf, " AND NOT p.proiswindow\n"); ! if (!showAggregate) ! appendPQExpBuffer(&buf, " AND NOT p.proisagg\n"); ! if (!showTrigger) ! { ! if (pset.sversion >= 80400) ! appendPQExpBuffer(&buf, ! " AND pg_catalog.pg_get_function_result(p.oid) <> 'trigger'\n"); ! else ! appendPQExpBuffer(&buf, ! " AND pg_catalog.format_type(p.prorettype, NULL) <> 'trigger'\n"); ! } ! } ! else ! { ! bool needs_or = false; ! ! appendPQExpBuffer(&buf, " AND (\n "); ! if (showAggregate) ! { ! appendPQExpBuffer(&buf,"p.proisagg\n"); ! needs_or = true; ! } ! if (showTrigger) ! { ! if (needs_or) ! appendPQExpBuffer(&buf, " OR "); ! if (pset.sversion >= 80400) ! appendPQExpBuffer(&buf, ! "pg_catalog.pg_get_function_result(p.oid) = 'trigger'\n"); ! else ! appendPQExpBuffer(&buf, ! "'trigger' <> pg_catalog.format_type(p.prorettype, NULL)\n"); ! needs_or = true; ! } ! if (showWindow) ! { ! if (needs_or) ! appendPQExpBuffer(&buf, " OR "); ! appendPQExpBuffer(&buf, "p.proiswindow\n"); ! } ! appendPQExpBuffer(&buf, " )\n"); ! } if (!showSystem && !pattern) appendPQExpBuffer(&buf, " AND n.nspname <> 'pg_catalog'\n" " AND n.nspname <> 'information_schema'\n"); appendPQExpBuffer(&buf, "ORDER BY 1, 2, 4;"); res = PSQLexec(buf.data, false); Index: src/bin/psql/describe.h =================================================================== RCS file: /cvsroot/pgsql/src/bin/psql/describe.h,v retrieving revision 1.39 diff -c -c -r1.39 describe.h *** src/bin/psql/describe.h 20 Jan 2009 02:13:42 -0000 1.39 --- src/bin/psql/describe.h 21 Apr 2009 15:45:56 -0000 *************** *** 15,22 **** /* \db */ extern bool describeTablespaces(const char *pattern, bool verbose); ! /* \df */ ! extern bool describeFunctions(const char *pattern, bool verbose, bool showSystem); /* \dT */ extern bool describeTypes(const char *pattern, bool verbose, bool showSystem); --- 15,22 ---- /* \db */ extern bool describeTablespaces(const char *pattern, bool verbose); ! /* \df, \dfa, \dfn, \dft, \dfw, etc. */ ! extern bool describeFunctions(const char *functypes, const char *pattern, bool verbose, bool showSystem); /* \dT */ extern bool describeTypes(const char *pattern, bool verbose, bool showSystem); Index: src/bin/psql/help.c =================================================================== RCS file: /cvsroot/pgsql/src/bin/psql/help.c,v retrieving revision 1.146 diff -c -c -r1.146 help.c *** src/bin/psql/help.c 11 Apr 2009 14:11:21 -0000 1.146 --- src/bin/psql/help.c 21 Apr 2009 15:45:56 -0000 *************** *** 196,202 **** fprintf(output, _(" (options: S = show system objects, + = additional detail)\n")); fprintf(output, _(" \\d[S+] list tables, views, and sequences\n")); fprintf(output, _(" \\d[S+] NAME describe table, view, sequence, or index\n")); ! fprintf(output, _(" \\da[S] [PATTERN] list aggregate functions\n")); fprintf(output, _(" \\db[+] [PATTERN] list tablespaces\n")); fprintf(output, _(" \\dc[S] [PATTERN] list conversions\n")); fprintf(output, _(" \\dC [PATTERN] list casts\n")); --- 196,202 ---- fprintf(output, _(" (options: S = show system objects, + = additional detail)\n")); fprintf(output, _(" \\d[S+] list tables, views, and sequences\n")); fprintf(output, _(" \\d[S+] NAME describe table, view, sequence, or index\n")); ! fprintf(output, _(" \\da[+] [PATTERN] list aggregates\n")); fprintf(output, _(" \\db[+] [PATTERN] list tablespaces\n")); fprintf(output, _(" \\dc[S] [PATTERN] list conversions\n")); fprintf(output, _(" \\dC [PATTERN] list casts\n")); *************** *** 206,211 **** --- 206,212 ---- fprintf(output, _(" \\deu[+] [PATTERN] list user mappings\n")); fprintf(output, _(" \\dew[+] [PATTERN] list foreign-data wrappers\n")); fprintf(output, _(" \\df[S+] [PATTERN] list functions\n")); + fprintf(output, _(" \\df[antwS+] [PATTERN] list only agg/normal/trigger/window functions\n")); fprintf(output, _(" \\dF[+] [PATTERN] list text search configurations\n")); fprintf(output, _(" \\dFd[+] [PATTERN] list text search dictionaries\n")); fprintf(output, _(" \\dFp[+] [PATTERN] list text search parsers\n")); Index: src/test/regress/expected/polymorphism.out =================================================================== RCS file: /cvsroot/pgsql/src/test/regress/expected/polymorphism.out,v retrieving revision 1.19 diff -c -c -r1.19 polymorphism.out *** src/test/regress/expected/polymorphism.out 9 Feb 2009 21:18:28 -0000 1.19 --- src/test/regress/expected/polymorphism.out 21 Apr 2009 15:45:57 -0000 *************** *** 838,846 **** -- verify it lists properly \df dfunc List of functions ! Schema | Name | Result data type | Argument data types ! --------+-------+------------------+----------------------------------------------------------- ! public | dfunc | integer | a integer DEFAULT 1, OUT sum integer, b integer DEFAULT 2 (1 row) drop function dfunc(int, int); --- 838,846 ---- -- verify it lists properly \df dfunc List of functions ! Schema | Name | Result data type | Argument data types | Type ! --------+-------+------------------+-----------------------------------------------------------+-------- ! public | dfunc | integer | a integer DEFAULT 1, OUT sum integer, b integer DEFAULT 2 | normal (1 row) drop function dfunc(int, int); *************** *** 1006,1014 **** HINT: Use DROP FUNCTION first. \df dfunc List of functions ! Schema | Name | Result data type | Argument data types ! --------+-------+------------------+------------------------------------------------- ! public | dfunc | integer | VARIADIC a integer[] DEFAULT ARRAY[]::integer[] (1 row) drop function dfunc(a variadic int[]); --- 1006,1014 ---- HINT: Use DROP FUNCTION first. \df dfunc List of functions ! Schema | Name | Result data type | Argument data types | Type ! --------+-------+------------------+-------------------------------------------------+-------- ! public | dfunc | integer | VARIADIC a integer[] DEFAULT ARRAY[]::integer[] | normal (1 row) drop function dfunc(a variadic int[]);
Bruce Momjian <bruce@momjian.us> wrote:
> \df[S+] [PATTERN] list functions
> \df[antwS+] [PATTERN] list only agg/normal/trigger/window functions
Shouldn't that second line have some curly braces? Like maybe:
\df{antw}[S+] [PATTERN] list only agg/normal/trigger/window functions
Technically, it should probably be even more verbose, but this might
be adequate.
-Kevin
Kevin Grittner wrote:
> Bruce Momjian <bruce@momjian.us> wrote:
>
> > \df[S+] [PATTERN] list functions
> > \df[antwS+] [PATTERN] list only agg/normal/trigger/window functions
>
> Shouldn't that second line have some curly braces? Like maybe:
>
> \df{antw}[S+] [PATTERN] list only agg/normal/trigger/window functions
>
> Technically, it should probably be even more verbose, but this might
> be adequate.
I suggested this to Bruce over IM:
\df[antw][S+] list [only agg/normal/trigger/window] functions
(one line only, removing the second redundant line). This seems
clea[nr]er to me. Bruce says it would confuse users. But really, if
you can understand the [S+] stuff you should be able to get the [antw]
stuff too, right?
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
On Tue, 2009-04-21 at 12:19 -0400, Alvaro Herrera wrote: > I suggested this to Bruce over IM: > > \df[antw][S+] list [only agg/normal/trigger/window] functions > > (one line only, removing the second redundant line). This seems > clea[nr]er to me. Bruce says it would confuse users. But really, if > you can understand the [S+] stuff you should be able to get the [antw] > stuff too, right? > +1 to Alvaro. Joshua D. Drake > -- > Alvaro Herrera http://www.CommandPrompt.com/ > The PostgreSQL Company - Command Prompt, Inc. > -- PostgreSQL - XMPP: jdrake@jabber.postgresql.org Consulting, Development, Support, Training 503-667-4564 - http://www.commandprompt.com/ The PostgreSQL Company, serving since 1997
Kevin Grittner wrote:
> Bruce Momjian <bruce@momjian.us> wrote:
>
> > \df[S+] [PATTERN] list functions
> > \df[antwS+] [PATTERN] list only agg/normal/trigger/window functions
>
> Shouldn't that second line have some curly braces? Like maybe:
>
> \df{antw}[S+] [PATTERN] list only agg/normal/trigger/window functions
>
> Technically, it should probably be even more verbose, but this might
> be adequate.
Agreed. The problem is I don't see curly braces used anywhere in \?,
but now that I look at it it is just because there is no need for them.
How about this:
\\df{antw}[S+] [PATRN] list only agg/normal/trigger/window functions
In a way though, they are really still optional, they are just not
optional for this specific line.
-- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB
http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
On Tue, Apr 21, 2009 at 09:26:13AM -0700, Joshua D. Drake wrote: > On Tue, 2009-04-21 at 12:19 -0400, Alvaro Herrera wrote: > > > I suggested this to Bruce over IM: > > > > \df[antw][S+] list [only agg/normal/trigger/window] functions > > > > (one line only, removing the second redundant line). This seems > > clea[nr]er to me. Bruce says it would confuse users. But really, if > > you can understand the [S+] stuff you should be able to get the [antw] > > stuff too, right? > > > > +1 to Alvaro. Now psql allows you to mix all of the options freely (scary math word ahead) in an idempotent way. In other words, this works: \df++aannSw Expressed as a regex, it's \\df[antwS+]* 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 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Alvaro Herrera <alvherre@commandprompt.com> wrote: > \df[antw][S+] list [only agg/normal/trigger/window] functions > > (one line only, removing the second redundant line). This seems > clea[nr]er to me. Bruce says it would confuse users. That's clear to me, anyway (if you include PATTERN). I'd be fine with it or the alternative I posted. -Kevin
Bruce Momjian <bruce@momjian.us> wrote:
> The problem is I don't see curly braces used anywhere in \?
I see it in 8.3:
: \d{t|i|s|v|S} [PATTERN] (add "+" for more detail)
: list tables/indexes/sequences/views/system tables
and:
: \pset NAME [VALUE]
: set table output option
: (NAME :=
{format|border|expanded|fieldsep|footer|null|
:
numericlocale|recordsep|tuples_only|title|tableattr|pager})
> In a way though, they are really still optional, they are just not
> optional for this specific line.
Then we should indicate they're not optional on that line, or remove
that line as suggested by Alvaro.
-Kevin
On Tue, Apr 21, 2009 at 09:33:26AM -0700, David Fetter wrote: > On Tue, Apr 21, 2009 at 09:26:13AM -0700, Joshua D. Drake wrote: > > On Tue, 2009-04-21 at 12:19 -0400, Alvaro Herrera wrote: > > > > > I suggested this to Bruce over IM: > > > > > > \df[antw][S+] list [only agg/normal/trigger/window] functions > > > > > > (one line only, removing the second redundant line). This seems > > > clea[nr]er to me. Bruce says it would confuse users. But really, if > > > you can understand the [S+] stuff you should be able to get the [antw] > > > stuff too, right? > > > > > > > +1 to Alvaro. > > Now psql allows you to mix all of the options freely (scary math word > ahead) in an idempotent way. In other words, this works: > > \df++aannSw > > Expressed as a regex, it's \\df[antwS+]* > > Cheers, > David. Oh, and I forgot to send some error-handling and cleanup code per Alvaro. Please find attached. :) 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 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Attachment
David Fetter wrote: > Oh, and I forgot to send some error-handling and cleanup code per > Alvaro. Please find attached. :) Declarations before code please. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> Bruce Momjian <bruce@momjian.us> wrote:
>> The problem is I don't see curly braces used anywhere in \?
> I see it in 8.3:
> : \d{t|i|s|v|S} [PATTERN] (add "+" for more detail)
> : list tables/indexes/sequences/views/system tables
Yeah. The only reason that \d[tisv] doesn't have a similar issue
is that someone has dumbed down its entries to the point of being
flat-out incorrect. I trust we will not be equally bad about \df.
regards, tom lane
On Tue, Apr 21, 2009 at 01:04:44PM -0400, Alvaro Herrera wrote: > David Fetter wrote: > > > Oh, and I forgot to send some error-handling and cleanup code per > > Alvaro. Please find attached. :) > > Declarations before code please. Fixed patch attached. 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 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Attachment
Alvaro Herrera wrote:
> Kevin Grittner wrote:
> > Bruce Momjian <bruce@momjian.us> wrote:
> >
> > > \df[S+] [PATTERN] list functions
> > > \df[antwS+] [PATTERN] list only agg/normal/trigger/window functions
> >
> > Shouldn't that second line have some curly braces? Like maybe:
> >
> > \df{antw}[S+] [PATTERN] list only agg/normal/trigger/window functions
> >
> > Technically, it should probably be even more verbose, but this might
> > be adequate.
>
> I suggested this to Bruce over IM:
>
> \df[antw][S+] list [only agg/normal/trigger/window] functions
>
> (one line only, removing the second redundant line). This seems
> clea[nr]er to me. Bruce says it would confuse users. But really, if
> you can understand the [S+] stuff you should be able to get the [antw]
> stuff too, right?
I merged the entries into one line:
\df[antwS+] [PATTERN] list (only agg/normal/trigger/window) functions
I didn't feel I had room to do [][] like Alvaro suggested. I also went
with parntheses in the description. Are brackets better there?
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/ref/psql-ref.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/ref/psql-ref.sgml,v
retrieving revision 1.224
diff -c -c -r1.224 psql-ref.sgml
*** doc/src/sgml/ref/psql-ref.sgml 21 Apr 2009 15:49:06 -0000 1.224
--- doc/src/sgml/ref/psql-ref.sgml 21 Apr 2009 17:21:56 -0000
***************
*** 1038,1045 ****
<varlistentry>
! <term><literal>\df[S+] [ <replaceable class="parameter">pattern</replaceable> ]</literal></term>
! <term><literal>\df[antw][S+] [ <replaceable class="parameter">pattern</replaceable> ]</literal></term>
<listitem>
<para>
--- 1038,1044 ----
<varlistentry>
! <term><literal>\df[antwS+] [ <replaceable class="parameter">pattern</replaceable> ]</literal></term>
<listitem>
<para>
Index: src/bin/psql/help.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/help.c,v
retrieving revision 1.147
diff -c -c -r1.147 help.c
*** src/bin/psql/help.c 21 Apr 2009 15:49:06 -0000 1.147
--- src/bin/psql/help.c 21 Apr 2009 17:21:57 -0000
***************
*** 205,212 ****
fprintf(output, _(" \\des[+] [PATTERN] list foreign servers\n"));
fprintf(output, _(" \\deu[+] [PATTERN] list user mappings\n"));
fprintf(output, _(" \\dew[+] [PATTERN] list foreign-data wrappers\n"));
! fprintf(output, _(" \\df[S+] [PATTERN] list functions\n"));
! fprintf(output, _(" \\df[antwS+] [PATTERN] list only agg/normal/trigger/window functions\n"));
fprintf(output, _(" \\dF[+] [PATTERN] list text search configurations\n"));
fprintf(output, _(" \\dFd[+] [PATTERN] list text search dictionaries\n"));
fprintf(output, _(" \\dFp[+] [PATTERN] list text search parsers\n"));
--- 205,211 ----
fprintf(output, _(" \\des[+] [PATTERN] list foreign servers\n"));
fprintf(output, _(" \\deu[+] [PATTERN] list user mappings\n"));
fprintf(output, _(" \\dew[+] [PATTERN] list foreign-data wrappers\n"));
! fprintf(output, _(" \\df[antwS+] [PATTERN] list (only agg/normal/trigger/window) functions\n"));
fprintf(output, _(" \\dF[+] [PATTERN] list text search configurations\n"));
fprintf(output, _(" \\dFd[+] [PATTERN] list text search dictionaries\n"));
fprintf(output, _(" \\dFp[+] [PATTERN] list text search parsers\n"));
David Fetter wrote: > On Tue, Apr 21, 2009 at 01:04:44PM -0400, Alvaro Herrera wrote: > > David Fetter wrote: > > > > > Oh, and I forgot to send some error-handling and cleanup code per > > > Alvaro. Please find attached. :) > > > > Declarations before code please. > > Fixed patch attached. Applied. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Tue, Apr 21, 2009 at 01:26:33PM -0400, Bruce Momjian wrote: [...] > I merged the entries into one line: > > \df[antwS+] [PATTERN] list (only agg/normal/trigger/window) functions > > I didn't feel I had room to do [][] like Alvaro suggested. I also went > with parntheses in the description. Are brackets better there? FWIW brackets let the reader see better the correspondence between left and right. Regards - -- tomás -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFJ7pKUBcgs9XrR2kYRApZUAJ0XA2qER2Lzg3pIeV3giGhRKwzv3ACffd/Z IFsJt3v9F5Xr8qTkyP4fyQY= =N38M -----END PGP SIGNATURE-----
tomas@tuxteam.de wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On Tue, Apr 21, 2009 at 01:26:33PM -0400, Bruce Momjian wrote:
>
> [...]
>
> > I merged the entries into one line:
> >
> > \df[antwS+] [PATTERN] list (only agg/normal/trigger/window) functions
> >
> > I didn't feel I had room to do [][] like Alvaro suggested. I also went
> > with parntheses in the description. Are brackets better there?
>
> FWIW brackets let the reader see better the correspondence between left and
> right.
True, but the problem is that the brackets don't correspond. There are
two brackets on the left, options and "Pattern", and "S+" in the left
bracket is not related to the "only" options. If we had used braces:
\df{antw}[S+] [PATTERN] list {only agg/normal/trigger/window} functions
that would make sense, but we don't have space nor are braces logically
correct for this usage.
-- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB
http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Wed, Apr 22, 2009 at 08:32:20AM -0400, Bruce Momjian wrote: [...] > True, but the problem is that the brackets don't correspond [...] Yes, right. Still, square brackets seem (to me) to provide some visual cue. But I admit that this is already advanced bikeshedding and thus I'll politely bow out :-) Thanks - -- tomás -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFJ7xt+Bcgs9XrR2kYRAk6tAJ9nV7T4QOynxoIoh7xwAuGYkvGdmwCfel3e EDzEvm652zGok2ls3hi7Bi4= =mtST -----END PGP SIGNATURE-----
tomas@tuxteam.de wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On Wed, Apr 22, 2009 at 08:32:20AM -0400, Bruce Momjian wrote: > > [...] > > > True, but the problem is that the brackets don't correspond [...] > > Yes, right. Still, square brackets seem (to me) to provide some visual > cue. But I admit that this is already advanced bikeshedding and thus > I'll politely bow out :-) No problem; we like polish here. ;-) If I can get someone else to say they prefer brackets over parentheses in \? I will make the change. Right now we have: \df[antwS+] [PATTERN] list (only agg/normal/trigger/window) functions With brackets it would be: \df[antwS+] [PATTERN] list [only agg/normal/trigger/window] functions -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> If I can get someone else to say they prefer brackets over parentheses in
> \? I will make the change. Right now we have:
> \df[antwS+] [PATTERN] list (only agg/normal/trigger/window) functions
> With brackets it would be:
> \df[antwS+] [PATTERN] list [only agg/normal/trigger/window] functions
Yes, the brackets in the text match up with the brackets in the syntax,
so I think this looks more sensible.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > If I can get someone else to say they prefer brackets over parentheses in > > \? I will make the change. Right now we have: > > > \df[antwS+] [PATTERN] list (only agg/normal/trigger/window) functions > > > With brackets it would be: > > > \df[antwS+] [PATTERN] list [only agg/normal/trigger/window] functions > > Yes, the brackets in the text match up with the brackets in the syntax, > so I think this looks more sensible. Still, my original proposal was \df[antw][S+]. The extra brackets are obviously redundant, but if we're about providing cues, this is a good cue IMO. It allows the [S+] to match the other lines. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Still, my original proposal was \df[antw][S+]. The extra brackets are
> obviously redundant, but if we're about providing cues, this is a good
> cue IMO. It allows the [S+] to match the other lines.
I'm for that too. Bruce was complaining that it'd make the column
wider, but I don't find that a compelling objection.
regards, tom lane
Tom Lane wrote: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > Still, my original proposal was \df[antw][S+]. The extra brackets are > > obviously redundant, but if we're about providing cues, this is a good > > cue IMO. It allows the [S+] to match the other lines. > > I'm for that too. Bruce was complaining that it'd make the column > wider, but I don't find that a compelling objection. OK, I switch to brackets and added separate brackets around the "antw" options. I changed "PATTERN" to "PATRN" so I would not have to shift over _every_ other \? option to align properly; patch attached and applied. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/bin/psql/help.c =================================================================== RCS file: /cvsroot/pgsql/src/bin/psql/help.c,v retrieving revision 1.148 diff -c -c -r1.148 help.c *** src/bin/psql/help.c 21 Apr 2009 17:23:05 -0000 1.148 --- src/bin/psql/help.c 22 Apr 2009 14:58:16 -0000 *************** *** 205,211 **** fprintf(output, _(" \\des[+] [PATTERN] list foreign servers\n")); fprintf(output, _(" \\deu[+] [PATTERN] list user mappings\n")); fprintf(output, _(" \\dew[+] [PATTERN] list foreign-data wrappers\n")); ! fprintf(output, _(" \\df[antwS+] [PATTERN] list (only agg/normal/trigger/window) functions\n")); fprintf(output, _(" \\dF[+] [PATTERN] list text search configurations\n")); fprintf(output, _(" \\dFd[+] [PATTERN] list text search dictionaries\n")); fprintf(output, _(" \\dFp[+] [PATTERN] list text search parsers\n")); --- 205,211 ---- fprintf(output, _(" \\des[+] [PATTERN] list foreign servers\n")); fprintf(output, _(" \\deu[+] [PATTERN] list user mappings\n")); fprintf(output, _(" \\dew[+] [PATTERN] list foreign-data wrappers\n")); ! fprintf(output, _(" \\df[antw][S+] [PATRN] list [only agg/normal/trigger/window] functions\n")); fprintf(output, _(" \\dF[+] [PATTERN] list text search configurations\n")); fprintf(output, _(" \\dFd[+] [PATTERN] list text search dictionaries\n")); fprintf(output, _(" \\dFp[+] [PATTERN] list text search parsers\n"));
David Fetter wrote: > On Fri, Apr 17, 2009 at 04:42:31PM -0400, Alvaro Herrera wrote: > > It also seems like we're missing tab completion support for this. > > Oops. Working on that now. Any luck with this? -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Mon, Apr 27, 2009 at 10:39:33AM -0400, Alvaro Herrera wrote: > David Fetter wrote: > > On Fri, Apr 17, 2009 at 04:42:31PM -0400, Alvaro Herrera wrote: > > > > It also seems like we're missing tab completion support for this. > > > > Oops. Working on that now. > > Any luck with this? I have a handle on the problem, which is that the tab completion code assumes, wrongly, that it only needs to deal with fixed strings. It's actually been false for some time in the \div case, for example. The S option has shattered the fixed-string assumption. I'm proposing to refactor the backslash handling code so all of it is in one spot with differences in the target list and WHERE clauses depending on whether it's tab completion, S, +, what I'm thinking of as second-level options--the [ivt]* in \d[ivt]* and the [antw]* in \df[antw]*[S+]--and patterns. Does this sound reasonable? 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 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
David Fetter <david@fetter.org> writes:
> I have a handle on the problem, which is that the tab completion code
> assumes, wrongly, that it only needs to deal with fixed strings. It's
> actually been false for some time in the \div case, for example. The
> S option has shattered the fixed-string assumption.
Check.
> I'm proposing to refactor the backslash handling code so all of it is
> in one spot with differences in the target list and WHERE clauses
> depending on whether it's tab completion, S, +, what I'm thinking of
> as second-level options--the [ivt]* in \d[ivt]* and the [antw]* in
> \df[antw]*[S+]--and patterns.
> Does this sound reasonable?
It seems like rather a large change to be making in beta. Can you make
a small patch that fixes the immediate problem, and leave the
refactoring for 8.5?
regards, tom lane
On Mon, Apr 27, 2009 at 01:11:44PM -0400, Tom Lane wrote: > David Fetter <david@fetter.org> writes: > > I have a handle on the problem, which is that the tab completion > > code assumes, wrongly, that it only needs to deal with fixed > > strings. It's actually been false for some time in the \div case, > > for example. The S option has shattered the fixed-string > > assumption. > > Check. > > > I'm proposing to refactor the backslash handling code so all of it > > is in one spot with differences in the target list and WHERE > > clauses depending on whether it's tab completion, S, +, what I'm > > thinking of as second-level options--the [ivt]* in \d[ivt]* and > > the [antw]* in \df[antw]*[S+]--and patterns. > > > Does this sound reasonable? > > It seems like rather a large change to be making in beta. Can you > make a small patch that fixes the immediate problem, and leave the > refactoring for 8.5? The hack I've come up with short of the refactor is to duplicate a lot of the backslash handling code from command.c and describe.c. It doesn't strike me as being all that much less work than the refactor. Is the hack worth doing? What other ways to approach this have I missed? 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 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
David Fetter wrote: > On Mon, Apr 27, 2009 at 01:11:44PM -0400, Tom Lane wrote: > > It seems like rather a large change to be making in beta. Can you > > make a small patch that fixes the immediate problem, and leave the > > refactoring for 8.5? > > The hack I've come up with short of the refactor is to duplicate a lot > of the backslash handling code from command.c and describe.c. It > doesn't strike me as being all that much less work than the refactor. Tab completion has never been perfect. I don't think beta is the best time to be improving it so much. I think a small patch that just adds "\dfa", "\dfw" and appropriate pattern completions (i.e. it lists window functions in \dfw, etc) should be enough for now, and let the big refactoring loose in the 8.5 timeframe. If we don't support \dfaw in 8.4, tough world. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Mon, Apr 27, 2009 at 01:31:11PM -0400, Alvaro Herrera wrote: > David Fetter wrote: > > On Mon, Apr 27, 2009 at 01:11:44PM -0400, Tom Lane wrote: > > > > It seems like rather a large change to be making in beta. Can > > > you make a small patch that fixes the immediate problem, and > > > leave the refactoring for 8.5? > > > > The hack I've come up with short of the refactor is to duplicate a > > lot of the backslash handling code from command.c and describe.c. > > It doesn't strike me as being all that much less work than the > > refactor. > > Tab completion has never been perfect. I don't think beta is the > best time to be improving it so much. I think a small patch that > just adds "\dfa", "\dfw" and appropriate pattern completions (i.e. > it lists window functions in \dfw, etc) should be enough for now, > and let the big refactoring loose in the 8.5 timeframe. If we don't > support \dfaw in 8.4, tough world. The problem I see that argues for a refactor is the S option. Short of the refactor, how do I handle it? 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 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
David Fetter wrote: > On Mon, Apr 27, 2009 at 01:31:11PM -0400, Alvaro Herrera wrote: > > Tab completion has never been perfect. I don't think beta is the > > best time to be improving it so much. I think a small patch that > > just adds "\dfa", "\dfw" and appropriate pattern completions (i.e. > > it lists window functions in \dfw, etc) should be enough for now, > > and let the big refactoring loose in the 8.5 timeframe. If we don't > > support \dfaw in 8.4, tough world. > > The problem I see that argues for a refactor is the S option. Short > of the refactor, how do I handle it? Just don't (IMHO anyway). -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Did we ever get tab completion support for these backslash commands?
---------------------------------------------------------------------------
David Fetter wrote:
> On Fri, Apr 17, 2009 at 04:42:31PM -0400, Alvaro Herrera wrote:
> > David Fetter wrote:
> >
> > > Is this any better?
> >
> > So what happens if I do \dfaQ? It should throw an error, yes?
>
> Interesting question.
> >
> > This help line:
> >
> > > + fprintf(output, _(" \\df[S+] [PATTERN] list functions. Add a, n, t, w for aggregate, normal, trigger,
window\n"));
> >
> > needs shortening to below 80 chars (or maybe split it in two lines.
> > Just make sure they are a single translation item).
> >
> > It also seems like we're missing tab completion support for this.
>
> This is another interesting question. I notice that the tab
> completion doesn't support things like \dit.
>
> Should I add that as a separate patch?
>
> 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
>
> Remember to vote!
> Consider donating to Postgres: http://www.postgresql.org/about/donate
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
-- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB
http://enterprisedb.comPG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do + If your life is a hard
drive,Christ can be your backup. +
On Thu, Feb 25, 2010 at 07:20:58PM -0500, Bruce Momjian wrote: > > Did we ever get tab completion support for these backslash commands? Nope :/ Not sure if I'll be able to get to it this week, either. 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
David Fetter wrote: > On Thu, Feb 25, 2010 at 07:20:58PM -0500, Bruce Momjian wrote: > > > > Did we ever get tab completion support for these backslash commands? > > Nope :/ > > Not sure if I'll be able to get to it this week, either. What is the TODO description then? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do