Hi
út 16. 2. 2021 v 2:49 odesílatel Thomas Munro <thomas.mu...@gmail.com>
napsal:
> On Fri, Jan 8, 2021 at 10:36 PM Pavel Stehule <pavel.steh...@gmail.com>
> wrote:
> > ne 19. 4. 2020 v 19:27 odesílatel Pavel Stehule <pavel.steh...@gmail.com>
> napsal:
> >> last week I finished pspg 3.0 https://github.com/okbob/pspg . pspg now
> supports pipes, named pipes very well. Today the pspg can be used as pager
> for output of \watch command. Sure, psql needs attached patch.
> >>
> >> I propose new psql environment variable PSQL_WATCH_PAGER. When this
> variable is not empty, then \watch command starts specified pager, and
> redirect output to related pipe. When pipe is closed - by pager, then
> \watch cycle is leaved.
> >>
> >> If you want to test proposed feature, you need a pspg with
> cb4114f98318344d162a84b895a3b7f8badec241 commit.
> >>
> >> Then you can set your env
> >>
> >> export PSQL_WATCH_PAGER="pspg --stream"
> >> psql
> >>
> >> SELECT * FROM pg_stat_database;
> >> \watch 1
> >>
> >> Comments, notes?
>
> I tried this out with pspg 4.1 from my package manager. It seems
> really useful, especially for demos. I like it!
>
> * Set up rendering options, in particular, disable the pager,
> because
> * nobody wants to be prompted while watching the output of
> 'watch'.
> */
> - myopt.topt.pager = 0;
> + if (!pagerpipe)
> + myopt.topt.pager = 0;
>
> Obsolete comment.
>
fixed
> +static bool sigpipe_received = false;
>
> This should be "static volatile sig_atomic_t", and I suppose our
> convention name for that variable would be got_SIGPIPE. Would it be
> possible to ignore SIGPIPE instead, and then rely on another way of
> knowing that the pager has quit? But... hmm:
>
> - long s = Min(i, 1000L);
> + long s = Min(i, pagerpipe ? 100L :
> 1000L);
>
> I haven't studied this (preexisting) polling loop, but I don't like
> it. I understand that it's there because on some systems, pg_usleep()
> won't wake up for SIGINT (^C), but now it's being used for a secondary
> purpose, that I haven't fully understood. After I quit pspg (by
> pressing q) while running \watch 10, I have to wait until the end of a
> 10 second cycle before it tries to write to the pipe again, unless I
> also press ^C. I feel like it has to be possible to achieve "precise"
> behaviour somehow when you quit; maybe something like waiting for
> readiness on the pager's stderr, or something like that -- I haven't
> thought hard about this and I admit that I have no idea how this works
> on Windows.
>
>
I rewrote this mechanism (it was broken, because the timing of SIGPIPE is
different, then I expected). An implementation can be significantly simpler
- just detect with waitpid any closed child and react. You proposed it.
Sometimes I see a message like this after I quit pspg:
>
> postgres=# \watch 10
> input stream was closed
>
I don't see this message. But I use fresh 4.3 pspg
please, see attached patch
Regards
Pavel
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 13c1edfa4d..fb70258b0e 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -4652,6 +4652,21 @@ PSQL_EDITOR_LINENUMBER_ARG='--line '
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><envar>PSQL_WATCH_PAGER</envar></term>
+
+ <listitem>
+ <para>
+ When an statement is evaluated repeatedly because <command>\watch</command>
+ was used, then an pager is not used. This behaviour can be changed by
+ setting <envar>PSQL_WATCH_PAGER</envar> to pager with necessary options.
+ Currently only <literal>pspg</literal> pager (version 3.0+) supports this
+ functionality (with an option <literal>--stream</literal>).
+ </para>
+
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><envar>PSQLRC</envar></term>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index c98e3d31d0..9875a8f87c 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -12,6 +12,7 @@
#include <pwd.h>
#ifndef WIN32
#include <sys/stat.h> /* for stat() */
+#include <sys/wait.h> /* for waitpid() */
#include <fcntl.h> /* open() flags */
#include <unistd.h> /* for geteuid(), getpid(), stat() */
#else
@@ -20,6 +21,7 @@
#include <fcntl.h>
#include <direct.h>
#include <sys/stat.h> /* for stat() */
+#include <sys/wait.h> /* for waitpid() */
#endif
#include "catalog/pg_class_d.h"
@@ -4766,6 +4768,8 @@ do_watch(PQExpBuffer query_buf, double sleep)
const char *strftime_fmt;
const char *user_title;
char *title;
+ const char *pagerprog = NULL;
+ FILE *pagerpipe = NULL;
int title_len;
int res = 0;
@@ -4775,6 +4779,17 @@ do_watch(PQExpBuffer query_buf, double sleep)
return false;
}
+ pagerprog = getenv("PSQL_WATCH_PAGER");
+ if (pagerprog)
+ {
+ disable_sigpipe_trap();
+ pagerpipe = popen(pagerprog, "w");
+
+ if (!pagerpipe)
+ /*silently proceed without pager */
+ restore_sigpipe_trap();
+ }
+
/*
* Choose format for timestamps. We might eventually make this a \pset
* option. In the meantime, using a variable for the format suppresses
@@ -4783,10 +4798,11 @@ do_watch(PQExpBuffer query_buf, double sleep)
strftime_fmt = "%c";
/*
- * Set up rendering options, in particular, disable the pager, because
- * nobody wants to be prompted while watching the output of 'watch'.
+ * Set up rendering options, in particular, disable the pager, when
+ * there an pipe to pager is not available.
*/
- myopt.topt.pager = 0;
+ if (!pagerpipe)
+ myopt.topt.pager = 0;
/*
* If there's a title in the user configuration, make sure we have room
@@ -4820,7 +4836,7 @@ do_watch(PQExpBuffer query_buf, double sleep)
myopt.title = title;
/* Run the query and print out the results */
- res = PSQLexecWatch(query_buf->data, &myopt);
+ res = PSQLexecWatch(query_buf->data, &myopt, pagerpipe);
/*
* PSQLexecWatch handles the case where we can no longer repeat the
@@ -4829,6 +4845,9 @@ do_watch(PQExpBuffer query_buf, double sleep)
if (res <= 0)
break;
+ if (pagerpipe && ferror(pagerpipe))
+ break;
+
/*
* Set up cancellation of 'watch' via SIGINT. We redo this each time
* through the loop since it's conceivable something inside
@@ -4839,23 +4858,49 @@ do_watch(PQExpBuffer query_buf, double sleep)
/*
* Enable 'watch' cancellations and wait a while before running the
- * query again. Break the sleep into short intervals (at most 1s)
- * since pg_usleep isn't interruptible on some platforms.
+ * query again. Break the sleep into short intervals (at most 100ms)
+ * since pg_usleep isn't interruptible on some platforms. The overhead
+ * of 100ms interval is compromise between overhead and ergonomentry
+ * (we don't want to wait long time after pager was ended).
*/
sigint_interrupt_enabled = true;
i = sleep_ms;
while (i > 0)
{
- long s = Min(i, 1000L);
+ long s = Min(i, 100L);
+
+ pg_usleep(1000L * s);
+
+ if (pagerpipe)
+ {
+ int status;
+ pid_t pid;
+
+ /*
+ * in this moment an pager process can be only one child of
+ * psql process. There cannot be other processes. So we can
+ * detect end of any child process for fast detection of
+ * pager process.
+ */
+ pid = waitpid(-1, &status, WNOHANG);
+ if (pid)
+ break;
+ }
- pg_usleep(s * 1000L);
if (cancel_pressed)
break;
+
i -= s;
}
sigint_interrupt_enabled = false;
}
+ if (pagerpipe)
+ {
+ pclose(pagerpipe);
+ restore_sigpipe_trap();
+ }
+
pg_free(title);
return (res >= 0);
}
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 925fe34a3f..29d8fd2aeb 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -592,12 +592,13 @@ PSQLexec(const char *query)
* e.g., because of the interrupt, -1 on error.
*/
int
-PSQLexecWatch(const char *query, const printQueryOpt *opt)
+PSQLexecWatch(const char *query, const printQueryOpt *opt, FILE *printQueryFout)
{
PGresult *res;
double elapsed_msec = 0;
instr_time before;
instr_time after;
+ FILE *fout;
if (!pset.db)
{
@@ -638,14 +639,16 @@ PSQLexecWatch(const char *query, const printQueryOpt *opt)
return 0;
}
+ fout = printQueryFout ? printQueryFout : pset.queryFout;
+
switch (PQresultStatus(res))
{
case PGRES_TUPLES_OK:
- printQuery(res, opt, pset.queryFout, false, pset.logfile);
+ printQuery(res, opt, fout, false, pset.logfile);
break;
case PGRES_COMMAND_OK:
- fprintf(pset.queryFout, "%s\n%s\n\n", opt->title, PQcmdStatus(res));
+ fprintf(fout, "%s\n%s\n\n", opt->title, PQcmdStatus(res));
break;
case PGRES_EMPTY_QUERY:
@@ -668,7 +671,7 @@ PSQLexecWatch(const char *query, const printQueryOpt *opt)
PQclear(res);
- fflush(pset.queryFout);
+ fflush(fout);
/* Possible microtiming output */
if (pset.timing)
diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h
index 041b2ac068..d8538a4e06 100644
--- a/src/bin/psql/common.h
+++ b/src/bin/psql/common.h
@@ -29,7 +29,7 @@ extern sigjmp_buf sigint_interrupt_jmp;
extern void psql_setup_cancel_handler(void);
extern PGresult *PSQLexec(const char *query);
-extern int PSQLexecWatch(const char *query, const printQueryOpt *opt);
+extern int PSQLexecWatch(const char *query, const printQueryOpt *opt, FILE *printQueryFout);
extern bool SendQuery(const char *query);
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index e44120bf76..673aa3304f 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -348,7 +348,7 @@ helpVariables(unsigned short int pager)
* Windows builds currently print one more line than non-Windows builds.
* Using the larger number is fine.
*/
- output = PageOutput(158, pager ? &(pset.popt.topt) : NULL);
+ output = PageOutput(160, pager ? &(pset.popt.topt) : NULL);
fprintf(output, _("List of specially treated variables\n\n"));
@@ -504,6 +504,8 @@ helpVariables(unsigned short int pager)
" alternative location for the command history file\n"));
fprintf(output, _(" PSQL_PAGER, PAGER\n"
" name of external pager program\n"));
+ fprintf(output, _(" PSQL_WATCH_PAGER\n"
+ " name of external pager program used for watch mode\n"));
fprintf(output, _(" PSQLRC\n"
" alternative location for the user's .psqlrc file\n"));
fprintf(output, _(" SHELL\n"
