> 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



Reply via email to