> On 4 Jun 2023, at 20:55, Greg Sabino Mullane <htamf...@gmail.com> wrote: > > On Sat, Jun 3, 2023 at 5:58 PM Michael Paquier <mich...@paquier.xyz> wrote: > > Wouldn't something like a target_rows be more flexible? You could use > this parameter with a target number of rows to expect, zero being one > choice in that. > > Thank you! That does feel better to me. Please see attached a new v2 patch > that uses > a min_rows=X syntax (defaults to 0). Also added some help.c changes.
This is a feature I've wanted on several occasions so definitely a +1 on this suggestion. I've yet to test it out and do a full review, but a few comments from skimming the patch: - bool is_watch, + bool is_watch, int min_rows, The comment on ExecQueryAndProcessResults() needs to be updated with an explanation of what this parameter is. - return cancel_pressed ? 0 : success ? 1 : -1; + return (cancel_pressed || return_early) ? 0 : success ? 1 : -1; I think this is getting tangled up enough that it should be replaced with separate if() statements for the various cases. - HELP0(" \\watch [[i=]SEC] [c=N] execute query every SEC seconds, up to N times\n"); + HELP0(" \\watch [[i=]SEC] [c=N] [m=ROW]\n"); + HELP0(" execute query every SEC seconds, up to N times\n"); + HELP0(" stop if less than ROW minimum rows are rerturned\n"); "less than ROW minimum rows" reads a bit awkward IMO, how about calling it [m=MIN] and describe as "stop if less than MIN rows are returned"? Also, there is a typo: s/rerturned/returned/. -- Daniel Gustafsson