On Thu, Mar 23, 2023 at 10:15 PM Yugo NAGATA <nagata@sraoss.co.jp> wrote:
>
> Here is my review on the v9 patch.
>
> + /* we do not prevent numerous names iterations like i=1 i=1 i=1 */
> + have_sleep = true;
>
> Why this is allowed here? I am not sure there is any reason to allow to specify
> multiple "interval" options. (I would apologize it if I missed past discussion.)
I do not know, it just seems normal to me. I've fixed this.
> postgres=# select 1 \watch interval=3 4
> \watch: incorrect interval value '4'
>
> I think it is ok, but this error message seems not user-friendly because,
> in the above example, interval values itself is correct, but it seems just
> a syntax error. I wonder it is better to use "watch interval must be specified
> only once" or such here, as the past patch.
Done.
>
> + <para>
> + If number of iterations is specified - query will be executed only
> + given number of times.
> + </para>
>
> Is it common to use "-" here? I think using comma like
> "If number of iterations is specified, "
> is natural.
Done.
Thank for the review!
Best regards, Andrey Borodin.
