Re: psql: show only failed queries

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

On Sat, Mar 1, 2014 at 8:01 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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

2014-03-04 6:35 GMT+01:00 Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>:

>
>
>
> On Sat, Mar 1, 2014 at 8:01 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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
>

Hello

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

current state:

[pavel(at)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(at)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(dot)stehule(at)gmail(dot)com>:

>
>
>
> 2014-03-04 6:35 GMT+01:00 Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com
> >:
>
>
>>
>>
>> On Sat, Mar 1, 2014 at 8:01 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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
>>
>
>

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

2014-06-04 18:16 GMT+02:00 Peter Eisentraut <peter_e(at)gmx(dot)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

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(dot)stehule(at)gmail(dot)com>
wrote:

>
>
>
> 2014-06-04 18:16 GMT+02:00 Peter Eisentraut <peter_e(at)gmx(dot)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(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

--
Regards,

Samrat Revgade

2014-06-25 12:32 GMT+02:00 Samrat Revagade <revagade(dot)samrat(at)gmail(dot)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(at)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(at)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(dot)stehule(at)gmail(dot)com>
> wrote:
>
>>
>>
>>
>> 2014-06-04 18:16 GMT+02:00 Peter Eisentraut <peter_e(at)gmx(dot)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(at)postgresql(dot)org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgsql-hackers
>>
>>
>
>
> --
> Regards,
>
> Samrat Revgade
>

ECHO_HIDDEN?

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

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

2014-06-26 8:22 GMT+02:00 Samrat Revagade <revagade(dot)samrat(at)gmail(dot)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

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

2014-06-30 8:17 GMT+02:00 Rajeev rastogi <rajeev(dot)rastogi(at)huawei(dot)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 *
>

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

2014-06-30 11:20 GMT+02:00 Rajeev rastogi <rajeev(dot)rastogi(at)huawei(dot)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*
>

At 2014-06-30 12:48:30 +0200, pavel(dot)stehule(at)gmail(dot)com wrote:
>
> + <para>
> + Print a failed SQL commands to standard error output. This is
> + equivalent to setting the variable <varname>ECHO</varname> to
> + <literal>errors</literal>.

No "a", just "Print failed SQL commands …".

> - <option>-e</option>.
> + <option>-e</option>. If set to <literal>error</literal> then only
> + failed queries are displayed.

Should be "errors" here, not "error".

> printf(_(" -a, --echo-all echo all input from script\n"));
> + printf(_(" -b --echo-errors echo failed commands sent to server\n"));
> printf(_(" -e, --echo-queries echo commands sent to server\n"));

Should have a comma after -b to match other options. Also I would remove
"sent to server" from the description: "echo failed commands" is fine.

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

2014-06-30 13:01 GMT+02:00 Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com>:

> At 2014-06-30 12:48:30 +0200, pavel(dot)stehule(at)gmail(dot)com wrote:
> >
> > + <para>
> > + Print a failed SQL commands to standard error output. This is
> > + equivalent to setting the variable <varname>ECHO</varname> to
> > + <literal>errors</literal>.
>
> No "a", just "Print failed SQL commands …".
>
> > - <option>-e</option>.
> > + <option>-e</option>. If set to <literal>error</literal> then
> only
> > + failed queries are displayed.
>
> Should be "errors" here, not "error".
>
> > printf(_(" -a, --echo-all echo all input from
> script\n"));
> > + printf(_(" -b --echo-errors echo failed commands sent to
> server\n"));
> > printf(_(" -e, --echo-queries echo commands sent to
> server\n"));
>
> Should have a comma after -b to match other options. Also I would remove
> "sent to server" from the description: "echo failed commands" is fine.
>

fixed

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

On Mon, Jun 30, 2014 at 8:33 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>
>
>
> 2014-06-30 13:01 GMT+02:00 Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com>:
>
>> At 2014-06-30 12:48:30 +0200, pavel(dot)stehule(at)gmail(dot)com wrote:
>> >
>> > + <para>
>> > + Print a failed SQL commands to standard error output. This is
>> > + equivalent to setting the variable <varname>ECHO</varname> to
>> > + <literal>errors</literal>.
>>
>> No "a", just "Print failed SQL commands …".
>>
>> > - <option>-e</option>.
>> > + <option>-e</option>. If set to <literal>error</literal> then
>> > only
>> > + failed queries are displayed.
>>
>> Should be "errors" here, not "error".
>>
>> > printf(_(" -a, --echo-all echo all input from
>> > script\n"));
>> > + printf(_(" -b --echo-errors echo failed commands sent to
>> > server\n"));
>> > printf(_(" -e, --echo-queries echo commands sent to
>> > server\n"));
>>
>> Should have a comma after -b to match other options. Also I would remove
>> "sent to server" from the description: "echo failed commands" is fine.
>
>
> fixed

$ psql -b
bin/psql: invalid option -- 'b'
Try "psql --help" for more information.

I got this error. ISTM you forgot to add 'b' into the third argument of
getopt_long in startup.c.

<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

Hi

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

> On Mon, Jun 30, 2014 at 8:33 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> >
> >
> >
> > 2014-06-30 13:01 GMT+02:00 Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com>:
> >
> >> At 2014-06-30 12:48:30 +0200, pavel(dot)stehule(at)gmail(dot)com wrote:
> >> >
> >> > + <para>
> >> > + Print a failed SQL commands to standard error output. This is
> >> > + equivalent to setting the variable <varname>ECHO</varname> to
> >> > + <literal>errors</literal>.
> >>
> >> No "a", just "Print failed SQL commands …".
> >>
> >> > - <option>-e</option>.
> >> > + <option>-e</option>. If set to <literal>error</literal> then
> >> > only
> >> > + failed queries are displayed.
> >>
> >> Should be "errors" here, not "error".
> >>
> >> > printf(_(" -a, --echo-all echo all input from
> >> > script\n"));
> >> > + printf(_(" -b --echo-errors echo failed commands sent
> to
> >> > server\n"));
> >> > printf(_(" -e, --echo-queries echo commands sent to
> >> > server\n"));
> >>
> >> Should have a comma after -b to match other options. Also I would remove
> >> "sent to server" from the description: "echo failed commands" is fine.
> >
> >
> > fixed
>
> $ psql -b
> bin/psql: invalid option -- 'b'
> Try "psql --help" for more information.
>
> I got this error. ISTM you forgot to add 'b' into the third argument of
> getopt_long in startup.c.
>
>
fixed

> <application>psql</application> merely prints all queries as
> they are sent to the server. The switch for this is
> - <option>-e</option>.
> + <option>-e</option>. If set to <literal>errors</literal> then only
> + failed queries are displayed.
>
> I think that where failed queries are output should be documented here.
> Otherwise users might misunderstand they are output to standard output
> like ECHO=all and queries do.
>
> It's better to add "The switch for this is <option>-b</option>." into the
> doc.
>

fixed

> + else if (strcmp(prev2_wd, "\\set") == 0)
> + {
> + if (strcmp(prev_wd, "ECHO") == 0)
> + {
> + static const char *const my_list[] =
> + {"none", "errors", "queries", "all", NULL};
> +
> + COMPLETE_WITH_LIST_CS(my_list);
> + }
> + else if (strcmp(prev_wd, "ECHO_HIDDEN") == 0)
> + {
> + static const char *const my_list[] =
> + {"noexec", "off", "on", NULL};
> +
> + COMPLETE_WITH_LIST_CS(my_list);
> + }
> + }
>
> I think that adding tab-completions of psql variables is good, but
> adding those of only ECHO and ECHO_HIDDEN seems half-baked.
> Probably this part should be split into separate patch.
>

fixed

please, see updated patch in attachment

Thank you

Regards

Pavel

>
> Regards,
>
> --
> Fujii Masao
>

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

Thanks for updating the patch!

Barring any objection, I will commit this patch except tab-completion part.

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

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

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

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

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

Regards

Pavel

> Regards,
>
> --
> Fujii Masao
>

On Wed, Jul 9, 2014 at 9:44 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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

2014-07-10 7:32 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:

> On Wed, Jul 9, 2014 at 9:44 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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
>

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(dot)stehule(at)gmail(dot)com>:

>
>
>
> 2014-07-10 7:32 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
>
> On Wed, Jul 9, 2014 at 9:44 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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
>>
>
>

On Thu, Jul 10, 2014 at 9:56 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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

2014-07-15 11:29 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:

> On Thu, Jul 10, 2014 at 9:56 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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
>

On Tue, Jul 15, 2014 at 7:01 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>
>
>
> 2014-07-15 11:29 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
>
>> On Thu, Jul 10, 2014 at 9:56 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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

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

> On Tue, Jul 15, 2014 at 7:01 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> >
> >
> >
> > 2014-07-15 11:29 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
> >
> >> On Thu, Jul 10, 2014 at 9:56 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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
>

On Wed, Jul 16, 2014 at 4:34 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>
>
>
> 2014-07-15 12:07 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
>
>> On Tue, Jul 15, 2014 at 7:01 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
>> wrote:
>> >
>> >
>> >
>> > 2014-07-15 11:29 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
>> >
>> >> On Thu, Jul 10, 2014 at 9:56 PM, Pavel Stehule
>> >> <pavel(dot)stehule(at)gmail(dot)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

Hello

updated version patch in attachment

2014-08-05 13:31 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:

> On Wed, Jul 16, 2014 at 4:34 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> >
> >
> >
> > 2014-07-15 12:07 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
> >
> >> On Tue, Jul 15, 2014 at 7:01 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com
> >
> >> wrote:
> >> >
> >> >
> >> >
> >> > 2014-07-15 11:29 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
> >> >
> >> >> On Thu, Jul 10, 2014 at 9:56 PM, Pavel Stehule
> >> >> <pavel(dot)stehule(at)gmail(dot)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
>

On Thu, Aug 7, 2014 at 6:26 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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

2014-08-07 7:10 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:

> On Thu, Aug 7, 2014 at 6:26 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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
>

On Fri, Aug 8, 2014 at 4:31 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>
>
>
> 2014-08-07 7:10 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
>
>> On Thu, Aug 7, 2014 at 6:26 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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

Hi

2014-08-08 13:58 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:

> On Fri, Aug 8, 2014 at 4:31 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> >
> >
> >
> > 2014-08-07 7:10 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
> >
> >> On Thu, Aug 7, 2014 at 6:26 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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
>

On Sat, Aug 9, 2014 at 4:16 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> Hi
>
>
> 2014-08-08 13:58 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
>
>> On Fri, Aug 8, 2014 at 4:31 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
>> wrote:
>> >
>> >
>> >
>> > 2014-08-07 7:10 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
>> >
>> >> On Thu, Aug 7, 2014 at 6:26 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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

2014-08-11 14:59 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:

> On Sat, Aug 9, 2014 at 4:16 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> > Hi
> >
> >
> > 2014-08-08 13:58 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
> >
> >> On Fri, Aug 8, 2014 at 4:31 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> >> wrote:
> >> >
> >> >
> >> >
> >> > 2014-08-07 7:10 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
> >> >
> >> >> On Thu, Aug 7, 2014 at 6:26 AM, Pavel Stehule <
> pavel(dot)stehule(at)gmail(dot)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
>

On Tue, Aug 12, 2014 at 4:41 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>
>
>
> 2014-08-11 14:59 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
>
>> On Sat, Aug 9, 2014 at 4:16 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
>> wrote:
>> > Hi
>> >
>> >
>> > 2014-08-08 13:58 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
>> >
>> >> On Fri, Aug 8, 2014 at 4:31 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
>> >> wrote:
>> >> >
>> >> >
>> >> >
>> >> > 2014-08-07 7:10 GMT+02:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
>> >> >
>> >> >> On Thu, Aug 7, 2014 at 6:26 AM, Pavel Stehule
>> >> >> <pavel(dot)stehule(at)gmail(dot)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

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

super

thank you very much

Regards

Pavel

>
> Regards,
>
> --
> Fujii Masao
>