On Wed, Oct 09, 2024 at 11:03:03AM -0400, Tom Lane wrote: > in the loop, which I bet some cowboy added to fix the zero-wait > problem you complained of. But it's doing the wrong thing because > it checks sleep not sleep_ms. > > We should change this to test sleep_ms, and we should probably > fix the code that says what the wait interval is to print > sleep_ms/1000.0 not sleep. And some more comments would be good.
Cowboy of 6f9ee74d45aa reporting for duty. This was not backpatched as of the reason written in the commit log. If folks would rather get the zero behavior backported, I'm OK with a more aggressive backpatch if that's the consensus. A cherry-pick of 6f9ee74d45aa down to v12 is clean enough, with a small conflict related to pagerpipe so it looks pretty much fine. Also, long is 4 bytes on Windows. Should we just use uint64 for sleep_ms? I am not sure that it is worth changing as it still gives plenty of maximum interval time even with sleep_ms in milliseconds. Anyway, I agree that this could be better for values with lower digits, even if \watch would output 0s when using an internal lower than 10^-3. How about something like the simple patch attached? -- Michael
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 2bb8789750..1c9905bad4 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -5359,6 +5359,10 @@ do_shell(const char *command)
*
* We break this out of exec_command to avoid having to plaster "volatile"
* onto a bunch of exec_command's variables to silence stupider compilers.
+ *
+ * "sleep" is the amount of time to sleep during each loop, measured in
+ * seconds. The internals of this function should use "sleep_ms" for
+ * precise sleep time calculations.
*/
static bool
do_watch(PQExpBuffer query_buf, double sleep, int iter, int min_rows)
@@ -5484,10 +5488,10 @@ do_watch(PQExpBuffer query_buf, double sleep, int iter, int min_rows)
if (user_title)
snprintf(title, title_len, _("%s\t%s (every %gs)\n"),
- user_title, timebuf, sleep);
+ user_title, timebuf, sleep_ms / 1000.0);
else
snprintf(title, title_len, _("%s (every %gs)\n"),
- timebuf, sleep);
+ timebuf, sleep_ms / 1000.0);
myopt.title = title;
/* Run the query and print out the result */
@@ -5508,7 +5512,7 @@ do_watch(PQExpBuffer query_buf, double sleep, int iter, int min_rows)
if (pagerpipe && ferror(pagerpipe))
break;
- if (sleep == 0)
+ if (sleep_ms == 0)
continue;
#ifdef WIN32
signature.asc
Description: PGP signature
