From: | Andrey Borodin <amborodin86(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Nathan Bossart <nathandbossart(at)gmail(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, sfrost(at)snowman(dot)net, peter(dot)eisentraut(at)enterprisedb(dot)com, pgsql-hackers(at)postgresql(dot)org, samokhvalov(at)gmail(dot)com |
Subject: | Re: psql \watch 2nd argument: iteration count |
Date: | 2023-03-17 04:15:30 |
Message-ID: | CAAhFRxjzbVTBPgGfKNdh+g_WyOu9SEkZkP1ZvY5CQ5oPZ84SrA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Mar 15, 2023 at 5:54 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Wed, Mar 15, 2023 at 04:58:49PM +0900, Michael Paquier wrote:
> > Yep. You are right.
>
> Fixed that and applied 0001.
Great, thanks!
>
> + valptr++;
> + if (strncmp("i", opt, strlen("i")) == 0 ||
> + strncmp("interval", opt, strlen("interval")) == 0)
> + {
>
> Did you look at process_command_g_options() and if some consolidation
> was possible? It would be nice to have APIs shaped so as more
> sub-commands could rely on the same facility in the future.
I've tried, but they behave so differently. I could reuse only the
"char *valptr = strchr(opt, '=');" thing from there :)
And process_command_g_options() changes data in-place...
Actually, I'm not sure having "i" == "interval" and "c"=="count" is a
good idea here too. I mean I like it, but is it coherent?
Also I do not like repeating 4 times this 5 lines
+ pg_log_error("\\watch: incorrect interval value '%s'", valptr);
+ free(opt);
+ resetPQExpBuffer(query_buf);
+ psql_scan_reset(scan_state);
+ return PSQL_CMD_ERROR;
But I hesitate defining a new function for this...
>
> - <term><literal>\watch [ <replaceable class="parameter">seconds</replaceable> ]</literal></term>
> + <term><literal>\watch [ <replaceable class="parameter">seconds</replaceable> [ <replaceable class="parameter">iterations</replaceable> ] ]</literal></term>
>
> This set of changes is not reflected in the documentation.
Done.
> With an interval in place, we could now automate some tests with
> \watch where it does not fail. What do you think about adding a test
> with a simple query, an interval of 0s and one iteration?
Done. Also found a bug that we actually were doing N+1 iterations.
Thank you for working on this!
Best regards, Andrey Borodin.
Attachment | Content-Type | Size |
---|---|---|
v9-0001-Iteration-count-argument-to-psql-watch-command.patch | application/octet-stream | 8.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | vignesh C | 2023-03-17 05:34:00 | Re: logical decoding and replication of sequences, take 2 |
Previous Message | Tom Lane | 2023-03-17 03:52:04 | Re: slapd logs to syslog during tests |