On Fri, Mar 24, 2023 at 10:32 PM Andrey Borodin <amborodi...@gmail.com> wrote:
> 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. > 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 Marked as Ready for Committer.
