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.
Okay, I tested this. It handles bad param names, values correctly. Nice Feature, especially if you leave a 1hr task running and you need to step away... Built/Reviewed the Docs. They are correct. Reviewed \? command. It has the parameters updated, shown as optional
