On Sat, Dec 14, 2019 at 12:09:51AM +0100, Fabien COELHO wrote: > >> explain (analyze) select * from pgbench_accounts \watch 1 >> >> It behaves as expected. But once I break out of the loop with ctrl-C, then >> if I execute the same thing again it executes the command once, but shows >> no output and doesn't loop. It seems like some flag is getting set with >> ctrl-C, but then never gets reset. >> >> >> I've not dug into code itself, I just bisected it. > > Thanks for the report. I'll look into it.
Looked at it already. And yes, I can see the difference. This comes from the switch from cancel_pressed to CancelRequested in psql, especially PSQLexecWatch() in this case. And actually, now that I look at it, I think that we should simply get rid of cancel_pressed in psql completely and replace it with CancelRequested. This also removes the need of having cancel_pressed defined in print.c, which was not really wanted originally. Attached is a patch which addresses the issue for me, and cleans up the code while on it. Fabien, Jeff, can you confirm please? -- Michael
diff --git a/src/include/fe_utils/print.h b/src/include/fe_utils/print.h
index f138d963d3..0324c48301 100644
--- a/src/include/fe_utils/print.h
+++ b/src/include/fe_utils/print.h
@@ -175,8 +175,6 @@ typedef struct printQueryOpt
} printQueryOpt;
-extern volatile bool cancel_pressed;
-
extern const printTextFormat pg_asciiformat;
extern const printTextFormat pg_asciiformat_old;
extern printTextFormat pg_utf8format; /* ideally would be const, but... */
diff --git a/src/fe_utils/print.c b/src/fe_utils/print.c
index b9cd6a1752..7729a26ee2 100644
--- a/src/fe_utils/print.c
+++ b/src/fe_utils/print.c
@@ -3,9 +3,10 @@
* Query-result printing support for frontend code
*
* This file used to be part of psql, but now it's separated out to allow
- * other frontend programs to use it. Because the printing code needs
- * access to the cancel_pressed flag as well as SIGPIPE trapping and
- * pager open/close functions, all that stuff came with it.
+ * other frontend programs to use it. The printing code relies on the
+ * cancellation logic in fe_utils with an access to the flag CancelRequested
+ * The SIGPIPE trapping and pager open/close functions came in with the
+ * original refactoring.
*
*
* Portions Copyright (c) 1996-2019, PostgreSQL Global Development Group
@@ -31,17 +32,17 @@
#endif
#include "catalog/pg_type_d.h"
+#include "fe_utils/cancel.h"
#include "fe_utils/mbprint.h"
#include "fe_utils/print.h"
/*
* If the calling program doesn't have any mechanism for setting
- * cancel_pressed, it will have no effect.
+ * CancelRequested, note that query cancellation would have not effect.
*
- * Note: print.c's general strategy for when to check cancel_pressed is to do
+ * Note: print.c's general strategy for when to check CancelRequested is to do
* so at completion of each row of output.
*/
-volatile bool cancel_pressed = false;
static bool always_ignore_sigpipe = false;
@@ -371,7 +372,7 @@ print_unaligned_text(const printTableContent *cont, FILE *fout)
const char *const *ptr;
bool need_recordsep = false;
- if (cancel_pressed)
+ if (CancelRequested)
return;
if (cont->opt->start_table)
@@ -406,7 +407,7 @@ print_unaligned_text(const printTableContent *cont, FILE *fout)
{
print_separator(cont->opt->recordSep, fout);
need_recordsep = false;
- if (cancel_pressed)
+ if (CancelRequested)
break;
}
fputs(*ptr, fout);
@@ -422,7 +423,7 @@ print_unaligned_text(const printTableContent *cont, FILE *fout)
{
printTableFooter *footers = footers_with_default(cont);
- if (!opt_tuples_only && footers != NULL && !cancel_pressed)
+ if (!opt_tuples_only && footers != NULL && !CancelRequested)
{
printTableFooter *f;
@@ -462,7 +463,7 @@ print_unaligned_vertical(const printTableContent *cont, FILE *fout)
const char *const *ptr;
bool need_recordsep = false;
- if (cancel_pressed)
+ if (CancelRequested)
return;
if (cont->opt->start_table)
@@ -487,7 +488,7 @@ print_unaligned_vertical(const printTableContent *cont, FILE *fout)
print_separator(cont->opt->recordSep, fout);
print_separator(cont->opt->recordSep, fout);
need_recordsep = false;
- if (cancel_pressed)
+ if (CancelRequested)
break;
}
@@ -504,7 +505,7 @@ print_unaligned_vertical(const printTableContent *cont, FILE *fout)
if (cont->opt->stop_table)
{
/* print footers */
- if (!opt_tuples_only && cont->footers != NULL && !cancel_pressed)
+ if (!opt_tuples_only && cont->footers != NULL && !CancelRequested)
{
printTableFooter *f;
@@ -614,7 +615,7 @@ print_aligned_text(const printTableContent *cont, FILE *fout, bool is_pager)
int output_columns = 0; /* Width of interactive console */
bool is_local_pager = false;
- if (cancel_pressed)
+ if (CancelRequested)
return;
if (opt_border > 2)
@@ -968,7 +969,7 @@ print_aligned_text(const printTableContent *cont, FILE *fout, bool is_pager)
{
bool more_lines;
- if (cancel_pressed)
+ if (CancelRequested)
break;
/*
@@ -1127,12 +1128,12 @@ print_aligned_text(const printTableContent *cont, FILE *fout, bool is_pager)
{
printTableFooter *footers = footers_with_default(cont);
- if (opt_border == 2 && !cancel_pressed)
+ if (opt_border == 2 && !CancelRequested)
_print_horizontal_line(col_count, width_wrap, opt_border,
PRINT_RULE_BOTTOM, format, fout);
/* print footers */
- if (footers && !opt_tuples_only && !cancel_pressed)
+ if (footers && !opt_tuples_only && !CancelRequested)
{
printTableFooter *f;
@@ -1249,7 +1250,7 @@ print_aligned_vertical(const printTableContent *cont,
dmultiline = false;
int output_columns = 0; /* Width of interactive console */
- if (cancel_pressed)
+ if (CancelRequested)
return;
if (opt_border > 2)
@@ -1260,7 +1261,7 @@ print_aligned_vertical(const printTableContent *cont,
{
printTableFooter *footers = footers_with_default(cont);
- if (!opt_tuples_only && !cancel_pressed && footers)
+ if (!opt_tuples_only && !CancelRequested && footers)
{
printTableFooter *f;
@@ -1498,7 +1499,7 @@ print_aligned_vertical(const printTableContent *cont,
offset,
chars_to_output;
- if (cancel_pressed)
+ if (CancelRequested)
break;
if (i == 0)
@@ -1706,12 +1707,12 @@ print_aligned_vertical(const printTableContent *cont,
if (cont->opt->stop_table)
{
- if (opt_border == 2 && !cancel_pressed)
+ if (opt_border == 2 && !CancelRequested)
print_aligned_vertical_line(format, opt_border, 0, hwidth, dwidth,
PRINT_RULE_BOTTOM, fout);
/* print footers */
- if (!opt_tuples_only && cont->footers != NULL && !cancel_pressed)
+ if (!opt_tuples_only && cont->footers != NULL && !CancelRequested)
{
printTableFooter *f;
@@ -1785,7 +1786,7 @@ print_csv_text(const printTableContent *cont, FILE *fout)
const char *const *ptr;
int i;
- if (cancel_pressed)
+ if (CancelRequested)
return;
/*
@@ -1828,7 +1829,7 @@ print_csv_vertical(const printTableContent *cont, FILE *fout)
/* print records */
for (i = 0, ptr = cont->cells; *ptr; i++, ptr++)
{
- if (cancel_pressed)
+ if (CancelRequested)
return;
/* print name of column */
@@ -1901,7 +1902,7 @@ print_html_text(const printTableContent *cont, FILE *fout)
unsigned int i;
const char *const *ptr;
- if (cancel_pressed)
+ if (CancelRequested)
return;
if (cont->opt->start_table)
@@ -1938,7 +1939,7 @@ print_html_text(const printTableContent *cont, FILE *fout)
{
if (i % cont->ncolumns == 0)
{
- if (cancel_pressed)
+ if (CancelRequested)
break;
fputs(" <tr valign=\"top\">\n", fout);
}
@@ -1963,7 +1964,7 @@ print_html_text(const printTableContent *cont, FILE *fout)
fputs("</table>\n", fout);
/* print footers */
- if (!opt_tuples_only && footers != NULL && !cancel_pressed)
+ if (!opt_tuples_only && footers != NULL && !CancelRequested)
{
printTableFooter *f;
@@ -1991,7 +1992,7 @@ print_html_vertical(const printTableContent *cont, FILE *fout)
unsigned int i;
const char *const *ptr;
- if (cancel_pressed)
+ if (CancelRequested)
return;
if (cont->opt->start_table)
@@ -2015,7 +2016,7 @@ print_html_vertical(const printTableContent *cont, FILE *fout)
{
if (i % cont->ncolumns == 0)
{
- if (cancel_pressed)
+ if (CancelRequested)
break;
if (!opt_tuples_only)
fprintf(fout,
@@ -2044,7 +2045,7 @@ print_html_vertical(const printTableContent *cont, FILE *fout)
fputs("</table>\n", fout);
/* print footers */
- if (!opt_tuples_only && cont->footers != NULL && !cancel_pressed)
+ if (!opt_tuples_only && cont->footers != NULL && !CancelRequested)
{
printTableFooter *f;
@@ -2093,7 +2094,7 @@ print_asciidoc_text(const printTableContent *cont, FILE *fout)
unsigned int i;
const char *const *ptr;
- if (cancel_pressed)
+ if (CancelRequested)
return;
if (cont->opt->start_table)
@@ -2152,7 +2153,7 @@ print_asciidoc_text(const printTableContent *cont, FILE *fout)
{
if (i % cont->ncolumns == 0)
{
- if (cancel_pressed)
+ if (CancelRequested)
break;
}
@@ -2180,7 +2181,7 @@ print_asciidoc_text(const printTableContent *cont, FILE *fout)
printTableFooter *footers = footers_with_default(cont);
/* print footers */
- if (!opt_tuples_only && footers != NULL && !cancel_pressed)
+ if (!opt_tuples_only && footers != NULL && !CancelRequested)
{
printTableFooter *f;
@@ -2204,7 +2205,7 @@ print_asciidoc_vertical(const printTableContent *cont, FILE *fout)
unsigned int i;
const char *const *ptr;
- if (cancel_pressed)
+ if (CancelRequested)
return;
if (cont->opt->start_table)
@@ -2243,7 +2244,7 @@ print_asciidoc_vertical(const printTableContent *cont, FILE *fout)
{
if (i % cont->ncolumns == 0)
{
- if (cancel_pressed)
+ if (CancelRequested)
break;
if (!opt_tuples_only)
fprintf(fout,
@@ -2270,7 +2271,7 @@ print_asciidoc_vertical(const printTableContent *cont, FILE *fout)
if (cont->opt->stop_table)
{
/* print footers */
- if (!opt_tuples_only && cont->footers != NULL && !cancel_pressed)
+ if (!opt_tuples_only && cont->footers != NULL && !CancelRequested)
{
printTableFooter *f;
@@ -2361,7 +2362,7 @@ print_latex_text(const printTableContent *cont, FILE *fout)
unsigned int i;
const char *const *ptr;
- if (cancel_pressed)
+ if (CancelRequested)
return;
if (opt_border > 3)
@@ -2422,7 +2423,7 @@ print_latex_text(const printTableContent *cont, FILE *fout)
fputs(" \\\\\n", fout);
if (opt_border == 3)
fputs("\\hline\n", fout);
- if (cancel_pressed)
+ if (CancelRequested)
break;
}
else
@@ -2439,7 +2440,7 @@ print_latex_text(const printTableContent *cont, FILE *fout)
fputs("\\end{tabular}\n\n\\noindent ", fout);
/* print footers */
- if (footers && !opt_tuples_only && !cancel_pressed)
+ if (footers && !opt_tuples_only && !CancelRequested)
{
printTableFooter *f;
@@ -2471,7 +2472,7 @@ print_latex_longtable_text(const printTableContent *cont, FILE *fout)
const char *last_opt_table_attr_char = NULL;
const char *const *ptr;
- if (cancel_pressed)
+ if (CancelRequested)
return;
if (opt_border > 3)
@@ -2607,7 +2608,7 @@ print_latex_longtable_text(const printTableContent *cont, FILE *fout)
if (opt_border == 3)
fputs(" \\hline\n", fout);
}
- if (cancel_pressed)
+ if (CancelRequested)
break;
}
@@ -2625,7 +2626,7 @@ print_latex_vertical(const printTableContent *cont, FILE *fout)
unsigned int i;
const char *const *ptr;
- if (cancel_pressed)
+ if (CancelRequested)
return;
if (opt_border > 2)
@@ -2658,7 +2659,7 @@ print_latex_vertical(const printTableContent *cont, FILE *fout)
/* new record */
if (i % cont->ncolumns == 0)
{
- if (cancel_pressed)
+ if (CancelRequested)
break;
if (!opt_tuples_only)
{
@@ -2688,7 +2689,7 @@ print_latex_vertical(const printTableContent *cont, FILE *fout)
fputs("\\end{tabular}\n\n\\noindent ", fout);
/* print footers */
- if (cont->footers && !opt_tuples_only && !cancel_pressed)
+ if (cont->footers && !opt_tuples_only && !CancelRequested)
{
printTableFooter *f;
@@ -2734,7 +2735,7 @@ print_troff_ms_text(const printTableContent *cont, FILE *fout)
unsigned int i;
const char *const *ptr;
- if (cancel_pressed)
+ if (CancelRequested)
return;
if (opt_border > 2)
@@ -2788,7 +2789,7 @@ print_troff_ms_text(const printTableContent *cont, FILE *fout)
if ((i + 1) % cont->ncolumns == 0)
{
fputc('\n', fout);
- if (cancel_pressed)
+ if (CancelRequested)
break;
}
else
@@ -2802,7 +2803,7 @@ print_troff_ms_text(const printTableContent *cont, FILE *fout)
fputs(".TE\n.DS L\n", fout);
/* print footers */
- if (footers && !opt_tuples_only && !cancel_pressed)
+ if (footers && !opt_tuples_only && !CancelRequested)
{
printTableFooter *f;
@@ -2828,7 +2829,7 @@ print_troff_ms_vertical(const printTableContent *cont, FILE *fout)
const char *const *ptr;
unsigned short current_format = 0; /* 0=none, 1=header, 2=body */
- if (cancel_pressed)
+ if (CancelRequested)
return;
if (opt_border > 2)
@@ -2864,7 +2865,7 @@ print_troff_ms_vertical(const printTableContent *cont, FILE *fout)
/* new record */
if (i % cont->ncolumns == 0)
{
- if (cancel_pressed)
+ if (CancelRequested)
break;
if (!opt_tuples_only)
{
@@ -2909,7 +2910,7 @@ print_troff_ms_vertical(const printTableContent *cont, FILE *fout)
fputs(".TE\n.DS L\n", fout);
/* print footers */
- if (cont->footers && !opt_tuples_only && !cancel_pressed)
+ if (cont->footers && !opt_tuples_only && !CancelRequested)
{
printTableFooter *f;
@@ -3052,7 +3053,7 @@ ClosePager(FILE *pagerpipe)
* pager quit as a result of the SIGINT, this message won't go
* anywhere ...
*/
- if (cancel_pressed)
+ if (CancelRequested)
fprintf(pagerpipe, _("Interrupted\n"));
pclose(pagerpipe);
@@ -3333,7 +3334,7 @@ printTable(const printTableContent *cont,
{
bool is_local_pager = false;
- if (cancel_pressed)
+ if (CancelRequested)
return;
if (cont->opt->format == PRINT_NOTHING)
@@ -3439,7 +3440,7 @@ printQuery(const PGresult *result, const printQueryOpt *opt,
r,
c;
- if (cancel_pressed)
+ if (CancelRequested)
return;
printTableInit(&cont, &opt->topt, opt->title,
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 48b6279403..f86d21850e 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -4508,7 +4508,7 @@ do_watch(PQExpBuffer query_buf, double sleep)
long s = Min(i, 1000L);
pg_usleep(s * 1000L);
- if (cancel_pressed)
+ if (CancelRequested)
break;
i -= s;
}
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 9c0d53689e..5f5908a5c2 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -257,9 +257,6 @@ psql_cancel_callback(void)
sigint_interrupt_enabled = false;
siglongjmp(sigint_interrupt_jmp, 1);
}
-
- /* else, set cancel flag to stop any long-running loops */
- CancelRequested = true;
}
#else
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index b3b9313b36..5ccd3a66a6 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -21,6 +21,7 @@
#include "common.h"
#include "common/logging.h"
#include "describe.h"
+#include "fe_utils/cancel.h"
#include "fe_utils/mbprint.h"
#include "fe_utils/print.h"
#include "fe_utils/string_utils.h"
@@ -1422,7 +1423,7 @@ describeTableDetails(const char *pattern, bool verbose, bool showSystem)
PQclear(res);
return false;
}
- if (cancel_pressed)
+ if (CancelRequested)
{
PQclear(res);
return false;
@@ -4748,7 +4749,7 @@ listTSParsersVerbose(const char *pattern)
return false;
}
- if (cancel_pressed)
+ if (CancelRequested)
{
PQclear(res);
return false;
@@ -5143,7 +5144,7 @@ listTSConfigsVerbose(const char *pattern)
return false;
}
- if (cancel_pressed)
+ if (CancelRequested)
{
PQclear(res);
return false;
@@ -5648,7 +5649,7 @@ listExtensionContents(const char *pattern)
PQclear(res);
return false;
}
- if (cancel_pressed)
+ if (CancelRequested)
{
PQclear(res);
return false;
diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c
index f7b1b94599..83871de2d0 100644
--- a/src/bin/psql/mainloop.c
+++ b/src/bin/psql/mainloop.c
@@ -10,6 +10,7 @@
#include "command.h"
#include "common.h"
#include "common/logging.h"
+#include "fe_utils/cancel.h"
#include "input.h"
#include "mainloop.h"
#include "mb/pg_wchar.h"
@@ -88,7 +89,7 @@ MainLoop(FILE *source)
/*
* Clean up after a previous Control-C
*/
- if (cancel_pressed)
+ if (CancelRequested)
{
if (!pset.cur_cmd_interactive)
{
@@ -99,7 +100,7 @@ MainLoop(FILE *source)
break;
}
- cancel_pressed = false;
+ CancelRequested = false;
}
/*
@@ -121,7 +122,7 @@ MainLoop(FILE *source)
prompt_status = PROMPT_READY;
need_redisplay = false;
pset.stmt_lineno = 1;
- cancel_pressed = false;
+ CancelRequested = false;
if (pset.cur_cmd_interactive)
{
diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c
index 3f7b64143f..875b06fe15 100644
--- a/src/bin/psql/variables.c
+++ b/src/bin/psql/variables.c
@@ -9,6 +9,7 @@
#include "common.h"
#include "common/logging.h"
+#include "fe_utils/cancel.h"
#include "variables.h"
/*
@@ -194,7 +195,7 @@ PrintVariables(VariableSpace space)
{
if (ptr->value)
printf("%s = '%s'\n", ptr->name, ptr->value);
- if (cancel_pressed)
+ if (CancelRequested)
break;
}
}
signature.asc
Description: PGP signature
