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

Attachment: signature.asc
Description: PGP signature

Reply via email to