On 2021/03/09 0:54, Fujii Masao wrote:
On 2021/03/08 23:10, Alvaro Herrera wrote:
On 2021-Mar-08, kuroda.hay...@fujitsu.com wrote:
Dear Fujii-san, Miyake-san
Isn't it better to accept even negative sleep time like currently pgbench does?
Otherwise we always need to check the variable is a positive integer
(for example, using \if command) when using it as the sleep time in \sleep
command. That seems inconvenient.
Both of them are acceptable for me.
But we should write down how it works when the negative value is input if we
adopt.
Not sleeping at all seems a good reaction (same as for zero, I guess.)
+1. BTW, IIUC currently \sleep works in that way.
Attached is the updated version of the patch.
Currently a variable whose value is a negative number is allowed to be
specified as a sleep time as follows. In this case \sleep command doesn't
sleep. The patch doesn't change this behavior at all.
\set hoge -1
\sleep :hoge s
Currently a variable whose value is a double is allowed to be specified as
a sleep time as follows. In this case the integer value that atoi() converts
the double number into is used as a sleep time. The patch also doesn't
change this behavior at all because I've not found any strong reason to
ban that usage.
\set hoge 10 / 4.0
\sleep :hoge s
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index e69d43b26b..a6d91d1089 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -2953,7 +2953,24 @@ evaluateSleep(CState *st, int argc, char **argv, int
*usecs)
pg_log_error("%s: undefined variable \"%s\"", argv[0],
argv[1] + 1);
return false;
}
+
usec = atoi(var);
+ if (usec == 0)
+ {
+ char *c = var;
+
+ /* Skip sign */
+ if (*c == '+' || *c == '-')
+ c++;
+
+ /* Raise an error if the value of a variable is not a
number */
+ if (!isdigit((unsigned char) *c))
+ {
+ pg_log_error("%s: invalid sleep time \"%s\" for
variable \"%s\"",
+ argv[0], var, argv[1]
+ 1);
+ return false;
+ }
+ }
}
else
usec = atoi(argv[1]);
@@ -4788,17 +4805,41 @@ process_backslash_command(PsqlScanState sstate, const
char *source)
* will be parsed with atoi, which ignores trailing non-digit
* characters.
*/
- if (my_command->argc == 2 && my_command->argv[1][0] != ':')
+ if (my_command->argv[1][0] != ':')
{
char *c = my_command->argv[1];
+ bool have_digit = false;
- while (isdigit((unsigned char) *c))
+ /* Skip sign */
+ if (*c == '+' || *c == '-')
c++;
+
+ /* Require at least one digit */
+ if (*c && isdigit((unsigned char) *c))
+ have_digit = true;
+
+ /* Eat all digits */
+ while (*c && isdigit((unsigned char) *c))
+ c++;
+
if (*c)
{
- my_command->argv[2] = c;
- offsets[2] = offsets[1] + (c -
my_command->argv[1]);
- my_command->argc = 3;
+ if (my_command->argc == 2 && have_digit)
+ {
+ my_command->argv[2] = c;
+ offsets[2] = offsets[1] + (c -
my_command->argv[1]);
+ my_command->argc = 3;
+ }
+ else
+ {
+ /*
+ * Raise an error if argument starts
with non-digit
+ * character (after sign).
+ */
+ syntax_error(source, lineno,
my_command->first_line, my_command->argv[0],
+ "invalid sleep
time, must be an integer",
+
my_command->argv[1], offsets[1] - start_offset);
+ }
}
}
