Re: psql: show only failed queries

2014-07-09 14:39 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:

> On Wed, Jul 9, 2014 at 9:06 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> > Hi
> >
> >
> > 2014-07-09 7:07 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
> >
> >> On Mon, Jun 30, 2014 at 8:33 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com
> >
> >> wrote:
> >> >
> >> >
> >> >
> >> > 2014-06-30 13:01 GMT+02:00 Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com>:
> >> >
> >> >> At 2014-06-30 12:48:30 +0200, pavel(dot)stehule(at)gmail(dot)com wrote:
> >> >> >
> >> >> > + <para>
> >> >> > + Print a failed SQL commands to standard error output. This
> is
> >> >> > + equivalent to setting the variable <varname>ECHO</varname>
> to
> >> >> > + <literal>errors</literal>.
> >> >>
> >> >> No "a", just "Print failed SQL commands …".
> >> >>
> >> >> > - <option>-e</option>.
> >> >> > + <option>-e</option>. If set to <literal>error</literal>
> then
> >> >> > only
> >> >> > + failed queries are displayed.
> >> >>
> >> >> Should be "errors" here, not "error".
> >> >>
> >> >> > printf(_(" -a, --echo-all echo all input from
> >> >> > script\n"));
> >> >> > + printf(_(" -b --echo-errors echo failed commands
> sent
> >> >> > to
> >> >> > server\n"));
> >> >> > printf(_(" -e, --echo-queries echo commands sent to
> >> >> > server\n"));
> >> >>
> >> >> Should have a comma after -b to match other options. Also I would
> >> >> remove
> >> >> "sent to server" from the description: "echo failed commands" is
> fine.
> >> >
> >> >
> >> > fixed
> >>
> >> $ psql -b
> >> bin/psql: invalid option -- 'b'
> >> Try "psql --help" for more information.
> >>
> >> I got this error. ISTM you forgot to add 'b' into the third argument of
> >> getopt_long in startup.c.
> >>
> >
> > fixed
> >
> >>
> >> <application>psql</application> merely prints all queries as
> >> they are sent to the server. The switch for this is
> >> - <option>-e</option>.
> >> + <option>-e</option>. If set to <literal>errors</literal> then
> >> only
> >> + failed queries are displayed.
> >>
> >> I think that where failed queries are output should be documented here.
> >> Otherwise users might misunderstand they are output to standard output
> >> like ECHO=all and queries do.
> >>
> >> It's better to add "The switch for this is <option>-b</option>." into
> the
> >> doc.
> >
> >
> > fixed
> >
> >>
> >> + else if (strcmp(prev2_wd, "\\set") == 0)
> >> + {
> >> + if (strcmp(prev_wd, "ECHO") == 0)
> >> + {
> >> + static const char *const my_list[] =
> >> + {"none", "errors", "queries", "all", NULL};
> >> +
> >> + COMPLETE_WITH_LIST_CS(my_list);
> >> + }
> >> + else if (strcmp(prev_wd, "ECHO_HIDDEN") == 0)
> >> + {
> >> + static const char *const my_list[] =
> >> + {"noexec", "off", "on", NULL};
> >> +
> >> + COMPLETE_WITH_LIST_CS(my_list);
> >> + }
> >> + }
> >>
> >> I think that adding tab-completions of psql variables is good, but
> >> adding those of only ECHO and ECHO_HIDDEN seems half-baked.
> >> Probably this part should be split into separate patch.
> >
> >
> > fixed
> >
> > please, see updated patch in attachment
>
> Thanks for updating the patch!
>
> Barring any objection, I will commit this patch except tab-completion part.
>
>
Thank you

> I'm not against adding tab-completion support for psql variables, but I'm
> not sure if it's good idea or not to treat only one or two variables
> special
> and add tab-completions for them. There are other variables which accept
> special argument (e.g., COMP_KEYWORD_CASE) and it's basically worth
> adding tab-completion support for them.
>
>

It can be a second discussion, but I am thinking so anywhere when we can
use autocomplete, then we should it - Although it should not to substitute
documentation, it is fast help about available options, mainly in situation
where variables can hold a relative different content. I can prepare, if
there will not any objection addition patch with complete autocomplete for
all psql variables described in doc.

Regards

Pavel

> Regards,
>
> --
> Fujii Masao
>