Thread: patch to add \watch to psql
This patch adds \watch to psql. It is much like the unix equivalent, defaulting to every 2 seconds, and allowing you optionally specify a number of seconds.
I will add this to the commit fest app.
Thanks,
Will Leinweber
Example:
psql (9.3devel, server 9.1.4)
Type "help" for help.
will=# \watch select now();
Watch every 2s Fri Oct 19 17:09:23 2012
now
-------------------------------
2012-10-19 17:09:23.743176-07
(1 row)
Watch every 2s Fri Oct 19 17:09:25 2012
now
-------------------------------
2012-10-19 17:09:25.745125-07
(1 row)
Watch every 2s Fri Oct 19 17:09:27 2012
now
-------------------------------
2012-10-19 17:09:27.746732-07
(1 row)
^Cwill=# \watch 5 select now();
Watch every 5s Fri Oct 19 17:09:33 2012
now
-------------------------------
2012-10-19 17:09:33.563695-07
(1 row)
Watch every 5s Fri Oct 19 17:09:38 2012
now
-------------------------------
2012-10-19 17:09:38.564802-07
(1 row)
^Cwill=# \watch select pg_sleep(20);
^CCancel request sent
ERROR: canceling statement due to user request
will=#
Attachment
At 2012-10-19 17:15:27 -0700, will@heroku.com wrote: > > will=# \watch select now(); > Watch every 2s Fri Oct 19 17:09:23 2012 > > now > ------------------------------- > 2012-10-19 17:09:23.743176-07 > (1 row) The patch looks OK at first glance, and I can confirm that it works as intended. I can imagine this functionality being useful, e.g. to watch pg_stat_activity or similar tables. But a big part of why watch(1) is nice is that the output is overwritten rather than scrolling. When it's scrolling, it's (a) difficult to notice that something has changed, and (b) harder to compare values, especially when the interval is short. For these reasons, I can imagine using "watch -n2 psql -c …", but not \watch in its present form. (Of course, I doubt anyone would be enthused about a proposal to link ncurses into psql, but that's another matter.) Maybe you should call it \repeat or something. I'm sure people would get around to using \watch that way eventually. :-) -- Abhijit
On Sat, Oct 20, 2012 at 12:19 AM, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote: > For these reasons, I can imagine using "watch -n2 psql -c …", but not > \watch in its present form. (Of course, I doubt anyone would be enthused > about a proposal to link ncurses into psql, but that's another matter.) A good point. Part of the patch is that it disables the pager as so things just keep being appended to the buffer. However, I wonder if clever utilization of a pager would give you the effect you mention with much the same code, and while allowing the old behavior to be retained, as it is also useful. (appending to the screen, and not overwriting) An unrelated defect, although the patch tries to carefully clean up the 'res' result from psqlexec in the error cases, it does forget to do that, seemingly, in the 'positive' case, while it is looping. I think it needs another pqclear in there. -- fdr
On 10/19/12 8:15 PM, Will Leinweber wrote: > This patch adds \watch to psql. It is much like the unix equivalent, > defaulting to every 2 seconds, and allowing you optionally specify a > number of seconds. This doesn't handle multiline queries: => \watch select 1 + ERROR: 42601: syntax error at end of input LINE 1: select + ^ I think to make it cooperate better with psql syntax, put the \watch at the end, as a replacement for \g, like => select something \watch
> On Sat, Oct 20, 2012 at 9:49 PM, Daniel Farina <daniel@heroku.com> wrote: >> >> An unrelated defect, although the patch tries to carefully clean up >> the 'res' result from psqlexec in the error cases, it does forget to >> do that, seemingly, in the 'positive' case, while it is looping. I >> think it needs another pqclear in there. > I have another unrelated defect. I think the lower bound of "zero" time for repeating a query is too low -- I made a bad query and ran it against my local machine and my terminal was flooded with some kind of output (maybe a NULL byte per iteration?). I couldn't look too carefully, because it basically hosed that terminal session almost instantly. Also, because the interrupts are enabled only during the sleep section (this may also be a defect), I couldn't send SIGINT very well. I think. My computer was struggling, in any case. Deceptively, this would not occur on any database that was remote, because you'd get at *least* a millisecond of delay. -- fdr
I have adjusted this patch a little bit to take care of the review issues, along with just doing a bit of review myself. On Thu, Oct 25, 2012 at 2:25 AM, Will Leinweber <will@heroku.com> wrote: > Thanks for the reviews and comments. Responses inline: > . > On Sat, Oct 20, 2012 at 9:19 AM, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote: >> Maybe you should call it \repeat or something. I'm sure people would get >> around to using \watch that way eventually. :-) > > > While I agree that clearing the screen would be nicer, I feel that > watching the bottom of the screen instead of the top gets you 95% of > the value of unix watch(1), and having the same name will greatly > enhance discoverability. Perhaps later on if ncurses is linked for > some other reason, this could take advantage of it then. > > That said, I'm not that strongly attached to the name one way or the other. The name \repeat has grown on me, but I haven't bothered renaming it for the time being. I think sameness with the familiar 'watch' program may not be such a big deal as I thought originally, but 'repeat' sounds a lot more like a kind of flow control for scripts, whereas \watch is more clearly for humans, which is the idea. > On Wed, Oct 24, 2012 at 2:55 PM, Peter Eisentraut <peter_e@gmx.net> wrote: >> This doesn't handle multiline queries: >> >> => \watch select 1 + >> ERROR: 42601: syntax error at end of input >> LINE 1: select + >> ^ >> >> I think to make it cooperate better with psql syntax, put the \watch at >> the end, as a replacement for \g, like I have implemented some kind of multi-line support. The rough part is in this part of the patch: + if (query_buf && query_buf->len > 0) + { + /* + * Check that the query in query_buf has been terminated. This + * is mostly consistent with behavior from commands like \g. + * The reason this is here is to prevent terrible things from + * occuring from submitting incomplete input of statements + * like: + * + * DELETE FROM foo + * \watch + * WHERE.... + * + * Wherein \watch will go ahead and send whatever has been + * submitted so far. So instead, insist that the user + * terminate the query with a semicolon to be safe. + */ + if (query_buf->data[query_buf->len - 1] == ';') What I found myself reaching for when giving up and writing this hack was a way to thread through the last lexer state of query_buf, which seems it could stand to be accrue a bit more information than being just a byte buffer. But this is the simplest possible thing, so I'll let others comment... -- fdr
Attachment
On Thu, Jan 17, 2013 at 5:07 PM, Daniel Farina <daniel@heroku.com> wrote:
> I have adjusted this patch a little bit to take care of the review
> issues, along with just doing a bit of review myself.
I realized while making my adjustments that I pointlessly grew some input checking in the inner loop. I just hoisted it out in this version.
--
fdr
> I have adjusted this patch a little bit to take care of the review
> issues, along with just doing a bit of review myself.
I realized while making my adjustments that I pointlessly grew some input checking in the inner loop. I just hoisted it out in this version.
--
fdr
Attachment
2013/1/17 Daniel Farina <daniel@heroku.com>: > I realized while making my adjustments that I pointlessly grew some input > checking in the inner loop. I just hoisted it out in this version. Since psql uses libreadline, what do you think about to call rl_clear_screen() inside that "while (true)" loop? Obviously we should test #if we have readline enabled to use it, but when we have it a nice output will bring to us. BTW, I don't know how this will behaves on OSX or Windows. []s -- Dickson S. Guedes mail/xmpp: guedes@guedesoft.net - skype: guediz http://github.com/guedes - http://guedesoft.net http://www.postgresql.org.br
On 01/21/2013 02:11 AM, Dickson S. Guedes wrote: > 2013/1/17 Daniel Farina <daniel@heroku.com>: >> I realized while making my adjustments that I pointlessly grew some input >> checking in the inner loop. I just hoisted it out in this version. > Since psql uses libreadline, what do you think about to call > rl_clear_screen() inside that "while (true)" loop? Obviously we should > test #if we have readline enabled to use it, but when we have it a > nice output will bring to us. > > BTW, I don't know how this will behaves on OSX or Windows. OSX uses libedit, which is readline-compatible modulo some bugs. Windows doesn't have line-editing (sadly) so it won't do anything at all. -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Daniel Farina <daniel@heroku.com> writes: > The name \repeat has grown on me, but I haven't bothered renaming it > for the time being. I think sameness with the familiar 'watch' > program may not be such a big deal as I thought originally, but > 'repeat' sounds a lot more like a kind of flow control for scripts, > whereas \watch is more clearly for humans, which is the idea. I think that is a very good argument not to use \repeat --- we might conceivably want that name for something else someday. >>> I think to make it cooperate better with psql syntax, put the \watch at >>> the end, as a replacement for \g, like > I have implemented some kind of multi-line support. The rough part is > in this part of the patch: > + if (query_buf && query_buf->len > 0) > + { > + /* > + * Check that the query in query_buf has been terminated. This > + * is mostly consistent with behavior from commands like \g. > + * The reason this is here is to prevent terrible things from > + * occuring from submitting incomplete input of statements > + * like: > + * > + * DELETE FROM foo > + * \watch > + * WHERE.... > + * > + * Wherein \watch will go ahead and send whatever has been > + * submitted so far. So instead, insist that the user > + * terminate the query with a semicolon to be safe. > + */ > + if (query_buf->data[query_buf->len - 1] == ';') I think this is simply gratuitous inconsistency. We have had \g for many years and the number of complaints about it prematurely sending input is nil. I see no reason to think that \watch is more dangerous than \g. Frankly I'd remove the *entire* chunk testing the state of the query buffer. You don't even need the emptiness test, because the check later for PGRES_EMPTY_QUERY handles that issue more completely. But speaking of PGRES_EMPTY_QUERY, surely the loop also ought to exit on PGRES_FATAL_ERROR, or indeed any result other than PGRES_TUPLES_OK? pg_usleep isn't interruptable on some platforms, so I'm not sure this is a good idea: + pg_usleep(1000000 * sleep); Probably better is something like for (i = 0; i < sleep; i++){ pg_usleep(1000000L); if (cancel_pressed) break;} Also, this avoids pg_usleep's limit of 2000 sec on 32-bit platforms. (Speaking of which, that code to limit the sleep to 1 day seems completely pointless.) I also concur with the complaint here http://www.postgresql.org/message-id/CAAZKuFZxyj-RT1aeC6s0g7zM68tDLfbBM1R6HGrbbxnz80KcoA@mail.gmail.com that allowing a minimum sleep of 0 is rather dangerous (especially so in view of the fact that that's what strtol will probably return for bad input, and there's no code at all here to notice a bogus option value). Perhaps it would be better to tweak the above loop so that the sleep quantum is 100ms or so, and force it to iterate at least once even if "zero seconds" are requested. Another minor question is whether we really need the time-of-day in the banner, and if we do, whether it shouldn't be captured after receiving the result instead of before sending the query. Also, please make the banner translatable (just add _(), I think). One other thought is that it might be best to redo the sigsetjmp call in each loop, immediately before "sigint_interrupt_enabled = true;". This avoids the risky assumption that nothing down inside PSQLExec would change sigint_interrupt_jmp. regards, tom lane
On 3/24/13 3:10 PM, Tom Lane wrote: > I also concur with the complaint here > http://www.postgresql.org/message-id/CAAZKuFZxyj-RT1aeC6s0g7zM68tDLfbBM1R6HGrbbxnz80KcoA@mail.gmail.com > that allowing a minimum sleep of 0 is rather dangerous The original "watch" command apparently silently corrects a delay of 0 to 0.1 seconds. > Another minor question is whether we really need the time-of-day in the > banner, That's also part of the original "watch" and occasionally useful, I think.
Here is an updated patch that addresses several of the points brought up so far, such as the sleep, internationalization banner, and zero wait check, and it removes the premature input check.
Unfortunately rl_clear_screen() is not included at all in libedit, causing compilation to fail, and I was completely unable to find a way to distinguish libedit from readline on OS X. It tries extraordinarily hard to pretend that it's readline. Instead falling back to simple control characters to clear the screen worked very well on both linux and OS X.
On Tue, Mar 26, 2013 at 2:14 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
On 3/24/13 3:10 PM, Tom Lane wrote:The original "watch" command apparently silently corrects a delay of 0
> I also concur with the complaint here
> http://www.postgresql.org/message-id/CAAZKuFZxyj-RT1aeC6s0g7zM68tDLfbBM1R6HGrbbxnz80KcoA@mail.gmail.com
> that allowing a minimum sleep of 0 is rather dangerous
to 0.1 seconds.That's also part of the original "watch" and occasionally useful, I think.
> Another minor question is whether we really need the time-of-day in the
> banner,
Attachment
2013/4/3 Will Leinweber <will@heroku.com>: > Here is an updated patch that addresses several of the points brought up so > far, such as the sleep, internationalization banner, and zero wait check, > and it removes the premature input check. > > Unfortunately rl_clear_screen() is not included at all in libedit, causing > compilation to fail, and I was completely unable to find a way to > distinguish libedit from readline on OS X. It tries extraordinarily hard to > pretend that it's readline. Instead falling back to simple control > characters to clear the screen worked very well on both linux and OS X. I don't have access to an OSX box ATM but term_clear_screen(), in libedit, didn't help? -- Dickson S. Guedes mail/xmpp: guedes@guedesoft.net - skype: guediz http://github.com/guedes - http://guedesoft.net http://www.postgresql.org.br
Will Leinweber <will@heroku.com> writes: > Here is an updated patch that addresses several of the points brought up so > far, such as the sleep, internationalization banner, and zero wait check, > and it removes the premature input check. I whacked this around some more, added basic docs, and committed it. > Unfortunately rl_clear_screen() is not included at all in libedit, causing > compilation to fail, and I was completely unable to find a way to > distinguish libedit from readline on OS X. It tries extraordinarily hard to > pretend that it's readline. Instead falling back to simple control > characters to clear the screen worked very well on both linux and OS X. I took that out; "works on the two cases I tried" does not mean "portable". It's possible we could do something involving having configure check for rl_clear_screen() etc, but that seems like more work than is justified, not to mention that the results would then be platform-dependent *by design*. Frankly I kinda prefer the behavior without a screen clear anyway; though this may just prove that I'm not accustomed to using the original "watch". Anyway, that's open to a followup patch if anybody is sufficiently set on doing it differently. regards, tom lane
On Thu, Apr 4, 2013 at 5:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I whacked this around some more, added basic docs, and committed it.
Thanks!
> Unfortunately rl_clear_screen() is not included at all in libedit, causingI took that out; "works on the two cases I tried" does not mean "portable".
> compilation to fail, and I was completely unable to find a way to
> distinguish libedit from readline on OS X. It tries extraordinarily hard to
> pretend that it's readline. Instead falling back to simple control
> characters to clear the screen worked very well on both linux and OS X.
Completely understandable.
I'm very excited to have helped contribute something, however small this was, to the project.
Thanks again,
Will