psql: show only failed queries

Started by Pavel Stehuleabout 12 years ago38 messageshackers
Jump to latest
#1Pavel Stehule
pavel.stehule@gmail.com

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

Attachments:

echo-error.patchtext/x-patch; charset=US-ASCII; name=echo-error.patchDownload+26-0
#2Fabrízio de Royes Mello
fabriziomello@gmail.com
In reply to: Pavel Stehule (#1)
Re: psql: show only failed queries

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

Show quoted text

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

#3Pavel Stehule
pavel.stehule@gmail.com
In reply to: Fabrízio de Royes Mello (#2)
Re: psql: show only failed queries

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

Show quoted text

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

#4Pavel Stehule
pavel.stehule@gmail.com
In reply to: Pavel Stehule (#3)
Re: psql: show only failed queries

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

Show quoted text

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

Attachments:

echo_error.patchtext/x-patch; charset=US-ASCII; name=echo_error.patchDownload+28-1
#5Peter Eisentraut
peter_e@gmx.net
In reply to: Pavel Stehule (#4)
Re: psql: show only failed queries

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

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

#6Pavel Stehule
pavel.stehule@gmail.com
In reply to: Peter Eisentraut (#5)
Re: psql: show only failed queries

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

Attachments:

echo_error.patchtext/x-patch; charset=US-ASCII; name=echo_error.patchDownload+28-1
#7Samrat Revagade
revagade.samrat@gmail.com
In reply to: Pavel Stehule (#6)
Re: psql: show only failed queries

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

#8Pavel Stehule
pavel.stehule@gmail.com
In reply to: Samrat Revagade (#7)
Re: psql: show only failed queries

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

Show quoted text

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

Attachments:

echo-error-01.patchtext/x-patch; charset=US-ASCII; name=echo-error-01.patchDownload+25-1
#9Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Pavel Stehule (#8)
Re: psql: show only failed queries

ECHO_HIDDEN?

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

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

#10Samrat Revagade
revagade.samrat@gmail.com
In reply to: Pavel Stehule (#8)
Re: psql: show only failed queries

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.

#11Pavel Stehule
pavel.stehule@gmail.com
In reply to: Samrat Revagade (#10)
Re: psql: show only failed queries

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

#12Rajeev rastogi
rajeev.rastogi@huawei.com
In reply to: Samrat Revagade (#10)
Re: psql: show only failed queries

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.

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

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.

Thanks and Regards,
Kumar Rajeev Rastogi

#13Pavel Stehule
pavel.stehule@gmail.com
In reply to: Rajeev rastogi (#12)
Re: psql: show only failed queries

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

Show quoted text

*Thanks and Regards,*

*Kumar Rajeev Rastogi *

#14Rajeev rastogi
rajeev.rastogi@huawei.com
In reply to: Pavel Stehule (#13)
Re: psql: show only failed queries

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.

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.
Thanks and Regards,
Kumar Rajeev Rastogi

#15Pavel Stehule
pavel.stehule@gmail.com
In reply to: Rajeev rastogi (#14)
Re: psql: show only failed queries

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

Show quoted text

*Thanks and Regards,*

*Kumar Rajeev Rastogi*

Attachments:

echo-error-02.patchtext/x-patch; charset=US-ASCII; name=echo-error-02.patchDownload+42-1
#16Abhijit Menon-Sen
ams@2ndQuadrant.com
In reply to: Pavel Stehule (#15)
Re: psql: show only failed queries

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

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

#17Pavel Stehule
pavel.stehule@gmail.com
In reply to: Abhijit Menon-Sen (#16)
Re: psql: show only failed queries

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

Show quoted text

-- Abhijit

Attachments:

echo-error-03.patchtext/x-patch; charset=US-ASCII; name=echo-error-03.patchDownload+42-1
#18Fujii Masao
masao.fujii@gmail.com
In reply to: Pavel Stehule (#17)
Re: psql: show only failed queries

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

+    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

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

#19Pavel Stehule
pavel.stehule@gmail.com
In reply to: Fujii Masao (#18)
Re: psql: show only failed queries

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

Show quoted text

Regards,

--
Fujii Masao

Attachments:

echo-error-04.patchtext/x-patch; charset=US-ASCII; name=echo-error-04.patchDownload+37-2
#20Fujii Masao
masao.fujii@gmail.com
In reply to: Pavel Stehule (#19)
Re: psql: show only failed queries

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

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

#21Pavel Stehule
pavel.stehule@gmail.com
In reply to: Fujii Masao (#20)
#22Fujii Masao
masao.fujii@gmail.com
In reply to: Pavel Stehule (#21)
#23Pavel Stehule
pavel.stehule@gmail.com
In reply to: Fujii Masao (#22)
#24Pavel Stehule
pavel.stehule@gmail.com
In reply to: Pavel Stehule (#23)
#25Fujii Masao
masao.fujii@gmail.com
In reply to: Pavel Stehule (#24)
#26Pavel Stehule
pavel.stehule@gmail.com
In reply to: Fujii Masao (#25)
#27Fujii Masao
masao.fujii@gmail.com
In reply to: Pavel Stehule (#26)
#28Pavel Stehule
pavel.stehule@gmail.com
In reply to: Fujii Masao (#27)
#29Fujii Masao
masao.fujii@gmail.com
In reply to: Pavel Stehule (#28)
#30Pavel Stehule
pavel.stehule@gmail.com
In reply to: Fujii Masao (#29)
#31Fujii Masao
masao.fujii@gmail.com
In reply to: Pavel Stehule (#30)
#32Pavel Stehule
pavel.stehule@gmail.com
In reply to: Fujii Masao (#31)
#33Fujii Masao
masao.fujii@gmail.com
In reply to: Pavel Stehule (#32)
#34Pavel Stehule
pavel.stehule@gmail.com
In reply to: Fujii Masao (#33)
#35Fujii Masao
masao.fujii@gmail.com
In reply to: Pavel Stehule (#34)
#36Pavel Stehule
pavel.stehule@gmail.com
In reply to: Fujii Masao (#35)
#37Fujii Masao
masao.fujii@gmail.com
In reply to: Pavel Stehule (#36)
#38Pavel Stehule
pavel.stehule@gmail.com
In reply to: Fujii Masao (#37)