| From: | R Sri Vishnu <sri207881(at)gmail(dot)com> |
|---|---|
| To: | pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: [PATCH] psql: reject invalid \watch intervals before timer conversion |
| Date: | 2026-11-11 17:06:45 |
| Message-ID: | CANM6wFKnvao2JCA_esOsVb=bHoLsRcEWkSzTmd310f7qUenizA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi, Here is v2 of the psql \watch interval patch, rebased onto current
master (a9cb202e9e09). This version adds tests for the maximum
accepted interval and infinity, alongside the NaN, empty, and
oversized interval cases from v1. The behavior and documented
0–1000000 second range are unchanged. Validation: make -j4 and make -C
src/bin/psql check passed on Ubuntu; 156 TAP tests passed. Tab
completion was skipped because the build used --without-readline. git
diff --check passed. The complete patch is attached, and the
corresponding commit is available at
https://github.com/SriVishnu230707/postgres/commit/b0d89dfcc96. AI
assistance helped prepare this revision; I reviewed the changes and
test results. Regards, R Sri Vishnu
On Mon, Nov 2, 2026 at 8:22 PM R Sri Vishnu <sri207881(at)gmail(dot)com> wrote:
>
> Hi,
>
> psql currently accepts NaN as a \watch interval (and as
> WATCH_INTERVAL), and accepts an empty interval= argument. It also
> accepts finite command intervals above the limit already enforced for
> WATCH_INTERVAL. do_watch() converts the supplied double to long
> milliseconds, so NaN and sufficiently large values reach an
> out-of-range floating-to-integer conversion.
>
> This patch rejects those inputs before the timer conversion, documents
> the interval range, and adds TAP regression cases. It is against
> master (633dee1bd1b4). I built PostgreSQL with make -j4 and ran make
> -C src/bin/psql check: 149 tests passed (tab completion was skipped
> because this build used --without-readline). git diff --check also
> passed. The change is not platform-specific. AI assistance helped
> identify the edge cases; I reviewed the patch and test results.
>
> The patch is inline below so it is available to reviewers without an attachment.
>
> Regards,
> R Sri Vishnu
>
> diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
> index 4058caf..7a99134 100644
> --- a/doc/src/sgml/ref/psql-ref.sgml
> +++ b/doc/src/sgml/ref/psql-ref.sgml
> @@ -3933,6 +3933,7 @@ SELECT 1 \bind \sendpipeline
> until interrupted, or the query fails, or the execution count limit
> (if given) is reached, or the query no longer returns the
> minimum number
> of rows. Wait the specified number of seconds (default 2)
> between executions.
> + The interval must be a finite number between 0 and 1000000 seconds.
> The default wait can be changed with the variable
> <xref linkend="app-psql-variables-watch-interval"/>.
> For backwards compatibility,
> diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
> index e0c4d7a..51d89cb 100644
> --- a/src/bin/psql/command.c
> +++ b/src/bin/psql/command.c
> @@ -3433,7 +3433,8 @@ exec_command_watch(PsqlScanState scan_state,
> bool active_branch,
> have_sleep = true;
> errno = 0;
> sleep = strtod(valptr, &opt_end);
> - if (sleep < 0 || *opt_end || errno == ERANGE)
> + if (!(sleep >= 0 && sleep <= DEFAULT_WATCH_INTERVAL_MAX) ||
> + opt_end == valptr || *opt_end || errno == ERANGE)
> {
> pg_log_error("\\watch: incorrect interval value \"%s\"", valptr);
> success = false;
> @@ -3499,7 +3500,8 @@ exec_command_watch(PsqlScanState scan_state,
> bool active_branch,
> have_sleep = true;
> errno = 0;
> sleep = strtod(opt, &opt_end);
> - if (sleep < 0 || *opt_end || errno == ERANGE)
> + if (!(sleep >= 0 && sleep <= DEFAULT_WATCH_INTERVAL_MAX) ||
> + opt_end == opt || *opt_end || errno == ERANGE)
> {
> pg_log_error("\\watch: incorrect interval value \"%s\"", opt);
> success = false;
> diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl
> index 028df33..ab3ca2a 100644
> --- a/src/bin/psql/t/001_basic.pl
> +++ b/src/bin/psql/t/001_basic.pl
> @@ -424,6 +424,21 @@ psql_fails_like(
> 'SELECT 1 \watch 10e400',
> qr/incorrect interval value "10e400"/,
> '\watch, out-of-range interval');
> +psql_fails_like(
> + $node,
> + 'SELECT 1 \watch i=NaN',
> + qr/incorrect interval value "NaN"/,
> + '\watch, NaN interval');
> +psql_fails_like(
> + $node,
> + 'SELECT 1 \watch i=',
> + qr/incorrect interval value ""/,
> + '\watch, empty interval');
> +psql_fails_like(
> + $node,
> + 'SELECT 1 \watch 1000001',
> + qr/incorrect interval value "1000001"/,
> + '\watch, interval above timer limit');
> psql_fails_like(
> $node,
> 'SELECT 1 \watch 1 1',
> @@ -452,6 +467,11 @@ psql_fails_like(
> '\set WATCH_INTERVAL 1e500',
> qr/is out of range/,
> 'WATCH_INTERVAL variable is out of range');
> +psql_fails_like(
> + $node,
> + '\set WATCH_INTERVAL NaN',
> + qr/invalid value "NaN" for variable "WATCH_INTERVAL"/,
> + 'WATCH_INTERVAL rejects NaN');
> psql_like($node, '\echo :WATCH_INTERVAL',
> qr/^2$/m, 'WATCH_INTERVAL variable was not altered');
> diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c
> index 8060f29..a5f2b2c 100644
> --- a/src/bin/psql/variables.c
> +++ b/src/bin/psql/variables.c
> @@ -212,6 +212,12 @@ ParseVariableDouble(const char *value, const char
> *name, double *result, double
> dblval = strtod(value, &end);
> if (errno == 0 && *end == '\0' && end != value)
> {
> + if (isnan(dblval))
> + {
> + if (name)
> + pg_log_error("invalid value \"%s\" for variable \"%s\"", value, name);
> + return false;
> + }
> if (dblval < min)
> {
> if (name)
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Bharath Rupireddy | 2026-11-11 17:13:35 | Re: Pass heap tuple counts to parallel vacuum as double |
| Previous Message | Bharath Rupireddy | 2026-11-11 16:39:47 | Re: Autovacuum freeze scores wrap around for just-vacuumed tables |