Thread: Re: Allow default \watch interval in psql to be configured

Re: Allow default \watch interval in psql to be configured

From
Heikki Linnakangas
Date:
On 09/10/2024 16:08, Daniel Gustafsson wrote:
> Scratching an old itch; I've long wanted to be able to set the default interval
> for \watch in psql since I almost never want a 2 second wait.  The interval can
> of course be set by passing it to \watch but it's handy during testing and
> debugging to save that with just quick \watch.
> 
> The attached adds a new variable, WATCH_INTERVAL, which is used as the default
> value in case no value is defined when executing the command.  The defualt of
> this remains at 2 seconds as it is now.  The count and min_rows values are not
> affected by this as those seem less meaningful to set defaults on.

../src/bin/psql/startup.c:953:80: error: too many arguments to function 
call, expected 4, have 5
         return ParseVariableDouble(newval, "WATCH_INTERVAL", 
&pset.watch_interval, 0, 1000);
                ~~~~~~~~~~~~~~~~~~~ 
               ^~~~
../src/bin/psql/variables.h:84:7: note: 'ParseVariableDouble' declared here
bool            ParseVariableDouble(const char *value, const char *name,
                 ^
1 error generated.

I guess the '1000' was supposed to be the maximum, but 
ParseVariableDouble doesn't take a maximum.

After fixing that by removing the '1000' argument:


postgres=# \set WATCH_INTERVAL -10
invalid value "-10" for "WATCH_INTERVAL": must be greater than 0.00

That's a little inaccurate: 0 is also accepted, so should be "must be 
greater than *or equal to* 0". Or maybe "cannot be negative". -0 is also 
accepted, though.

> +        This variable set the default interval which <command>\watch</command>

set -> sets

> +       HELP0("  WATCH_INTERVAL\n"
> +                 "    number of seconds \\watch waits beetween queries\n");

beetween -> between

-- 
Heikki Linnakangas
Neon (https://neon.tech)




Re: Allow default \watch interval in psql to be configured

From
Pavel Stehule
Date:


čt 10. 10. 2024 v 2:02 odesílatel Michael Paquier <michael@paquier.xyz> napsal:
On Wed, Oct 09, 2024 at 04:24:27PM +0200, Daniel Gustafsson wrote:
> Fixed.

-        double        sleep = 2;
+        double        sleep = pset.watch_interval;

This forces the use of seconds as unit.  The interval values I have
been using a lot myself are between 0.2s and 0.5s because I usually
want a lot more granularity in my lookups than the 1s interval.  Could
it be better to allow values lower than 1s or let this value be a
string with optional "s" or "ms" units?

Linux "watch" uses just seconds. If I remember correctly the psql doesn't use units in settings, so I prefer just the value from interval 0.1 .. 3600 * n

and the number can be rounded to 0.1

Regards

Pavel

 
--
Michael

Re: Allow default \watch interval in psql to be configured

From
Daniel Gustafsson
Date:
> On 10 Oct 2024, at 02:01, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Wed, Oct 09, 2024 at 04:24:27PM +0200, Daniel Gustafsson wrote:
>> Fixed.
>
> -        double        sleep = 2;
> +        double        sleep = pset.watch_interval;
>
> This forces the use of seconds as unit.  The interval values I have
> been using a lot myself are between 0.2s and 0.5s because I usually
> want a lot more granularity in my lookups than the 1s interval.  Could
> it be better to allow values lower than 1s or let this value be a
> string with optional "s" or "ms" units?

I'm not sure I follow, it's true that the unit is seconds but the patch doesn't
change the ability to use fractions of a second that we already support today.

db=# \echo :WATCH_INTERVAL
2
db=# \set WATCH_INTERVAL 0.1
db=# \echo :WATCH_INTERVAL
0.1
db=# select 1;
 ?column?
----------
        1
(1 row)

danielg=# \watch
Thu Oct 10 09:32:05 2024 (every 0.1s)

 ?column?
----------
        1
(1 row)

Thu Oct 10 09:32:05 2024 (every 0.1s)

 ?column?
----------
        1
(1 row)

Or did I misunderstand your email?

We could support passing in an optional unit, and assume the unit to be seconds
if none was used, but it doesn't really fit nicely with the current API we have
so I wonder if the added complexity is worth it?

--
Daniel Gustafsson




Re: Allow default \watch interval in psql to be configured

From
Kirill Reshke
Date:
On Wed, 9 Oct 2024 at 19:24, Daniel Gustafsson <daniel@yesql.se> wrote:
>
> > On 9 Oct 2024, at 16:05, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>
> Thanks for looking!
>
> > I guess the '1000' was supposed to be the maximum, but ParseVariableDouble doesn't take a maximum.
>
> Doh, I had a max parameter during hacking but removed it since I didn't see a
> clear usecase for it.  Since it's not an externally published API we can
> alwasys add it when there is need.  Clearly I managed to generate the patch at
> the wrong time without noticing.  Fixed.
>
> > That's a little inaccurate: 0 is also accepted, so should be "must be greater than *or equal to* 0". Or maybe
"cannotbe negative". -0 is also accepted, though.
 
>
> I changed to "must be at least XX" to keep the message short.
>
> > set -> sets
> >
> > beetween -> between
>
> Fixed.
>
> --
> Daniel Gustafsson
>

Hi!
I'm mostly fine with this patch, but maybe we need to handle `errno ==
ERANGE` inside ParseVariableDouble and provide a better error msg in
this case?
Something like:
```
reshke=# \set WATCH_INTERVAL -1e-309
underflow while parsing parameter
```
Also, maybe we should provide `double max` arg to the
ParseVariableDouble function, because this is a general-use function?


-- 
Best regards,
Kirill Reshke



Re: Allow default \watch interval in psql to be configured

From
Masahiro Ikeda
Date:
Hi,

Thanks for developing this useful feature!
I've tested it and reviewed the patch. I'd like to provide some 
feedback.

(1)

I tested with v3 patch and found the following compile error.
It seems that math.h needs to be included in variables.c.

variables.c: In function 'ParseVariableDouble':
variables.c:220:54: error: 'HUGE_VAL' undeclared (first use in this 
function)
   220 |                          (dblval == 0.0 || dblval >= HUGE_VAL || 
dblval <= -HUGE_VAL))
       |                                                      ^~~~~~~~
variables.c:220:54: note: each undeclared identifier is reported only 
once for each function it appears in
variables.c:232:1: warning: control reaches end of non-void function 
[-Wreturn-type]
   232 | }
       | ^

(2)

Although the error handling logic is being discussed now, I think it 
would be better,
at least, to align the logic and messages of exec_command_watch() and 
ParseVariableDouble().
I understand that the error message 'for "WATCH_INTERVAL"' will remain 
as a difference
since it should be considered when loaded via psqlrc.

# v3 patch test result

* minus value
=# \watch i=-1
\watch: incorrect interval value "-1"

=# \set WATCH_INTERVAL -1
invalid value "-1" for "WATCH_INTERVAL": must be at least 0.00

* not interval value
=# \watch i=1s
\watch: incorrect interval value "1s"

=# \set WATCH_INTERVAL 1s
invalid value "1s" for "WATCH_INTERVAL"

* maximum value
=# \watch i=1e500
\watch: incorrect interval value "1e500"

=# \set WATCH_INTERVAL 1e500
"1e500" is out of range for "WATCH_INTERVAL"


(3)

ParseVariableDouble() doesn't have a comment yet, though you may be 
planning to add one later.


(4)

I believe the default value is 2 after the WATCH_INTERVAL is specified 
because \unset
WATCH_INTERVAL sets it to '2'. So, why not update the following sentence 
accordingly?

-        of rows. Wait the specified number of seconds (default 2) 
between executions.
-        For backwards compatibility,
+        of rows. Wait the specified number of seconds (default 2, which 
can be
+        changed with the variable
+        between executions.  For backwards compatibility,

For example,

> Wait <varname>WATCH_INTERVAL</varname> seconds unless the interval 
> option is specified.
> If the interval option is specified, wait the specified number of 
> seconds instead.

+        This variable sets the default interval which 
<command>\watch</command>
+        waits between executing the query.  Specifying an interval in 
the
+        command overrides this variable.

> This variable sets the interval in seconds that 
> <command>\watch</command> waits
> between executions. The default value is 2.0.


(5)

I think it's better to replace queries with executions because the 
\watch
documentation says so.

+    HELP0("  WATCH_INTERVAL\n"
+          "    number of seconds \\watch waits between queries\n");


Regards,
-- 
Masahiro Ikeda
NTT DATA CORPORATION



Re: Allow default \watch interval in psql to be configured

From
Greg Sabino Mullane
Date:
Patch looks good. One minor issue:

greg=# \set WATCH_INTERVAL -1
invalid value "-1" for "WATCH_INTERVAL": must be greater than 0.00
greg=# \set WATCH_INTERVAL 0.00
greg=#

We should disallow 0 as the error message implies

I've long wanted to be able to set the default interval for \watch in psql since I almost never want a 2 second wait. 

Curious what other's personal defaults are? I usually use 1 second or 0.5 depending on things.


Cheers,
Greg

--
Enterprise Postgres Software Products & Tech Support

Re: Allow default \watch interval in psql to be configured

From
Daniel Gustafsson
Date:
On 13 Mar 2025, at 15:03, Greg Sabino Mullane <htamfids@gmail.com> wrote:

Patch looks good. One minor issue:

greg=# \set WATCH_INTERVAL -1
invalid value "-1" for "WATCH_INTERVAL": must be greater than 0.00
greg=# \set WATCH_INTERVAL 0.00
greg=#

We should disallow 0 as the error message implies

Ah, nice catch, fixed in the attached along with a test for the minimum bound (ie zero).

I've long wanted to be able to set the default interval for \watch in psql since I almost never want a 2 second wait. 

Curious what other's personal defaults are? I usually use 1 second or 0.5 depending on things.

I rarely use anything higher than 0.5.

--
Daniel Gustafsson

Attachment

Re: Allow default \watch interval in psql to be configured

From
Greg Sabino Mullane
Date:
New patch looks good. TIL I learned you can even use things like  

\set WATCH_INTERVAL 0xAp-4

:)


Cheers,
Greg

--
Enterprise Postgres Software Products & Tech Support

Re: Allow default \watch interval in psql to be configured

From
Michael Paquier
Date:
On Fri, Mar 14, 2025 at 09:11:15AM -0400, Greg Sabino Mullane wrote:
> New patch looks good. TIL I learned you can even use things like
>
> \set WATCH_INTERVAL 0xAp-4
>
> :)

You have taste.
--
Michael

Attachment

Re: Allow default \watch interval in psql to be configured

From
Ashutosh Bapat
Date:
Hi Daniel,

On Fri, Mar 14, 2025 at 2:26 PM Daniel Gustafsson <daniel@yesql.se> wrote:
>
> On 13 Mar 2025, at 15:03, Greg Sabino Mullane <htamfids@gmail.com> wrote:
>
> Patch looks good. One minor issue:
>
> greg=# \set WATCH_INTERVAL -1
> invalid value "-1" for "WATCH_INTERVAL": must be greater than 0.00
> greg=# \set WATCH_INTERVAL 0.00
> greg=#
>
> We should disallow 0 as the error message implies
>
>
> Ah, nice catch, fixed in the attached along with a test for the minimum bound (ie zero).

#\watch c=4 i=
Mon 17 Mar 2025 05:52:50 PM IST (every 0s)

 ?column?
----------
        1
(1 row)

Mon 17 Mar 2025 05:52:50 PM IST (every 0s)

 ?column?
----------
        1
(1 row)

Mon 17 Mar 2025 05:52:50 PM IST (every 0s)

 ?column?
----------
        1
(1 row)

Mon 17 Mar 2025 05:52:50 PM IST (every 0s)

 ?column?
----------
        1
(1 row)

0 is an accepted value for interval, even though it might look insensible.

The behaviour should be same in both cases \watch i=<some value> and
\set WATCH_INTERVAL <some value> \watch. In this case it's not. In
fact, we should use the same validation code in both the cases. Why
don't we perform the same  additional validations in
ParseVariableDouble() in exec_watch_command() as well?

The test only validate default variable value. We need a test where we
see variable value being honored lie tests between 369 to 376 in the
same file.

--
Best Wishes,
Ashutosh Bapat



Re: Allow default \watch interval in psql to be configured

From
Daniel Gustafsson
Date:
> On 17 Mar 2025, at 13:37, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:

> 0 is an accepted value for interval, even though it might look insensible.
>
> The behaviour should be same in both cases \watch i=<some value> and
> \set WATCH_INTERVAL <some value> \watch. In this case it's not.

Having a watch interval of zero is IMHO somewhat nonsensical, but since it was
done intentionally in 6f9ee74d45 (which I had missed) I agree that the default
should support it as well. Fixed.

> In fact, we should use the same validation code in both the cases. Why
> don't we perform the same  additional validations in
> ParseVariableDouble() in exec_watch_command() as well?

Well, they don't use the same code since they are two different things
(variables and command input, while using the same syscalls they have different
errorhandling requirements).  If executing \watch would use ParseVariableDouble
it would make errorhandling quite awkward at best.  I added a comment in
exec_command_watch that any changes in internval parsing should take default
intervals into consideration.

> The test only validate default variable value. We need a test where we
> see variable value being honored lie tests between 369 to 376 in the
> same file.

I'm not sure it's worth spending test cycles on as this code doesn't affect the
execution of \watch at all.  That being said, I added a testcase which sets a
default and then executes \watch.  It doesn't test that the interval was
correct (we don't do that for any), but at least it will catch if setting a
default totally breaks \watch.

--
Daniel Gustafsson



Attachment

Re: Allow default \watch interval in psql to be configured

From
Greg Sabino Mullane
Date:
On Thu, Mar 20, 2025 at 4:45 PM Daniel Gustafsson <daniel@yesql.se> wrote:
Having a watch interval of zero is IMHO somewhat nonsensical, but since it was done intentionally in 6f9ee74d45 (which I had missed) I agree that the default should support it as well. Fixed.

Yeah, I forgot about that too.  The new patch looks good except for one tiny little thing: the error should say "must be at least 0.00" or similar, instead of "must be greater than 0.00" now that we allow 0.00

Cheers,
Greg

--
Enterprise Postgres Software Products & Tech Support

Re: Allow default \watch interval in psql to be configured

From
Ashutosh Bapat
Date:
On Fri, Mar 21, 2025 at 2:15 AM Daniel Gustafsson <daniel@yesql.se> wrote:
>
> > On 17 Mar 2025, at 13:37, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:
>
> > 0 is an accepted value for interval, even though it might look insensible.
> >
> > The behaviour should be same in both cases \watch i=<some value> and
> > \set WATCH_INTERVAL <some value> \watch. In this case it's not.
>
> Having a watch interval of zero is IMHO somewhat nonsensical, but since it was
> done intentionally in 6f9ee74d45 (which I had missed) I agree that the default
> should support it as well. Fixed.
>
> > In fact, we should use the same validation code in both the cases. Why
> > don't we perform the same  additional validations in
> > ParseVariableDouble() in exec_watch_command() as well?
>
> Well, they don't use the same code since they are two different things
> (variables and command input, while using the same syscalls they have different
> errorhandling requirements).  If executing \watch would use ParseVariableDouble
> it would make errorhandling quite awkward at best.  I added a comment in
> exec_command_watch that any changes in internval parsing should take default
> intervals into consideration.

There are following differences between command input parsing and
variable value parsing
1. empty string is considered as 0 value while parsing command input
whereas it wiil cause error when setting to a variable.
#\watch c=1 i=
Fri 21 Mar 2025 08:27:25 AM IST (every 0s)

 ?column?
----------
        1
(1 row)
#\set WATCH_INTERVAL
invalid input syntax for "WATCH_INTERVAL"

That can be considered as an existing bug and maybe fixed later.

2. The maximum value that can be specified as command input is limited
by what strtod can handle but for variable it's 1000000 which is 15
days. I can't imagine someone would want to set default value higher
than that but we need to be prepared to change it if a request comes.
Or increase it to a much higher value like seconds worth 100 years.

3. ParseVariableDouble() gives better error message when strtod() can
not handle the input string by looking at the output as well as errno.
But exec_command_watch() lacks that finesse.

ParseVariableDouble is better at parsing the input string than
exec_command_watch(). But that's something that should have been
tackled when exec_command_watch()'s parsing code was written instead
of ParseVariableDouble().

>
> > The test only validate default variable value. We need a test where we
> > see variable value being honored lie tests between 369 to 376 in the
> > same file.
>
> I'm not sure it's worth spending test cycles on as this code doesn't affect the
> execution of \watch at all.  That being said, I added a testcase which sets a
> default and then executes \watch.  It doesn't test that the interval was
> correct (we don't do that for any), but at least it will catch if setting a
> default totally breaks \watch.
>

This looks ok. We do both test 0 as well as the variable.

With this patch, we are doing something unprecedented (at least
AFAIK); allowing command arguments defaults to be configurable through
a psql variable (as against an environment variable). I admit that
configurable through a psql variable is better since it doesn't meddle
with environment. Glancing through psql documentation, I didn't find a
lot of command which may need default argument to be configurable.
Nonetheless we should mention why this is special and set some
guidance for such future additions - preferrably in code or at least
in the commit message.

- of rows. Wait the specified number of seconds (default 2) between executions.
- For backwards compatibility,
+ of rows. Wait the specified number of seconds (defaults to 2 if
omitted, which can be
+ changed with the variable <xref linkend="app-psql-variables-watch-interval"/>)
+ between executions. For backwards compatibility,

The text in parenthesis is quite long and it's hard to read ...
seconds between execution. I suggest
"Wait the specified number of seconds (default 2) between executions.
The default wait can be changed by setting the variable <xref
linkend="app-psql-variables-watch-interval"/>."

+ " number of seconds \\watch waits between executing the query buffer\n");

I am feeling that this should mention "default" somewhere - maybe just
add it before "number of ".

--
Best Wishes,
Ashutosh Bapat



Re: Allow default \watch interval in psql to be configured

From
Daniel Gustafsson
Date:
> On 21 Mar 2025, at 11:34, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:

> There are following differences between command input parsing and
> variable value parsing
> 1. empty string is considered as 0 value while parsing command input
> whereas it wiil cause error when setting to a variable.
> #\watch c=1 i=
> Fri 21 Mar 2025 08:27:25 AM IST (every 0s)
>
> ?column?
> ----------
>        1
> (1 row)
> #\set WATCH_INTERVAL
> invalid input syntax for "WATCH_INTERVAL"
>
> That can be considered as an existing bug and maybe fixed later.

An empty interval in command parsing implies "use the default", I don't think
there is a sensical counterpart in parsing actually setting the default value.
I think trying to define the default value without providing a value is an
error condition.

> With this patch, we are doing something unprecedented (at least
> AFAIK); allowing command arguments defaults to be configurable through
> a psql variable (as against an environment variable). I admit that
> configurable through a psql variable is better since it doesn't meddle
> with environment. Glancing through psql documentation, I didn't find a
> lot of command which may need default argument to be configurable.
> Nonetheless we should mention why this is special and set some
> guidance for such future additions - preferrably in code or at least
> in the commit message.

Sure, I'll mention it in the commit message.

> - of rows. Wait the specified number of seconds (default 2) between executions.
> - For backwards compatibility,
> + of rows. Wait the specified number of seconds (defaults to 2 if
> omitted, which can be
> + changed with the variable <xref linkend="app-psql-variables-watch-interval"/>)
> + between executions. For backwards compatibility,
>
> The text in parenthesis is quite long and it's hard to read ...
> seconds between execution. I suggest
> "Wait the specified number of seconds (default 2) between executions.
> The default wait can be changed by setting the variable <xref
> linkend="app-psql-variables-watch-interval"/>."

Fixed.

> + " number of seconds \\watch waits between executing the query buffer\n");
>
> I am feeling that this should mention "default" somewhere - maybe just
> add it before "number of ".

Fixed.

--
Daniel Gustafsson



Attachment

Re: Allow default \watch interval in psql to be configured

From
Ashutosh Bapat
Date:
On Mon, Mar 24, 2025 at 5:40 PM Daniel Gustafsson <daniel@yesql.se> wrote:

>
> > With this patch, we are doing something unprecedented (at least
> > AFAIK); allowing command arguments defaults to be configurable through
> > a psql variable (as against an environment variable). I admit that
> > configurable through a psql variable is better since it doesn't meddle
> > with environment. Glancing through psql documentation, I didn't find a
> > lot of command which may need default argument to be configurable.
> > Nonetheless we should mention why this is special and set some
> > guidance for such future additions - preferrably in code or at least
> > in the commit message.
>
> Sure, I'll mention it in the commit message.
>
> > - of rows. Wait the specified number of seconds (default 2) between executions.
> > - For backwards compatibility,
> > + of rows. Wait the specified number of seconds (defaults to 2 if
> > omitted, which can be
> > + changed with the variable <xref linkend="app-psql-variables-watch-interval"/>)
> > + between executions. For backwards compatibility,
> >
> > The text in parenthesis is quite long and it's hard to read ...
> > seconds between execution. I suggest
> > "Wait the specified number of seconds (default 2) between executions.
> > The default wait can be changed by setting the variable <xref
> > linkend="app-psql-variables-watch-interval"/>."
>
> Fixed.
>
> > + " number of seconds \\watch waits between executing the query buffer\n");
> >
> > I am feeling that this should mention "default" somewhere - maybe just
> > add it before "number of ".
>
> Fixed.

LGTM. I think this is RFC. Updated CF entry.

--
Best Wishes,
Ashutosh Bapat



Re: Allow default \watch interval in psql to be configured

From
Daniel Gustafsson
Date:
> On 24 Mar 2025, at 13:42, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:

> LGTM. I think this is RFC. Updated CF entry.

Thanks all for review, committed.

--
Daniel Gustafsson




Re: Allow default \watch interval in psql to be configured

From
Pavel Stehule
Date:
Hi

út 25. 3. 2025 v 20:09 odesílatel Daniel Gustafsson <daniel@yesql.se> napsal:
> On 24 Mar 2025, at 13:42, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:

> LGTM. I think this is RFC. Updated CF entry.

Thanks all for review, committed.


regress tests fails now in my

make[2]: Vstupuje se do adresáře „/home/pavel/src/postgresql/src/bin/psql“
echo "# +++ tap check in src/bin/psql +++" && rm -rf '/home/pavel/src/postgresql/src/bin/psql'/tmp_check && /usr/bin/mkdir -p '/home/pavel/src/postgresql/src/bin/psql'/tmp_check && cd . && TESTLOGDIR='/home/pavel/src/postgresql/src/bin/psql/tmp_check/log' TESTDATADIR='/home/pavel/src/postgresql/src/bin/psql/tmp_check' PATH="/home/pavel/src/postgresql/tmp_install/usr/local/pgsql/master/bin:/home/pavel/src/postgresql/src/bin/psql:$PATH" LD_LIBRARY_PATH="/home/pavel/src/postgresql/tmp_install/usr/local/pgsql/master/lib" INITDB_TEMPLATE='/home/pavel/src/postgresql'/tmp_install/initdb-template  PGPORT='65432' top_builddir='/home/pavel/src/postgresql/src/bin/psql/../../..' PG_REGRESS='/home/pavel/src/postgresql/src/bin/psql/../../../src/test/regress/pg_regress' share_contrib_dir='/home/pavel/src/postgresql/tmp_install/usr/local/pgsql/master/share/' /usr/bin/prove -I ../../../src/test/perl/ -I .  t/*.pl
# +++ tap check in src/bin/psql +++
t/001_basic.pl ........... 46/?
#   Failed test 'WATCH_INTERVAL variable is set and updated: exit code 0'
#   at t/001_basic.pl line 436.
#          got: '3'
#     expected: '0'

#   Failed test 'WATCH_INTERVAL variable is set and updated: no stderr'
#   at t/001_basic.pl line 436.
#          got: 'psql:<stdin>:2: error: "0.001" is out of range for "WATCH_INTERVAL"'
#     expected: ''

#   Failed test 'WATCH_INTERVAL variable is set and updated: matches'
#   at t/001_basic.pl line 436.
#                   '2'
#     doesn't match '(?^lm:^2$
# ^0.001$
# ^2$)'
# Looks like you failed 3 tests of 116.
t/001_basic.pl ........... Dubious, test returned 3 (wstat 768, 0x300)
Failed 3/116 subtests
t/010_tab_completion.pl .. ok    
t/020_cancel.pl .......... ok  

Test Summary Report
-------------------
t/001_basic.pl         (Wstat: 768 (exited 3) Tests: 116 Failed: 3)
  Failed tests:  95-97
  Non-zero exit status: 3
Files=3, Tests=207,  7 wallclock secs ( 0.12 usr  0.02 sys +  3.38 cusr  1.72 csys =  5.24 CPU)
Result: FAIL

The reason is probably my LANG=cs_CZ.UTF8. When I switched to LANG=C, then tests passed

Regards

Pavel
 
--
Daniel Gustafsson



Re: Allow default \watch interval in psql to be configured

From
Pavel Stehule
Date:


st 26. 3. 2025 v 7:59 odesílatel Pavel Stehule <pavel.stehule@gmail.com> napsal:
Hi

út 25. 3. 2025 v 20:09 odesílatel Daniel Gustafsson <daniel@yesql.se> napsal:
> On 24 Mar 2025, at 13:42, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:

> LGTM. I think this is RFC. Updated CF entry.

Thanks all for review, committed.


regress tests fails now in my

make[2]: Vstupuje se do adresáře „/home/pavel/src/postgresql/src/bin/psql“
echo "# +++ tap check in src/bin/psql +++" && rm -rf '/home/pavel/src/postgresql/src/bin/psql'/tmp_check && /usr/bin/mkdir -p '/home/pavel/src/postgresql/src/bin/psql'/tmp_check && cd . && TESTLOGDIR='/home/pavel/src/postgresql/src/bin/psql/tmp_check/log' TESTDATADIR='/home/pavel/src/postgresql/src/bin/psql/tmp_check' PATH="/home/pavel/src/postgresql/tmp_install/usr/local/pgsql/master/bin:/home/pavel/src/postgresql/src/bin/psql:$PATH" LD_LIBRARY_PATH="/home/pavel/src/postgresql/tmp_install/usr/local/pgsql/master/lib" INITDB_TEMPLATE='/home/pavel/src/postgresql'/tmp_install/initdb-template  PGPORT='65432' top_builddir='/home/pavel/src/postgresql/src/bin/psql/../../..' PG_REGRESS='/home/pavel/src/postgresql/src/bin/psql/../../../src/test/regress/pg_regress' share_contrib_dir='/home/pavel/src/postgresql/tmp_install/usr/local/pgsql/master/share/' /usr/bin/prove -I ../../../src/test/perl/ -I .  t/*.pl
# +++ tap check in src/bin/psql +++
t/001_basic.pl ........... 46/?
#   Failed test 'WATCH_INTERVAL variable is set and updated: exit code 0'
#   at t/001_basic.pl line 436.
#          got: '3'
#     expected: '0'

#   Failed test 'WATCH_INTERVAL variable is set and updated: no stderr'
#   at t/001_basic.pl line 436.
#          got: 'psql:<stdin>:2: error: "0.001" is out of range for "WATCH_INTERVAL"'
#     expected: ''

#   Failed test 'WATCH_INTERVAL variable is set and updated: matches'
#   at t/001_basic.pl line 436.
#                   '2'
#     doesn't match '(?^lm:^2$
# ^0.001$
# ^2$)'
# Looks like you failed 3 tests of 116.
t/001_basic.pl ........... Dubious, test returned 3 (wstat 768, 0x300)
Failed 3/116 subtests
t/010_tab_completion.pl .. ok    
t/020_cancel.pl .......... ok  

Test Summary Report
-------------------
t/001_basic.pl         (Wstat: 768 (exited 3) Tests: 116 Failed: 3)
  Failed tests:  95-97
  Non-zero exit status: 3
Files=3, Tests=207,  7 wallclock secs ( 0.12 usr  0.02 sys +  3.38 cusr  1.72 csys =  5.24 CPU)
Result: FAIL

The reason is probably my LANG=cs_CZ.UTF8. When I switched to LANG=C, then tests passed.

The main problem is in numeric format. Czech uses the comma instead of the dot.

pavel@nemesis:~/src/postgresql/src/bin/psql$ LANG=C psql -c "\set WATCH_INTERVAL 0.001"
pavel@nemesis:~/src/postgresql/src/bin/psql$ LANG=cs_CZ.UTF8 psql -c "\set WATCH_INTERVAL 0.001"
"0.001" is out of range for "WATCH_INTERVAL"
pavel@nemesis:~/src/postgresql/src/bin/psql$ LANG=cs_CZ.UTF8 psql -c "\set WATCH_INTERVAL 0,001"

Regards

Pavel



Regards

Pavel
 
--
Daniel Gustafsson



Re: Allow default \watch interval in psql to be configured

From
Daniel Gustafsson
Date:
> On 26 Mar 2025, at 08:42, Pavel Stehule <pavel.stehule@gmail.com> wrote:

> The reason is probably my LANG=cs_CZ.UTF8. When I switched to LANG=C, then tests passed.
>
> The main problem is in numeric format. Czech uses the comma instead of the dot.

Thanks for investigating!  The main value of the test is to test setting value
and unsetting it again, so we could just as well use an integer value like the
diff below.  Does it pass for you with that instead?

diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl
index 7192d96049d..739cb439708 100644
--- a/src/bin/psql/t/001_basic.pl
+++ b/src/bin/psql/t/001_basic.pl
@@ -436,12 +436,12 @@ psql_fails_like(
 psql_like(
        $node,
        '\echo :WATCH_INTERVAL
-\set WATCH_INTERVAL 0.001
+\set WATCH_INTERVAL 10
 \echo :WATCH_INTERVAL
 \unset WATCH_INTERVAL
 \echo :WATCH_INTERVAL',
        qr/^2$
-^0.001$
+^10$
 ^2$/m,
        'WATCH_INTERVAL variable is set and updated');
 psql_fails_like(

--
Daniel Gustafsson




Re: Allow default \watch interval in psql to be configured

From
Pavel Stehule
Date:


st 26. 3. 2025 v 9:05 odesílatel Daniel Gustafsson <daniel@yesql.se> napsal:
> On 26 Mar 2025, at 08:42, Pavel Stehule <pavel.stehule@gmail.com> wrote:

> The reason is probably my LANG=cs_CZ.UTF8. When I switched to LANG=C, then tests passed.
>
> The main problem is in numeric format. Czech uses the comma instead of the dot.

Thanks for investigating!  The main value of the test is to test setting value
and unsetting it again, so we could just as well use an integer value like the
diff below.  Does it pass for you with that instead?

diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl
index 7192d96049d..739cb439708 100644
--- a/src/bin/psql/t/001_basic.pl
+++ b/src/bin/psql/t/001_basic.pl
@@ -436,12 +436,12 @@ psql_fails_like(
 psql_like(
        $node,
        '\echo :WATCH_INTERVAL
-\set WATCH_INTERVAL 0.001
+\set WATCH_INTERVAL 10
 \echo :WATCH_INTERVAL
 \unset WATCH_INTERVAL
 \echo :WATCH_INTERVAL',
        qr/^2$
-^0.001$
+^10$
 ^2$/m,
        'WATCH_INTERVAL variable is set and updated');
 psql_fails_like(

--
Daniel Gustafsson

yes, it is ok after this change

Pavel

Re: Allow default \watch interval in psql to be configured

From
Daniel Gustafsson
Date:
> On 26 Mar 2025, at 10:12, Pavel Stehule <pavel.stehule@gmail.com> wrote:

> yes, it is ok after this change

Thanks for confirming, committed.

--
Daniel Gustafsson




Re: Allow default \watch interval in psql to be configured

From
Pavel Stehule
Date:


st 26. 3. 2025 v 13:24 odesílatel Daniel Gustafsson <daniel@yesql.se> napsal:
> On 26 Mar 2025, at 10:12, Pavel Stehule <pavel.stehule@gmail.com> wrote:

> yes, it is ok after this change

Thanks for confirming, committed.

Thank you

Pavel

--
Daniel Gustafsson

Re: Allow default \watch interval in psql to be configured

From
Ranier Vilela
Date:
Hi.

Em qua., 26 de mar. de 2025 às 09:24, Daniel Gustafsson <daniel@yesql.se> escreveu:
> On 26 Mar 2025, at 10:12, Pavel Stehule <pavel.stehule@gmail.com> wrote:

> yes, it is ok after this change

Thanks for confirming, committed.
Per Coverity.

I think that commit 1a759c8
left a minor oversight.

Typo comparison?

a trivial fix attached.

best regards,
Ranier Vilela
Attachment

Re: Allow default \watch interval in psql to be configured

From
Daniel Gustafsson
Date:
> On 27 Mar 2025, at 14:04, Ranier Vilela <ranier.vf@gmail.com> wrote:
>
> Hi.
>
> Em qua., 26 de mar. de 2025 às 09:24, Daniel Gustafsson <daniel@yesql.se> escreveu:
> > On 26 Mar 2025, at 10:12, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
> > yes, it is ok after this change
>
> Thanks for confirming, committed.
> Per Coverity.
>
> I think that commit 1a759c8
> left a minor oversight.

Thanks for reporting, fixing.

--
Daniel Gustafsson