Home > mailing lists

Thread: psql: show only failed queries

psql: show only failed queries

From

Pavel Stehule

Date:

01 March 2014, 14:02:06

Hello

I was asked, how can be showed only failed queries in psql.

I am thinking, so it is not possible now. But implementation is very simple

What do you think about it?

bash-4.1$ psql postgres -v ECHO=error -f data.sql
INSERT 0 1
Time: 27.735 ms
INSERT 0 1
Time: 8.303 ms
psql:data.sql:3: ERROR: value too long for type character varying(2)
insert into foo values('bbb');
Time: 0.178 ms
INSERT 0 1
Time: 8.285 ms
psql:data.sql:5: ERROR: value too long for type character varying(2)
insert into foo values('ssssss');
Time: 0.422 ms

Regards

Pavel

Attachment

echo-error.patch

Re: psql: show only failed queries

From

Fabrízio de Royes Mello

Date:

04 March 2014, 08:36:01

On Sat, Mar 1, 2014 at 8:01 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
> Hello
>
> I was asked, how can be showed only failed queries in psql.
>
> I am thinking, so it is not possible now. But implementation is very simple
>
> What do you think about it?
>
> bash-4.1$ psql postgres -v ECHO=error -f data.sql
> INSERT 0 1
> Time: 27.735 ms
> INSERT 0 1
> Time: 8.303 ms
> psql:data.sql:3: ERROR: value too long for type character varying(2)
> insert into foo values('bbb');
> Time: 0.178 ms
> INSERT 0 1
> Time: 8.285 ms
> psql:data.sql:5: ERROR: value too long for type character varying(2)
> insert into foo values('ssssss');
> Time: 0.422 ms
>

The patch works fine, but I think we must add some prefix to printed query. Like that:

fabrizio=# \set ECHO error
fabrizio=# insert into foo values ('XXX');
ERROR: value too long for type character varying(2)
DETAIL: insert into foo values ('XXX');

or

fabrizio=# \set ECHO error
fabrizio=# insert into foo values ('XXX');
ERROR: value too long for type character varying(2)
QUERY: insert into foo values ('XXX');

This may help to filter the output with some tool like 'grep'.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

Re: psql: show only failed queries

From

Pavel Stehule

Date:

04 March 2014, 10:53:20

2014-03-04 6:35 GMT+01:00 Fabrízio de Royes Mello <fabriziomello@gmail.com>:

On Sat, Mar 1, 2014 at 8:01 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
> Hello
>
> I was asked, how can be showed only failed queries in psql.
>
> I am thinking, so it is not possible now. But implementation is very simple
>
> What do you think about it?
>
> bash-4.1$ psql postgres -v ECHO=error -f data.sql
> INSERT 0 1
> Time: 27.735 ms
> INSERT 0 1
> Time: 8.303 ms
> psql:data.sql:3: ERROR: value too long for type character varying(2)
> insert into foo values('bbb');
> Time: 0.178 ms
> INSERT 0 1
> Time: 8.285 ms
> psql:data.sql:5: ERROR: value too long for type character varying(2)
> insert into foo values('ssssss');
> Time: 0.422 ms
>

The patch works fine, but I think we must add some prefix to printed query. Like that:

fabrizio=# \set ECHO error
fabrizio=# insert into foo values ('XXX');

ERROR: value too long for type character varying(2)
DETAIL: insert into foo values ('XXX');

or

fabrizio=# \set ECHO error
fabrizio=# insert into foo values ('XXX');

ERROR: value too long for type character varying(2)
QUERY: insert into foo values ('XXX');

This may help to filter the output with some tool like 'grep'.

sure, good idea.

I add link to your notice to commitfest app

Regards

Pavel

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

Re: psql: show only failed queries

From

Pavel Stehule

Date:

04 June 2014, 18:55:23

Hello

updated patch - only one change: query is prefixed by "QUERY: "

current state:

[pavel@localhost ~]$ src/postgresql/src/bin/psql/psql postgres -q -f data.sql
psql:data.sql:6: ERROR: value too long for type character varying(3)

Show only errors mode:

[pavel@localhost ~]$ src/postgresql/src/bin/psql/psql postgres -q -v ECHO=error -f data.sql
psql:data.sql:6: ERROR: value too long for type character varying(3)
QUERY: INSERT INTO bubu VALUES('Ahoj');

Now, when I am thinking about these results, I am thinking, so second variant is more practical and can be default.

Opinions, notes?

Regards

Pavel

2014-03-04 8:52 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:

2014-03-04 6:35 GMT+01:00 Fabrízio de Royes Mello <fabriziomello@gmail.com>:

On Sat, Mar 1, 2014 at 8:01 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
> Hello
>
> I was asked, how can be showed only failed queries in psql.
>
> I am thinking, so it is not possible now. But implementation is very simple
>
> What do you think about it?
>
> bash-4.1$ psql postgres -v ECHO=error -f data.sql
> INSERT 0 1
> Time: 27.735 ms
> INSERT 0 1
> Time: 8.303 ms
> psql:data.sql:3: ERROR: value too long for type character varying(2)
> insert into foo values('bbb');
> Time: 0.178 ms
> INSERT 0 1
> Time: 8.285 ms
> psql:data.sql:5: ERROR: value too long for type character varying(2)
> insert into foo values('ssssss');
> Time: 0.422 ms
>

The patch works fine, but I think we must add some prefix to printed query. Like that:

fabrizio=# \set ECHO error
fabrizio=# insert into foo values ('XXX');

ERROR: value too long for type character varying(2)
DETAIL: insert into foo values ('XXX');

or

fabrizio=# \set ECHO error
fabrizio=# insert into foo values ('XXX');

ERROR: value too long for type character varying(2)
QUERY: insert into foo values ('XXX');

This may help to filter the output with some tool like 'grep'.

sure, good idea.

I add link to your notice to commitfest app

Regards

Pavel

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

Attachment

echo_error.patch

Re: psql: show only failed queries

From

Peter Eisentraut

Date:

04 June 2014, 19:16:26

On 6/4/14, 11:54 AM, Pavel Stehule wrote:
> updated patch - only one change: query is prefixed by "QUERY:  "

In the backend server log, this is called "STATEMENT:  ".

Re: psql: show only failed queries

From

Pavel Stehule

Date:

04 June 2014, 19:23:12

2014-06-04 18:16 GMT+02:00 Peter Eisentraut <peter_e@gmx.net>:

On 6/4/14, 11:54 AM, Pavel Stehule wrote:
> updated patch - only one change: query is prefixed by "QUERY: "

In the backend server log, this is called "STATEMENT: ".

good idea

updated patch

Pavel

Attachment

echo_error.patch

Re: psql: show only failed queries

From

Samrat Revagade

Date:

25 June 2014, 13:32:47

Hi Pavel,

After applying patch, on error condition it displays error message two times as follows:

ERROR: column "abc" does not exist at character 23

STATEMENT: insert into ax

values(abc);

psql:a.sql:7: ERROR: column "abc" does not exist

LINE 2: values(abc);

user may confuse because of repeated error messages. so I think its better to display only one message, one of the possible ways is as follows:

ERROR: column "abc" does not exist at character 23

STATEMENT: insert into ax

values(abc);

Am I missing something ?

On Wed, Jun 4, 2014 at 9:52 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:

2014-06-04 18:16 GMT+02:00 Peter Eisentraut <peter_e@gmx.net>:

On 6/4/14, 11:54 AM, Pavel Stehule wrote:
> updated patch - only one change: query is prefixed by "QUERY: "

In the backend server log, this is called "STATEMENT: ".

good idea

updated patch

Pavel

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--

Regards,

Samrat Revgade

Re: psql: show only failed queries

From

Pavel Stehule

Date:

25 June 2014, 21:49:02

2014-06-25 12:32 GMT+02:00 Samrat Revagade <revagade.samrat@gmail.com>:

Hi Pavel,

After applying patch, on error condition it displays error message two times as follows:

ERROR: column "abc" does not exist at character 23
STATEMENT: insert into ax
values(abc);
psql:a.sql:7: ERROR: column "abc" does not exist
LINE 2: values(abc);

user may confuse because of repeated error messages. so I think its better to display only one message, one of the possible ways is as follows:

ERROR: column "abc" does not exist at character 23
STATEMENT: insert into ax
values(abc);

Am I missing something ?

LINE info is a part of error message and should be eliminated by terse mode

[pavel@localhost ~]$ psql -v ECHO=error -f test.sql postgres > /dev/null
psql:test.sql:4: ERROR: syntax error at or near ";"
LINE 2: 10 + ;
^
psql:test.sql:4: STATEMENT: select
10 + ;
psql:test.sql:8: ERROR: syntax error at end of input
LINE 2: 30 +
^
psql:test.sql:8: STATEMENT: select
30 +

but you can switch to terse mode:

[pavel@localhost ~]$ psql -v ECHO=error -v VERBOSITY=terse -f test.sql postgres > /dev/null
psql:test.sql:4: ERROR: syntax error at or near ";" at character 13
psql:test.sql:4: STATEMENT: select
10 + ;
psql:test.sql:8: ERROR: syntax error at end of input at character 13
psql:test.sql:8: STATEMENT: select
30 +

What is what you would

I am sending updated patch - buggy statement is printed via more logical psql_error function instead printf

Regards

Pavel

On Wed, Jun 4, 2014 at 9:52 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:

2014-06-04 18:16 GMT+02:00 Peter Eisentraut <peter_e@gmx.net>:

On 6/4/14, 11:54 AM, Pavel Stehule wrote:
> updated patch - only one change: query is prefixed by "QUERY: "

In the backend server log, this is called "STATEMENT: ".

good idea

updated patch

Pavel

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Regards,

Samrat Revgade

Attachment

echo-error-01.patch

Re: psql: show only failed queries

From

Alvaro Herrera

Date:

25 June 2014, 23:32:40

ECHO_HIDDEN?

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Re: psql: show only failed queries

From

Samrat Revagade

Date:

26 June 2014, 09:23:00

<div dir="ltr"><div class="gmail_extra"><div style="font-family:arial,sans-serif;font-size:12.727272033691406px">> I
amsending updated patch - buggy statement is printed via more logical psql_error function instead printf</div><div
style="font-family:arial,sans-serif;font-size:12.727272033691406px"><br/></div><div
style="font-family:arial,sans-serif;font-size:12.727272033691406px">Thankyou for updating patch, I really appreciate
yourefforts.</div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px"><br /></div><div
style="font-family:arial,sans-serif;font-size:12.727272033691406px">Now,everything is good from my side.</div><div
style="font-family:arial,sans-serif;font-size:12.727272033691406px">* <span
style="color:rgb(0,0,0);font-family:sans-serif;font-size:12.666666984558105px;line-height:19.049999237060547px">itapply
cleanlyto the current git master</span></div><div
style="font-family:arial,sans-serif;font-size:12.727272033691406px"><span
style="color:rgb(0,0,0);font-family:sans-serif;font-size:12.666666984558105px;line-height:19.049999237060547px">* </span><span
style="color:rgb(0,0,0);font-family:sans-serif;font-size:12.666666984558105px;line-height:19.049999237060547px">includes
necessarydocs</span></div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px"><font
color="#000000"face="sans-serif"><span style="font-size:12.666666984558105px;line-height:19.049999237060547px">* I
thinkIt is very good  and necessary feature.</span></font></div><div
style="font-family:arial,sans-serif;font-size:12.727272033691406px"><fontcolor="#000000" face="sans-serif"><span
style="line-height:19.049999237060547px"><br/></span></font></div><div
style="font-family:arial,sans-serif;font-size:12.727272033691406px"><fontcolor="#000000" face="sans-serif"><span
style="line-height:19.049999237060547px">If KumarRajeev Rastogi do not have any extra comments, then I
think </span></font><spanstyle="line-height:19.049999237060547px;color:rgb(0,0,0);font-family:sans-serif">patch is
 readyfor committer.</span></div></div></div>

Re: psql: show only failed queries

From

Pavel Stehule

Date:

26 June 2014, 10:08:27

2014-06-26 8:22 GMT+02:00 Samrat Revagade <revagade.samrat@gmail.com>:

> I am sending updated patch - buggy statement is printed via more logical psql_error function instead printf

Thank you for updating patch, I really appreciate your efforts.

Now, everything is good from my side.
* it apply cleanly to the current git master
* includes necessary docs
* I think It is very good and necessary feature.

If Kumar Rajeev Rastogi do not have any extra comments, then I think patch is ready for committer.

Thank you very much

Regards

Pavel

Re: psql: show only failed queries

From

Rajeev rastogi

Date:

30 June 2014, 09:17:48

<div class="WordSection1"><p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">On</span><span
style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">26June 2014 11:53, Samrat Revagade Wrote:</span><p
class="MsoNormal"><spanstyle="font-size:10.0pt;font-family:"Tahoma","sans-serif""> </span><p class="MsoNormal"><span
style="font-size:9.5pt;font-family:"Arial","sans-serif"">>>I am sending updated patch - buggy statement is
printedvia more logical psql_error function instead printf</span><p class="MsoNormal"><span
style="font-size:9.5pt;font-family:"Arial","sans-serif""> </span><pclass="MsoNormal"><span
style="font-size:9.5pt;font-family:"Arial","sans-serif"">>Thankyou for updating patch, I really appreciate your
efforts.</span><pclass="MsoNormal"><span style="font-size:9.5pt;font-family:"Arial","sans-serif""> </span><p
class="MsoNormal"><spanstyle="font-size:9.5pt;font-family:"Arial","sans-serif"">>Now, everything is good from my
side.</span><pclass="MsoNormal"><span style="font-size:9.5pt;font-family:"Arial","sans-serif"">>* <span
style="color:black">itapply cleanly to the current git master</span></span><p class="MsoNormal"><span
style="font-size:9.5pt;font-family:"Arial","sans-serif";color:black">>* includesnecessary docs</span><span
style="font-size:9.5pt;font-family:"Arial","sans-serif""></span><pclass="MsoNormal"><span
style="font-size:9.5pt;font-family:"Arial","sans-serif";color:black">>*I think It is very good  and necessary
feature.</span><spanstyle="font-size:9.5pt;font-family:"Arial","sans-serif""></span><p class="MsoNormal"><span
style="font-size:9.5pt;font-family:"Arial","sans-serif""> </span><pclass="MsoNormal"><span
style="font-size:9.5pt;font-family:"Arial","sans-serif";color:black">>If KumarRajeev Rastogi do not have any extra
comments,then I think patch is  ready for committer.</span><p class="MsoNormal"><span
style="font-size:9.5pt;font-family:"Arial","sans-serif""> </span><pclass="MsoNormal"><span
style="font-size:9.5pt;font-family:"Arial","sans-serif"">Ihave reviewed this patch. Please find my review comments
below:</span><pclass="MsoListParagraph" style="text-indent:-18.0pt;mso-list:l0 level1 lfo1"><span
style="font-size:9.5pt;font-family:"Arial","sans-serif";color:black"><spanstyle="mso-list:Ignore">1.<span
style="font:7.0pt"Times New Roman"">     </span></span></span><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Commandstart-up option (e.g. -a/--echo-all for --ECHO=all),
fornew functionality is not provided.</span><p class="MsoListParagraph" style="text-indent:-18.0pt;mso-list:l0 level1
lfo1"><spanstyle="font-size:9.5pt;font-family:"Arial","sans-serif";color:black"><span style="mso-list:Ignore">2.<span
style="font:7.0pt"Times New Roman"">     </span></span></span><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif"">NewCommand start-up option should be added in "psql --help"
aswell as in documentation.</span><p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif""> </span><pclass="MsoNormal"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Alsoas I understand, this new option is kind of sub-set of
existingoption (ECHO=query), so should not we display</span><p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif"">querystring in the same format as it was getting printed
earlier.</span><pclass="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Though I also feel
thatprefixing query with STATEMENT word will be helpful to grep but at the same time I am worried</span><p
class="MsoNormal"><spanstyle="font-size:11.0pt;font-family:"Calibri","sans-serif"">about inconsistency with existing
option.  </span><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""> </span><p
class="MsoNormal"><i><spanstyle="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Thanks and
Regards,</span></i><pclass="MsoNormal"><i><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">KumarRajeev Rastogi </span></i></div>

Re: psql: show only failed queries

From

Pavel Stehule

Date:

30 June 2014, 09:54:43

2014-06-30 8:17 GMT+02:00 Rajeev rastogi <rajeev.rastogi@huawei.com>:

On 26 June 2014 11:53, Samrat Revagade Wrote:

>> I am sending updated patch - buggy statement is printed via more logical psql_error function instead printf

>Thank you for updating patch, I really appreciate your efforts.

>Now, everything is good from my side.
>* it apply cleanly to the current git master
>* includes necessary docs
>* I think It is very good and necessary feature.

>If Kumar Rajeev Rastogi do not have any extra comments, then I think patch is ready for committer.

I have reviewed this patch. Please find my review comments below:
1. Command start-up option (e.g. -a/--echo-all for --ECHO=all), for new functionality is not provided.

all not options entered via psql variables has psql option and psql comment. I'll plan add new decription to --help-variables list.

If it is necessary I can add long option --echo-errors, I didn't a good char for short option. Any idea?

2. New Command start-up option should be added in "psql --help" as well as in documentation.

depends on previous,

Also as I understand, this new option is kind of sub-set of existing option (ECHO=query), so should not we display
query string in the same format as it was getting printed earlier.
Though I also feel that prefixing query with STATEMENT word will be helpful to grep but at the same time I am worried
about inconsistency with existing option.

This is question. And I am not strong in feeling what should be preferred. But still I am inclined to prefer a variant with STATEMENT prefix. Mode with -a is used with different purpose than mode "show errors only" - and output with prefix is much more consistent with log entry - and displaying error. So I agree, so there is potential inconsistency (but nowhere is output defined), but this output is more practical, when you are concentrated to error's processing.

Regards

Pavel

Thanks and Regards,
Kumar Rajeev Rastogi

Re: psql: show only failed queries

From

Rajeev rastogi

Date:

30 June 2014, 12:21:13

<div class="WordSection1"><p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">On</span><span
style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">30June 2014 12:24, Pavel Stehule Wrote:</span><p
class="MsoNormal"><spanstyle="font-size:10.0pt;font-family:"Tahoma","sans-serif""> </span><p class="MsoNormal"
style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span
style="font-size:9.5pt;font-family:"Arial","sans-serif"">>>Ihave reviewed this patch. Please find my review
commentsbelow:</span><p><span
style="font-size:9.5pt;font-family:"Arial","sans-serif";color:black">>>1.</span><span
style="font-size:7.0pt;color:black">    </span><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Commandstart-up option (e.g. -a/--echo-all for --ECHO=all),
fornew functionality is not provided.</span><p class="MsoNormal" style="margin-bottom:12.0pt">>all not options
enteredvia psql variables has psql option and psql comment. I'll plan add new decription to --help-variables list.<p
class="MsoNormal">>Ifit is necessary I can add long option --echo-errors, I didn't a good char for short option. Any
idea?<pclass="MsoNormal"> <p class="MsoNormal">But the new option we are adding are on a track of existing option, so
betterwe add start-up option for this also.<p class="MsoNormal">Yeah long option –echo-errors seems to be fine to me
also.For short option, I think we can use “-b” stands for blunder. This is the closest one I could think of.<p
class="MsoNormal"> <p><spanstyle="font-size:9.5pt;font-family:"Arial","sans-serif";color:black">>>2.</span><span
style="font-size:7.0pt;color:black">    </span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">New
Commandstart-up option should be added in "psql --help" as well as in documentation.</span><p
class="MsoNormal">>dependson previous, <p class="MsoNormal"
style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Right.</span><pclass="MsoNormal"
style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif"">>>Alsoas I understand, this new option is kind of
sub-setof existing option (ECHO=query), so should not we display</span><p class="MsoNormal"
style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif"">>>querystring in the same format as it was getting
printedearlier.</span><p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif"">>>ThoughI also feel that prefixing query with
STATEMENTword will be helpful to grep but at the same time I am worried</span><p class="MsoNormal"
style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif"">>>aboutinconsistency with existing option.
  </span><pclass="MsoNormal"> <p class="MsoNormal" style="margin-bottom:12.0pt">>This is question. And I am not
strongin feeling what should be preferred. But still I am inclined to prefer a variant with STATEMENT prefix. Mode with
-ais used with different purpose than mode "show errors only" - and output with prefix is much <p class="MsoNormal"
style="margin-bottom:12.0pt">>more consistent with log entry - and displaying error. So I agree, so there is
potentialinconsistency (but nowhere is output defined), but this output is more practical, when you are concentrated to
error'sprocessing. <p class="MsoNormal" style="margin-bottom:12.0pt">Yeah right, I just wanted to raise point to
provokeother thought to see if anyone having different opinion. If no objection from others, we can go ahead with the
currentprefixing approach.<span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"></span><p
class="MsoNormal"><i><spanstyle="font-size:11.0pt;font-family:"Calibri","sans-serif";color:black">Thanks and
Regards,</span></i><pclass="MsoNormal"><i><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">KumarRajeev Rastogi</span></i><i><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:black"></span></i></div>

Re: psql: show only failed queries

From

Pavel Stehule

Date:

30 June 2014, 13:49:18

2014-06-30 11:20 GMT+02:00 Rajeev rastogi <rajeev.rastogi@huawei.com>:

On 30 June 2014 12:24, Pavel Stehule Wrote:

>>I have reviewed this patch. Please find my review comments below:
>>1. Command start-up option (e.g. -a/--echo-all for --ECHO=all), for new functionality is not provided.
>all not options entered via psql variables has psql option and psql comment. I'll plan add new decription to --help-variables list.
>If it is necessary I can add long option --echo-errors, I didn't a good char for short option. Any idea?

But the new option we are adding are on a track of existing option, so better we add start-up option for this also.
Yeah long option –echo-errors seems to be fine to me also. For short option, I think we can use “-b” stands for blunder. This is the closest one I could think of.

fixed

see a attachment pls

>>2. New Command start-up option should be added in "psql --help" as well as in documentation.
>depends on previous,
Right.
>>Also as I understand, this new option is kind of sub-set of existing option (ECHO=query), so should not we display
>>query string in the same format as it was getting printed earlier.
>>Though I also feel that prefixing query with STATEMENT word will be helpful to grep but at the same time I am worried
>>about inconsistency with existing option.

>This is question. And I am not strong in feeling what should be preferred. But still I am inclined to prefer a variant with STATEMENT prefix. Mode with -a is used with different purpose than mode "show errors only" - and output with prefix is much
> more consistent with log entry - and displaying error. So I agree, so there is potential inconsistency (but nowhere is output defined), but this output is more practical, when you are concentrated to error's processing.
Yeah right, I just wanted to raise point to provoke other thought to see if anyone having different opinion. If no objection from others, we can go ahead with the current prefixing approach.

ok, we can wait two days

Regards

Pavel

Thanks and Regards,
Kumar Rajeev Rastogi

Attachment

echo-error-02.patch

Re: psql: show only failed queries

From

Abhijit Menon-Sen

Date:

30 June 2014, 14:01:13

At 2014-06-30 12:48:30 +0200, pavel.stehule@gmail.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.

Otherwise looks fine. I see no reason to wait for further feedback, so
I'll mark this ready for committer if you make the above corrections.

At some point, you should probably also update your --help-variables
patch to add this new value to the description of ECHO.

-- Abhijit

Re: psql: show only failed queries

From

Pavel Stehule

Date:

30 June 2014, 14:33:58

2014-06-30 13:01 GMT+02:00 Abhijit Menon-Sen <ams@2ndquadrant.com>:

At 2014-06-30 12:48:30 +0200, pavel.stehule@gmail.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

Otherwise looks fine. I see no reason to wait for further feedback, so
I'll mark this ready for committer if you make the above corrections.

At some point, you should probably also update your --help-variables
patch to add this new value to the description of ECHO.

I have it in TODO. But I don't would to introduce a dependency between these patches - so when first patch will be committed, than I update second patch

Regards

Pavel

-- Abhijit

Attachment

echo-error-03.patch

Re: psql: show only failed queries

From

Fujii Masao

Date:

09 July 2014, 08:07:33

On Mon, Jun 30, 2014 at 8:33 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
>
>
> 2014-06-30 13:01 GMT+02:00 Abhijit Menon-Sen <ams@2ndquadrant.com>:
>
>> At 2014-06-30 12:48:30 +0200, pavel.stehule@gmail.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.
        <application>psql</application> merely prints all queries as        they are sent to the server. The switch for
thisis 
-        <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.

+    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.

Regards,

--
Fujii Masao

Re: psql: show only failed queries

From

Pavel Stehule

Date:

09 July 2014, 15:07:10

Hi

2014-07-09 7:07 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:

On Mon, Jun 30, 2014 at 8:33 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
>
>
> 2014-06-30 13:01 GMT+02:00 Abhijit Menon-Sen <ams@2ndquadrant.com>:
>
>> At 2014-06-30 12:48:30 +0200, pavel.stehule@gmail.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

Thank you

Regards

Pavel

Regards,

--
Fujii Masao

Attachment

echo-error-04.patch

Re: psql: show only failed queries

From

Fujii Masao

Date:

09 July 2014, 15:39:10

On Wed, Jul 9, 2014 at 9:06 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> Hi
>
>
> 2014-07-09 7:07 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
>
>> On Mon, Jun 30, 2014 at 8:33 PM, Pavel Stehule <pavel.stehule@gmail.com>
>> wrote:
>> >
>> >
>> >
>> > 2014-06-30 13:01 GMT+02:00 Abhijit Menon-Sen <ams@2ndquadrant.com>:
>> >
>> >> At 2014-06-30 12:48:30 +0200, pavel.stehule@gmail.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.

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.

Regards,

--
Fujii Masao

Re: psql: show only failed queries

From

Pavel Stehule

Date:

09 July 2014, 15:44:58

2014-07-09 14:39 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:

On Wed, Jul 9, 2014 at 9:06 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> Hi
>
>
> 2014-07-09 7:07 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
>
>> On Mon, Jun 30, 2014 at 8:33 PM, Pavel Stehule <pavel.stehule@gmail.com>
>> wrote:
>> >
>> >
>> >
>> > 2014-06-30 13:01 GMT+02:00 Abhijit Menon-Sen <ams@2ndquadrant.com>:
>> >
>> >> At 2014-06-30 12:48:30 +0200, pavel.stehule@gmail.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

Re: psql: show only failed queries

From

Fujii Masao

Date:

10 July 2014, 08:32:45

On Wed, Jul 9, 2014 at 9:44 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> Barring any objection, I will commit this patch except tab-completion
>> part.

Committed.

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

I have no objection. But I found that the tab-completion for psql
variables names
are not complete. For example, COMP_KEYWORD_CASE is not displayed.
Probably we should address this problem, too.

Regards,

-- 
Fujii Masao

Re: psql: show only failed queries

From

Pavel Stehule

Date:

10 July 2014, 10:51:40

2014-07-10 7:32 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:

On Wed, Jul 9, 2014 at 9:44 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> Barring any objection, I will commit this patch except tab-completion
>> part.

Committed.

Thank you very much

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

I have no objection. But I found that the tab-completion for psql
variables names
are not complete. For example, COMP_KEYWORD_CASE is not displayed.
Probably we should address this problem, too.

I prepare patch for next commitfest - it is relative trivial task

Pavel

Regards,

--
Fujii Masao

Re: psql: show only failed queries

From

Pavel Stehule

Date:

10 July 2014, 15:57:43

Hello

here is a proposed patch - autocomplete for known psql variable content

Regards

Pavel

2014-07-10 9:50 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:

2014-07-10 7:32 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:

On Wed, Jul 9, 2014 at 9:44 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> Barring any objection, I will commit this patch except tab-completion
>> part.

Committed.

Thank you very much

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

I have no objection. But I found that the tab-completion for psql
variables names
are not complete. For example, COMP_KEYWORD_CASE is not displayed.
Probably we should address this problem, too.

I prepare patch for next commitfest - it is relative trivial task

Pavel

Regards,

--
Fujii Masao

Attachment

psql-var-complete.patch

Re: psql: show only failed queries

From

Fujii Masao

Date:

15 July 2014, 12:29:16

On Thu, Jul 10, 2014 at 9:56 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> Hello
>
> here is a proposed patch - autocomplete for known psql variable content

Even after applying the patch, the following psql variables were not displayed
on the tab-completion of \set command.

HISTFILE
HISTSIZE
HOST
IGNOREEOF
LASTOID

I'm not sure if it's worth displaying all of them on the tab-completion of \set
because it seems strange to change some of them by \set command, for example
HOST, though.

Regards,

-- 
Fujii Masao

Re: psql: show only failed queries

From

Pavel Stehule

Date:

15 July 2014, 13:02:26

2014-07-15 11:29 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:

On Thu, Jul 10, 2014 at 9:56 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> Hello
>
> here is a proposed patch - autocomplete for known psql variable content

Even after applying the patch, the following psql variables were not displayed
on the tab-completion of \set command.

HISTFILE
HISTSIZE
HOST
IGNOREEOF
LASTOID

I'm not sure if it's worth displaying all of them on the tab-completion of \set
because it seems strange to change some of them by \set command, for example
HOST, though.

For these variables are not default - so doesn't exist and cannot be completed by default.

there are two fix:

a) fix autocomplete source - preferred

b) create default - but it is probably impossible

Regards

Pavel

Regards,

--
Fujii Masao

Re: psql: show only failed queries

From

Fujii Masao

Date:

15 July 2014, 13:07:30

On Tue, Jul 15, 2014 at 7:01 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
>
>
> 2014-07-15 11:29 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
>
>> On Thu, Jul 10, 2014 at 9:56 PM, Pavel Stehule <pavel.stehule@gmail.com>
>> wrote:
>> > Hello
>> >
>> > here is a proposed patch - autocomplete for known psql variable content
>>
>> Even after applying the patch, the following psql variables were not
>> displayed
>> on the tab-completion of \set command.
>>
>> HISTFILE
>> HISTSIZE
>> HOST
>> IGNOREEOF
>> LASTOID
>>
>> I'm not sure if it's worth displaying all of them on the tab-completion of
>> \set
>> because it seems strange to change some of them by \set command, for
>> example
>> HOST, though.
>
>
> For these variables are not default - so doesn't exist and cannot be
> completed by default.
>
> there are two fix:
>
> a) fix autocomplete source - preferred

+1

Regards,

-- 
Fujii Masao

Re: psql: show only failed queries

From

Pavel Stehule

Date:

15 July 2014, 22:35:10

2014-07-15 12:07 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:

On Tue, Jul 15, 2014 at 7:01 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
>
>
> 2014-07-15 11:29 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
>
>> On Thu, Jul 10, 2014 at 9:56 PM, Pavel Stehule <pavel.stehule@gmail.com>
>> wrote:
>> > Hello
>> >
>> > here is a proposed patch - autocomplete for known psql variable content
>>
>> Even after applying the patch, the following psql variables were not
>> displayed
>> on the tab-completion of \set command.
>>
>> HISTFILE
>> HISTSIZE
>> HOST
>> IGNOREEOF
>> LASTOID
>>
>> I'm not sure if it's worth displaying all of them on the tab-completion of
>> \set
>> because it seems strange to change some of them by \set command, for
>> example
>> HOST, though.
>
>
> For these variables are not default - so doesn't exist and cannot be
> completed by default.
>
> there are two fix:
>
> a) fix autocomplete source - preferred

+1

here is the patch

Regards

Pavel

Regards,

--
Fujii Masao

Attachment

psql-var-complete-2.patch

Re: psql: show only failed queries

From

Fujii Masao

Date:

05 August 2014, 14:31:31

On Wed, Jul 16, 2014 at 4:34 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
>
>
> 2014-07-15 12:07 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
>
>> On Tue, Jul 15, 2014 at 7:01 PM, Pavel Stehule <pavel.stehule@gmail.com>
>> wrote:
>> >
>> >
>> >
>> > 2014-07-15 11:29 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
>> >
>> >> On Thu, Jul 10, 2014 at 9:56 PM, Pavel Stehule
>> >> <pavel.stehule@gmail.com>
>> >> wrote:
>> >> > Hello
>> >> >
>> >> > here is a proposed patch - autocomplete for known psql variable
>> >> > content
>> >>
>> >> Even after applying the patch, the following psql variables were not
>> >> displayed
>> >> on the tab-completion of \set command.
>> >>
>> >> HISTFILE
>> >> HISTSIZE
>> >> HOST
>> >> IGNOREEOF
>> >> LASTOID
>> >>
>> >> I'm not sure if it's worth displaying all of them on the tab-completion
>> >> of
>> >> \set
>> >> because it seems strange to change some of them by \set command, for
>> >> example
>> >> HOST, though.
>> >
>> >
>> > For these variables are not default - so doesn't exist and cannot be
>> > completed by default.
>> >
>> > there are two fix:
>> >
>> > a) fix autocomplete source - preferred
>>
>> +1
>
>
> here is the patch

Thanks for the patch!

I got the following compiler warnings.

tab-complete.c:4155: warning: assignment discards qualifiers from
pointer target type
tab-complete.c:4166: warning: assignment discards qualifiers from
pointer target type

+        "FETCH_COUNT", "HISTCONTROL", "HISTFILE", "HISTSIZE", "HOST",
"IGNOREOFF",

Typo: IGNOREOFF -> IGNOREEOF

+    /* all psql known variables are included in list by default */
+    for (known_varname = known_varnames; *known_varname; known_varname++)
+        varnames[nvars++] = pg_strdup(*known_varname);

Don't we need to append both prefix and suffix to even known variables?

+    else if (strcmp(prev2_wd, "\\set") == 0)
+    {
+        if (strcmp(prev_wd, "AUTOCOMMIT") == 0)

ISTM that some psql variables like IGNOREEOF are not there. Why not?

Regards,

-- 
Fujii Masao

Re: psql: show only failed queries

From

Pavel Stehule

Date:

07 August 2014, 00:26:54

Hello

updated version patch in attachment

2014-08-05 13:31 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:

On Wed, Jul 16, 2014 at 4:34 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
>
>
> 2014-07-15 12:07 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
>
>> On Tue, Jul 15, 2014 at 7:01 PM, Pavel Stehule <pavel.stehule@gmail.com>
>> wrote:
>> >
>> >
>> >
>> > 2014-07-15 11:29 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
>> >
>> >> On Thu, Jul 10, 2014 at 9:56 PM, Pavel Stehule
>> >> <pavel.stehule@gmail.com>
>> >> wrote:
>> >> > Hello
>> >> >
>> >> > here is a proposed patch - autocomplete for known psql variable
>> >> > content
>> >>
>> >> Even after applying the patch, the following psql variables were not
>> >> displayed
>> >> on the tab-completion of \set command.
>> >>
>> >> HISTFILE
>> >> HISTSIZE
>> >> HOST
>> >> IGNOREEOF
>> >> LASTOID
>> >>
>> >> I'm not sure if it's worth displaying all of them on the tab-completion
>> >> of
>> >> \set
>> >> because it seems strange to change some of them by \set command, for
>> >> example
>> >> HOST, though.
>> >
>> >
>> > For these variables are not default - so doesn't exist and cannot be
>> > completed by default.
>> >
>> > there are two fix:
>> >
>> > a) fix autocomplete source - preferred
>>
>> +1
>
>
> here is the patch

Thanks for the patch!

I got the following compiler warnings.

tab-complete.c:4155: warning: assignment discards qualifiers from
pointer target type
tab-complete.c:4166: warning: assignment discards qualifiers from
pointer target type

fixed

+ "FETCH_COUNT", "HISTCONTROL", "HISTFILE", "HISTSIZE", "HOST",
"IGNOREOFF",

Typo: IGNOREOFF -> IGNOREEOF

fixed

+ /* all psql known variables are included in list by default */
+ for (known_varname = known_varnames; *known_varname; known_varname++)
+ varnames[nvars++] = pg_strdup(*known_varname);

Don't we need to append both prefix and suffix to even known variables?

??? I am not sure if I understand well - probably system "read only" variables as DBNAME, USER, VERSION should not be there

+ else if (strcmp(prev2_wd, "\\set") == 0)
+ {
+ if (strcmp(prev_wd, "AUTOCOMMIT") == 0)

ISTM that some psql variables like IGNOREEOF are not there. Why not?

yes, there are not complete for DBNAME, ENCODING, FETCH_COUNT, HISTCONTROL, HISTFILE, HISTSIZE, HOST, IGNOREEOFF, PROMPT*,USER, VERSION

There are more reasons:

* paremeter is not a enum (string, number or both): FETCH_COUNT, PROMPT, HISTSIZE, ..

* variable is pseudo read only - change has not any effect: DBNAME, ENCODING, VERSION

Regards

Pavel

Regards,

--
Fujii Masao

Re: psql: show only failed queries

From

Fujii Masao

Date:

07 August 2014, 08:10:43

On Thu, Aug 7, 2014 at 6:26 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> Hello
>
> updated version patch in attachment

Thanks! But ISTM you forgot to attached the patch.

>> +    /* all psql known variables are included in list by default */
>> +    for (known_varname = known_varnames; *known_varname; known_varname++)
>> +        varnames[nvars++] = pg_strdup(*known_varname);
>>
>> Don't we need to append both prefix and suffix to even known variables?
>
>
> ??? I am not sure if I understand well - probably system "read only"
> variables as DBNAME, USER, VERSION should not be there

I had that question because complete_from_variables() is also called by the
tab-completion of "\echo :" and it shows half-baked variables list. So
I thought probably we need to change complete_from_variables() more to
fix the problem.

>> +    else if (strcmp(prev2_wd, "\\set") == 0)
>> +    {
>> +        if (strcmp(prev_wd, "AUTOCOMMIT") == 0)
>>
>> ISTM that some psql variables like IGNOREEOF are not there. Why not?
>
>
> yes, there are not complete for DBNAME, ENCODING, FETCH_COUNT, HISTCONTROL,
> HISTFILE, HISTSIZE, HOST, IGNOREEOFF, PROMPT*,USER,  VERSION
>
> There are more reasons:
>
> * paremeter is not a enum (string, number or both): FETCH_COUNT, PROMPT,
> HISTSIZE, ..
>
> * variable is pseudo read only  - change has not any effect: DBNAME,
> ENCODING, VERSION

So HISTCONTROL should be there because it doesn't have such reasons at all?

Regards,

-- 
Fujii Masao

Re: psql: show only failed queries

From

Pavel Stehule

Date:

07 August 2014, 22:32:17

2014-08-07 7:10 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:

On Thu, Aug 7, 2014 at 6:26 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> Hello
>
> updated version patch in attachment

Thanks! But ISTM you forgot to attached the patch.

grr .. I am sorry

>> + /* all psql known variables are included in list by default */
>> + for (known_varname = known_varnames; *known_varname; known_varname++)
>> + varnames[nvars++] = pg_strdup(*known_varname);
>>
>> Don't we need to append both prefix and suffix to even known variables?
>
>
> ??? I am not sure if I understand well - probably system "read only"
> variables as DBNAME, USER, VERSION should not be there

I had that question because complete_from_variables() is also called by the
tab-completion of "\echo :" and it shows half-baked variables list. So
I thought probably we need to change complete_from_variables() more to
fix the problem.

I understand now.

I fixed it.

I have a question.\echo probably should not to show empty known variable.

data for autocomplete for \echo should be same as result of "\set"

>> + else if (strcmp(prev2_wd, "\\set") == 0)
>> + {
>> + if (strcmp(prev_wd, "AUTOCOMMIT") == 0)
>>
>> ISTM that some psql variables like IGNOREEOF are not there. Why not?
>
>
> yes, there are not complete for DBNAME, ENCODING, FETCH_COUNT, HISTCONTROL,
> HISTFILE, HISTSIZE, HOST, IGNOREEOFF, PROMPT*,USER, VERSION
>
> There are more reasons:
>
> * paremeter is not a enum (string, number or both): FETCH_COUNT, PROMPT,
> HISTSIZE, ..
>
> * variable is pseudo read only - change has not any effect: DBNAME,
> ENCODING, VERSION

So HISTCONTROL should be there because it doesn't have such reasons at all?

yes

Pavel

Regards,

--
Fujii Masao

Attachment

psql-var-complete-3.patch

Re: psql: show only failed queries

From

Fujii Masao

Date:

08 August 2014, 14:58:21

On Fri, Aug 8, 2014 at 4:31 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
>
>
> 2014-08-07 7:10 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
>
>> On Thu, Aug 7, 2014 at 6:26 AM, Pavel Stehule <pavel.stehule@gmail.com>
>> wrote:
>> > Hello
>> >
>> > updated version patch in attachment
>>
>> Thanks! But ISTM you forgot to attached the patch.
>
>
> grr .. I am sorry

No problem. Thanks for the patch! Attached is the revised version of the patch.

>> >> +    /* all psql known variables are included in list by default */
>> >> +    for (known_varname = known_varnames; *known_varname;
>> >> known_varname++)
>> >> +        varnames[nvars++] = pg_strdup(*known_varname);
>> >>
>> >> Don't we need to append both prefix and suffix to even known variables?
>> >
>> >
>> > ??? I am not sure if I understand well - probably system "read only"
>> > variables as DBNAME, USER, VERSION should not be there
>>
>> I had that question because complete_from_variables() is also called by
>> the
>> tab-completion of "\echo :" and it shows half-baked variables list. So
>> I thought probably we need to change complete_from_variables() more to
>> fix the problem.
>
>
> I understand now.
>
> I fixed it.
>
> I have a question.\echo probably should not to show empty known variable.
>
> data for autocomplete for \echo should be same as result of "\set"

Agreed. I think that only the variables having the set values should be
displayed in "\echo :" case. So I modified complete_from_variables()
so that the unset variables are not shown in that case but all the variables
are shown in the tab-completion of "\set".

>> >> +    else if (strcmp(prev2_wd, "\\set") == 0)
>> >> +    {
>> >> +        if (strcmp(prev_wd, "AUTOCOMMIT") == 0)
>> >>
>> >> ISTM that some psql variables like IGNOREEOF are not there. Why not?
>> >
>> >
>> > yes, there are not complete for DBNAME, ENCODING, FETCH_COUNT,
>> > HISTCONTROL,
>> > HISTFILE, HISTSIZE, HOST, IGNOREEOFF, PROMPT*,USER,  VERSION
>> >
>> > There are more reasons:
>> >
>> > * paremeter is not a enum (string, number or both): FETCH_COUNT, PROMPT,
>> > HISTSIZE, ..
>> >
>> > * variable is pseudo read only  - change has not any effect: DBNAME,
>> > ENCODING, VERSION
>>
>> So HISTCONTROL should be there because it doesn't have such reasons at
>> all?
>>
>
> yes

ISTM that you forgot to add HISTCONTROL to your patch. So I just added that.

I added the tab-completion for "\unset" command. Like "\echo :", only
the variables having the set values should be displayed in "\unset" case.

I changed complete_from_variables() so that it checks the memory size of
the variable array even when appending the known variables. If the memory
size is not enough, it's dynamically increased. Practically this check would
not be required because the initial size of the array is enough larger than
the number of the known variables. I added this as the safe-guard.

Typo: IGNOREEOFF -> IGNOREEOF

I removed the value "none" from the value list of "ECHO" because it's not
documented and a user might get confused when he or she sees the undocumented
value "none". Thought?

Regards,

--
Fujii Masao

Attachment

psql-var-complete-4_fujii.patch

Re: psql: show only failed queries

From

Pavel Stehule

Date:

08 August 2014, 22:17:09

Hi

2014-08-08 13:58 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:

On Fri, Aug 8, 2014 at 4:31 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
>
>
> 2014-08-07 7:10 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
>
>> On Thu, Aug 7, 2014 at 6:26 AM, Pavel Stehule <pavel.stehule@gmail.com>
>> wrote:
>> > Hello
>> >
>> > updated version patch in attachment
>>
>> Thanks! But ISTM you forgot to attached the patch.
>
>
> grr .. I am sorry

No problem. Thanks for the patch! Attached is the revised version of the patch.

>> >> + /* all psql known variables are included in list by default */
>> >> + for (known_varname = known_varnames; *known_varname;
>> >> known_varname++)
>> >> + varnames[nvars++] = pg_strdup(*known_varname);
>> >>
>> >> Don't we need to append both prefix and suffix to even known variables?
>> >
>> >
>> > ??? I am not sure if I understand well - probably system "read only"
>> > variables as DBNAME, USER, VERSION should not be there
>>
>> I had that question because complete_from_variables() is also called by
>> the
>> tab-completion of "\echo :" and it shows half-baked variables list. So
>> I thought probably we need to change complete_from_variables() more to
>> fix the problem.
>
>
> I understand now.
>
> I fixed it.
>
> I have a question.\echo probably should not to show empty known variable.
>
> data for autocomplete for \echo should be same as result of "\set"

Agreed. I think that only the variables having the set values should be
displayed in "\echo :" case. So I modified complete_from_variables()
so that the unset variables are not shown in that case but all the variables
are shown in the tab-completion of "\set".

>> >> + else if (strcmp(prev2_wd, "\\set") == 0)
>> >> + {
>> >> + if (strcmp(prev_wd, "AUTOCOMMIT") == 0)
>> >>
>> >> ISTM that some psql variables like IGNOREEOF are not there. Why not?
>> >
>> >
>> > yes, there are not complete for DBNAME, ENCODING, FETCH_COUNT,
>> > HISTCONTROL,
>> > HISTFILE, HISTSIZE, HOST, IGNOREEOFF, PROMPT*,USER, VERSION
>> >
>> > There are more reasons:
>> >
>> > * paremeter is not a enum (string, number or both): FETCH_COUNT, PROMPT,
>> > HISTSIZE, ..
>> >
>> > * variable is pseudo read only - change has not any effect: DBNAME,
>> > ENCODING, VERSION
>>
>> So HISTCONTROL should be there because it doesn't have such reasons at
>> all?
>>
>
> yes

ISTM that you forgot to add HISTCONTROL to your patch. So I just added that.

I added the tab-completion for "\unset" command. Like "\echo :", only
the variables having the set values should be displayed in "\unset" case.

perfect

I changed complete_from_variables() so that it checks the memory size of
the variable array even when appending the known variables. If the memory
size is not enough, it's dynamically increased. Practically this check would
not be required because the initial size of the array is enough larger than
the number of the known variables. I added this as the safe-guard.

Typo: IGNOREEOFF -> IGNOREEOF

I removed the value "none" from the value list of "ECHO" because it's not
documented and a user might get confused when he or she sees the undocumented
value "none". Thought?

isn't better to fix doc? I don't know any reason why we should not to support "none"

I looked to code, you removed a check against duplicate varname in list. Is it ok?

Regards

Pavel

Regards,

--
Fujii Masao

Re: psql: show only failed queries

From

Fujii Masao

Date:

11 August 2014, 15:59:54

On Sat, Aug 9, 2014 at 4:16 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> Hi
>
>
> 2014-08-08 13:58 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
>
>> On Fri, Aug 8, 2014 at 4:31 AM, Pavel Stehule <pavel.stehule@gmail.com>
>> wrote:
>> >
>> >
>> >
>> > 2014-08-07 7:10 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
>> >
>> >> On Thu, Aug 7, 2014 at 6:26 AM, Pavel Stehule <pavel.stehule@gmail.com>
>> >> wrote:
>> >> > Hello
>> >> >
>> >> > updated version patch in attachment
>> >>
>> >> Thanks! But ISTM you forgot to attached the patch.
>> >
>> >
>> > grr .. I am sorry
>>
>> No problem. Thanks for the patch! Attached is the revised version of the
>> patch.
>>
>> >> >> +    /* all psql known variables are included in list by default */
>> >> >> +    for (known_varname = known_varnames; *known_varname;
>> >> >> known_varname++)
>> >> >> +        varnames[nvars++] = pg_strdup(*known_varname);
>> >> >>
>> >> >> Don't we need to append both prefix and suffix to even known
>> >> >> variables?
>> >> >
>> >> >
>> >> > ??? I am not sure if I understand well - probably system "read only"
>> >> > variables as DBNAME, USER, VERSION should not be there
>> >>
>> >> I had that question because complete_from_variables() is also called by
>> >> the
>> >> tab-completion of "\echo :" and it shows half-baked variables list. So
>> >> I thought probably we need to change complete_from_variables() more to
>> >> fix the problem.
>> >
>> >
>> > I understand now.
>> >
>> > I fixed it.
>> >
>> > I have a question.\echo probably should not to show empty known
>> > variable.
>> >
>> > data for autocomplete for \echo should be same as result of "\set"
>>
>> Agreed. I think that only the variables having the set values should be
>> displayed in "\echo :" case. So I modified complete_from_variables()
>> so that the unset variables are not shown in that case but all the
>> variables
>> are shown in the tab-completion of "\set".
>>
>> >> >> +    else if (strcmp(prev2_wd, "\\set") == 0)
>> >> >> +    {
>> >> >> +        if (strcmp(prev_wd, "AUTOCOMMIT") == 0)
>> >> >>
>> >> >> ISTM that some psql variables like IGNOREEOF are not there. Why not?
>> >> >
>> >> >
>> >> > yes, there are not complete for DBNAME, ENCODING, FETCH_COUNT,
>> >> > HISTCONTROL,
>> >> > HISTFILE, HISTSIZE, HOST, IGNOREEOFF, PROMPT*,USER,  VERSION
>> >> >
>> >> > There are more reasons:
>> >> >
>> >> > * paremeter is not a enum (string, number or both): FETCH_COUNT,
>> >> > PROMPT,
>> >> > HISTSIZE, ..
>> >> >
>> >> > * variable is pseudo read only  - change has not any effect: DBNAME,
>> >> > ENCODING, VERSION
>> >>
>> >> So HISTCONTROL should be there because it doesn't have such reasons at
>> >> all?
>> >>
>> >
>> > yes
>>
>> ISTM that you forgot to add HISTCONTROL to your patch. So I just added
>> that.
>>
>> I added the tab-completion for "\unset" command. Like "\echo :", only
>> the variables having the set values should be displayed in "\unset" case.
>
>
> perfect
>
>>
>>
>> I changed complete_from_variables() so that it checks the memory size of
>> the variable array even when appending the known variables. If the memory
>> size is not enough, it's dynamically increased. Practically this check
>> would
>> not be required because the initial size of the array is enough larger
>> than
>> the number of the known variables. I added this as the safe-guard.
>>
>> Typo: IGNOREEOFF -> IGNOREEOF
>>
>> I removed the value "none" from the value list of "ECHO" because it's not
>> documented and a user might get confused when he or she sees the
>> undocumented
>> value "none". Thought?
>
>
> isn't better to fix doc? I don't know any reason why we should not to
> support "none"

I'm OK with this. The attached patch adds the support of "none" value both
in ECHO and HISTCONTROL variables (because HISTCONTROL had the same
problem as ECHO had), and also adds the description of that value into
the document.

> I looked to code, you removed a check against duplicate varname in list. Is
> it ok?

Oh, just revived that code.

Regards,

--
Fujii Masao

Attachment

psql-var-complete-5_fujii.patch

Re: psql: show only failed queries

From

Pavel Stehule

Date:

11 August 2014, 22:42:15

2014-08-11 14:59 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:

On Sat, Aug 9, 2014 at 4:16 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> Hi
>
>
> 2014-08-08 13:58 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
>
>> On Fri, Aug 8, 2014 at 4:31 AM, Pavel Stehule <pavel.stehule@gmail.com>
>> wrote:
>> >
>> >
>> >
>> > 2014-08-07 7:10 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
>> >
>> >> On Thu, Aug 7, 2014 at 6:26 AM, Pavel Stehule <pavel.stehule@gmail.com>
>> >> wrote:
>> >> > Hello
>> >> >
>> >> > updated version patch in attachment
>> >>
>> >> Thanks! But ISTM you forgot to attached the patch.
>> >
>> >
>> > grr .. I am sorry
>>
>> No problem. Thanks for the patch! Attached is the revised version of the
>> patch.
>>
>> >> >> + /* all psql known variables are included in list by default */
>> >> >> + for (known_varname = known_varnames; *known_varname;
>> >> >> known_varname++)
>> >> >> + varnames[nvars++] = pg_strdup(*known_varname);
>> >> >>
>> >> >> Don't we need to append both prefix and suffix to even known
>> >> >> variables?
>> >> >
>> >> >
>> >> > ??? I am not sure if I understand well - probably system "read only"
>> >> > variables as DBNAME, USER, VERSION should not be there
>> >>
>> >> I had that question because complete_from_variables() is also called by
>> >> the
>> >> tab-completion of "\echo :" and it shows half-baked variables list. So
>> >> I thought probably we need to change complete_from_variables() more to
>> >> fix the problem.
>> >
>> >
>> > I understand now.
>> >
>> > I fixed it.
>> >
>> > I have a question.\echo probably should not to show empty known
>> > variable.
>> >
>> > data for autocomplete for \echo should be same as result of "\set"
>>
>> Agreed. I think that only the variables having the set values should be
>> displayed in "\echo :" case. So I modified complete_from_variables()
>> so that the unset variables are not shown in that case but all the
>> variables
>> are shown in the tab-completion of "\set".
>>
>> >> >> + else if (strcmp(prev2_wd, "\\set") == 0)
>> >> >> + {
>> >> >> + if (strcmp(prev_wd, "AUTOCOMMIT") == 0)
>> >> >>
>> >> >> ISTM that some psql variables like IGNOREEOF are not there. Why not?
>> >> >
>> >> >
>> >> > yes, there are not complete for DBNAME, ENCODING, FETCH_COUNT,
>> >> > HISTCONTROL,
>> >> > HISTFILE, HISTSIZE, HOST, IGNOREEOFF, PROMPT*,USER, VERSION
>> >> >
>> >> > There are more reasons:
>> >> >
>> >> > * paremeter is not a enum (string, number or both): FETCH_COUNT,
>> >> > PROMPT,
>> >> > HISTSIZE, ..
>> >> >
>> >> > * variable is pseudo read only - change has not any effect: DBNAME,
>> >> > ENCODING, VERSION
>> >>
>> >> So HISTCONTROL should be there because it doesn't have such reasons at
>> >> all?
>> >>
>> >
>> > yes
>>
>> ISTM that you forgot to add HISTCONTROL to your patch. So I just added
>> that.
>>
>> I added the tab-completion for "\unset" command. Like "\echo :", only
>> the variables having the set values should be displayed in "\unset" case.
>
>
> perfect
>
>>
>>
>> I changed complete_from_variables() so that it checks the memory size of
>> the variable array even when appending the known variables. If the memory
>> size is not enough, it's dynamically increased. Practically this check
>> would
>> not be required because the initial size of the array is enough larger
>> than
>> the number of the known variables. I added this as the safe-guard.
>>
>> Typo: IGNOREEOFF -> IGNOREEOF
>>
>> I removed the value "none" from the value list of "ECHO" because it's not
>> documented and a user might get confused when he or she sees the
>> undocumented
>> value "none". Thought?
>
>
> isn't better to fix doc? I don't know any reason why we should not to
> support "none"

I'm OK with this. The attached patch adds the support of "none" value both
in ECHO and HISTCONTROL variables (because HISTCONTROL had the same
problem as ECHO had), and also adds the description of that value into
the document.

> I looked to code, you removed a check against duplicate varname in list. Is
> it ok?

Oh, just revived that code.

yes, It is looking well

regards

Pavel

Regards,

--
Fujii Masao

Re: psql: show only failed queries

From

Fujii Masao

Date:

12 August 2014, 06:16:45

On Tue, Aug 12, 2014 at 4:41 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
>
>
> 2014-08-11 14:59 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
>
>> On Sat, Aug 9, 2014 at 4:16 AM, Pavel Stehule <pavel.stehule@gmail.com>
>> wrote:
>> > Hi
>> >
>> >
>> > 2014-08-08 13:58 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
>> >
>> >> On Fri, Aug 8, 2014 at 4:31 AM, Pavel Stehule <pavel.stehule@gmail.com>
>> >> wrote:
>> >> >
>> >> >
>> >> >
>> >> > 2014-08-07 7:10 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
>> >> >
>> >> >> On Thu, Aug 7, 2014 at 6:26 AM, Pavel Stehule
>> >> >> <pavel.stehule@gmail.com>
>> >> >> wrote:
>> >> >> > Hello
>> >> >> >
>> >> >> > updated version patch in attachment
>> >> >>
>> >> >> Thanks! But ISTM you forgot to attached the patch.
>> >> >
>> >> >
>> >> > grr .. I am sorry
>> >>
>> >> No problem. Thanks for the patch! Attached is the revised version of
>> >> the
>> >> patch.
>> >>
>> >> >> >> +    /* all psql known variables are included in list by default
>> >> >> >> */
>> >> >> >> +    for (known_varname = known_varnames; *known_varname;
>> >> >> >> known_varname++)
>> >> >> >> +        varnames[nvars++] = pg_strdup(*known_varname);
>> >> >> >>
>> >> >> >> Don't we need to append both prefix and suffix to even known
>> >> >> >> variables?
>> >> >> >
>> >> >> >
>> >> >> > ??? I am not sure if I understand well - probably system "read
>> >> >> > only"
>> >> >> > variables as DBNAME, USER, VERSION should not be there
>> >> >>
>> >> >> I had that question because complete_from_variables() is also called
>> >> >> by
>> >> >> the
>> >> >> tab-completion of "\echo :" and it shows half-baked variables list.
>> >> >> So
>> >> >> I thought probably we need to change complete_from_variables() more
>> >> >> to
>> >> >> fix the problem.
>> >> >
>> >> >
>> >> > I understand now.
>> >> >
>> >> > I fixed it.
>> >> >
>> >> > I have a question.\echo probably should not to show empty known
>> >> > variable.
>> >> >
>> >> > data for autocomplete for \echo should be same as result of "\set"
>> >>
>> >> Agreed. I think that only the variables having the set values should be
>> >> displayed in "\echo :" case. So I modified complete_from_variables()
>> >> so that the unset variables are not shown in that case but all the
>> >> variables
>> >> are shown in the tab-completion of "\set".
>> >>
>> >> >> >> +    else if (strcmp(prev2_wd, "\\set") == 0)
>> >> >> >> +    {
>> >> >> >> +        if (strcmp(prev_wd, "AUTOCOMMIT") == 0)
>> >> >> >>
>> >> >> >> ISTM that some psql variables like IGNOREEOF are not there. Why
>> >> >> >> not?
>> >> >> >
>> >> >> >
>> >> >> > yes, there are not complete for DBNAME, ENCODING, FETCH_COUNT,
>> >> >> > HISTCONTROL,
>> >> >> > HISTFILE, HISTSIZE, HOST, IGNOREEOFF, PROMPT*,USER,  VERSION
>> >> >> >
>> >> >> > There are more reasons:
>> >> >> >
>> >> >> > * paremeter is not a enum (string, number or both): FETCH_COUNT,
>> >> >> > PROMPT,
>> >> >> > HISTSIZE, ..
>> >> >> >
>> >> >> > * variable is pseudo read only  - change has not any effect:
>> >> >> > DBNAME,
>> >> >> > ENCODING, VERSION
>> >> >>
>> >> >> So HISTCONTROL should be there because it doesn't have such reasons
>> >> >> at
>> >> >> all?
>> >> >>
>> >> >
>> >> > yes
>> >>
>> >> ISTM that you forgot to add HISTCONTROL to your patch. So I just added
>> >> that.
>> >>
>> >> I added the tab-completion for "\unset" command. Like "\echo :", only
>> >> the variables having the set values should be displayed in "\unset"
>> >> case.
>> >
>> >
>> > perfect
>> >
>> >>
>> >>
>> >> I changed complete_from_variables() so that it checks the memory size
>> >> of
>> >> the variable array even when appending the known variables. If the
>> >> memory
>> >> size is not enough, it's dynamically increased. Practically this check
>> >> would
>> >> not be required because the initial size of the array is enough larger
>> >> than
>> >> the number of the known variables. I added this as the safe-guard.
>> >>
>> >> Typo: IGNOREEOFF -> IGNOREEOF
>> >>
>> >> I removed the value "none" from the value list of "ECHO" because it's
>> >> not
>> >> documented and a user might get confused when he or she sees the
>> >> undocumented
>> >> value "none". Thought?
>> >
>> >
>> > isn't better to fix doc? I don't know any reason why we should not to
>> > support "none"
>>
>> I'm OK with this. The attached patch adds the support of "none" value both
>> in ECHO and HISTCONTROL variables (because HISTCONTROL had the same
>> problem as ECHO had), and also adds the description of that value into
>> the document.
>>
>> > I looked to code, you removed a check against duplicate varname in list.
>> > Is
>> > it ok?
>>
>> Oh, just revived that code.
>
>
> yes, It is looking well

Ok, committed!

Regards,

-- 
Fujii Masao

Re: psql: show only failed queries

From

Pavel Stehule

Date:

12 August 2014, 06:18:58

>>
>> Oh, just revived that code.
>
>
> yes, It is looking well

Ok, committed!

super

thank you very much

Regards

Pavel

Regards,

--
Fujii Masao