Thank you for your comments!
I fixed my patch based on your comment, and attached it to this mail.
2021-03-05 12:57, kuroda.hay...@fujitsu.com wrote:
* When the following line is input, the error message is not happy.
I think output should be " \sleep command argument must be an
integer...".
\sleep foo
-> pgbench: fatal: test.sql:5: unrecognized time unit, must be us, ms
or s (foo) in command "sleep"
\sleep foo
^ error found here
When argc == 2, pgbench assumes that (1) argv[1] is just a number (e.g.
\sleep 10) or (2) argv[1] is a pair of a number and a time unit (e.g.
\sleep 10ms).
So I fixed the problem as follows:
1) When argv[1] starts with a number, raises an error depending on
whether the time unit is correct or not.
2) When argv[1] does not starts with a number, raises an error because
it must be an integer.
With this modification, the behavior for input should be as follows.
"\sleep 10" -> pass
"\sleep 10s" -> pass
"\sleep 10foo" -> "unrecognized time unit" error
"\sleep foo10" -> "\sleep command argument must be an integer..." error
Is this OK? Please tell me what you think.
* A blank is missed in some lines, for example:
+ if (my_command->argc ==2)
A blank should be added between a variable and an operator.
OK! I fixed it.
Regards
--
Kota Miyake
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index f1d98be2d2..822461dce6 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -2861,7 +2861,18 @@ evaluateSleep(CState *st, int argc, char **argv, int *usecs)
pg_log_error("%s: undefined variable \"%s\"", argv[0], argv[1] + 1);
return false;
}
+
+ for (int i = 0; i < strlen(var); i++)
+ {
+ if(!isdigit(var[i]))
+ {
+ pg_log_error("\\sleep command argument must be an integer (not \"%s\")", var);
+ return false;
+ }
+ }
+
usec = atoi(var);
+
}
else
usec = atoi(argv[1]);
@@ -4648,7 +4659,7 @@ 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];
@@ -4656,9 +4667,19 @@ process_backslash_command(PsqlScanState sstate, const char *source)
c++;
if (*c)
{
- my_command->argv[2] = c;
- offsets[2] = offsets[1] + (c - my_command->argv[1]);
- my_command->argc = 3;
+ /* Raise an error if argv[1] does not start with a number */
+ if (my_command->argc == 2 && my_command->argv[1] != c)
+ {
+ my_command->argv[2] = c;
+ offsets[2] = offsets[1] + (c - my_command->argv[1]);
+ my_command->argc = 3;
+ }
+ else
+ {
+ syntax_error(source, lineno, my_command->first_line, my_command->argv[0],
+ "\\sleep command argument must be an integer",
+ my_command->argv[1], offsets[1] - start_offset);
+ }
}
}
