On Sun, Mar 12, 2023 at 08:59:44PM -0700, Andrey Borodin wrote: > I've tried this approach, but could not come up with sufficiently > different error messages... > >> Wouldn't it be better to have a couple of regression >> tests, as well? > Added two tests.
It should have three tests with one for ERANGE on top of the other two. Passing down a value like "10e400" should be enough to cause strtod() to fail, as far as I know. + if (sleep == 0) + continue; While on it, forgot to comment on this one.. Indeed, this choice to authorize 0 and not wait between two commands is more natural. I have tweaked things as bit as of the attached, and ran pgindent. What do you think? -- Michael
From f6dad7551c04c3e7b280c310a744f8d4dfc10d19 Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@paquier.xyz> Date: Tue, 14 Mar 2023 09:21:32 +0900 Subject: [PATCH v7] Fix incorrect argument handling in psql \watch Incorrectly parsed argument was silently substituted with 1 second. This is changed to proper error message. Authour: Andrey Borodin <amboro...@acm.org> Reviewed-by: Kyotaro Horiguchi <horikyota....@gmail.com> Reviewed-by: Nathan Bossart <nathandboss...@gmail.com> Reviewed-by: Michael Paquier <mich...@paquier.xyz> Thread: https://postgr.es/m/CAAhFRxiZ2-n_L1ErMm9AZjgmUK%3DqS6VHb%2B0SaMn8sqqbhF7How%40mail.gmail.com --- src/bin/psql/command.c | 21 ++++++++++++++++++--- src/bin/psql/t/001_basic.pl | 17 +++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 955397ee9d..60cabfd2c8 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -2776,9 +2776,21 @@ exec_command_watch(PsqlScanState scan_state, bool active_branch, /* Convert optional sleep-length argument */ if (opt) { - sleep = strtod(opt, NULL); - if (sleep <= 0) - sleep = 1; + char *opt_end; + sleep = strtod(opt, &opt_end); + if (sleep < 0 || *opt_end || errno == ERANGE) + { + if (*opt_end) + pg_log_error("\\watch: incorrect interval value '%s'", opt); + else if (errno == ERANGE) + pg_log_error("\\watch: out-of-range interval value '%s'", opt); + else + pg_log_error("\\watch: interval value '%s' less than zero", opt); + free(opt); + resetPQExpBuffer(query_buf); + psql_scan_reset(scan_state); + return PSQL_CMD_ERROR; + } free(opt); } @@ -5183,6 +5195,9 @@ do_watch(PQExpBuffer query_buf, double sleep) if (pagerpipe && ferror(pagerpipe)) break; + if (sleep == 0) + continue; + #ifdef WIN32 /* diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl index 0f394420b2..b86ff99a63 100644 --- a/src/bin/psql/t/001_basic.pl +++ b/src/bin/psql/t/001_basic.pl @@ -350,4 +350,21 @@ psql_like( '\copy from with DEFAULT' ); +# Check \watch errors +psql_fails_like( + $node, + 'SELECT 1;\watch -10', + qr/interval value '-10' less than zero/, + '\watch, negative interval'); +psql_fails_like( + $node, + 'SELECT 1;\watch 10ab', + qr/incorrect interval value '10ab'/, + '\watch incorrect interval'); +psql_fails_like( + $node, + 'SELECT 1;\watch 10e400', + qr/out-of-range interval value '10e400'/, + '\watch out-of-range interval'); + done_testing(); -- 2.39.2
signature.asc
Description: PGP signature