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

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