Re: [PATCH] pgbench: improve \sleep meta command

On 2021/03/08 14:58, kuroda(dot)hayato(at)fujitsu(dot)com wrote:
> Dear Miyake-san,
>
> Thank you for updating the patch.
>
>> 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).
>
> I see.
>
>> 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.
>
> I confirmed your code and how it works, it's OK.
> Finally the message should be "a positive integer" in order to handle
> the following case:
>
> \set time -1
> \sleep :time
>
> -> pgbench: error: \sleep command argument must be an integer (not "-1")

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.

+ syntax_error(source, lineno, my_command->first_line, my_command->argv[0],
+ "\\sleep command argument must be an integer",

I like the error message like "invalid sleep time, must be an integer".

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION