On Thu, Mar 23, 2023 at 10:15 PM Yugo NAGATA <nag...@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.

Attachment: v10-0001-Iteration-count-argument-to-psql-watch-command.patch
Description: Binary data

Reply via email to