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