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=#

At 2012-10-19 17:15:27 -0700, will(at)heroku(dot)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(at)2ndquadrant(dot)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(at)heroku(dot)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(at)heroku(dot)com> wrote:
> Thanks for the reviews and comments. Responses inline:
> .
> On Sat, Oct 20, 2012 at 9:19 AM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)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(at)gmx(dot)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

On Thu, Jan 17, 2013 at 5:07 PM, Daniel Farina <daniel(at)heroku(dot)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

2013/1/17 Daniel Farina <daniel(at)heroku(dot)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(at)guedesoft(dot)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(at)heroku(dot)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(at)heroku(dot)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(at)gmx(dot)net> wrote:

> 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.
>
>

2013/4/3 Will Leinweber <will(at)heroku(dot)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(at)guedesoft(dot)net - skype: guediz
http://github.com/guedes - http://guedesoft.net
http://www.postgresql.org.br

Will Leinweber <will(at)heroku(dot)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(at)sss(dot)pgh(dot)pa(dot)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,
> 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".
>

Completely understandable.

I'm very excited to have helped contribute something, however small this
was, to the project.

Thanks again,
Will