On Sat, Sep 5, 2015 at 5:55 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Ordinarily I might think that was overkill, but given the number of times > that we've failed to update those counts in the past, I think this is > definitely a worthwhile investment in maintainability.
So to preface this, this was just a silly hack that turned out to be more effective and simpler to code than I expected. But I suspect it's still just a silly idea and easier to do what you suggested. Also, it seems we often get the count wrong on SQL output and elsewhere anyways and I'm not sure we really want to make that a strict rule. Also, as someone who doesn't really like the whole "sometimes you get a pager sometimes you don't" thing and turns it off whenever he sees it I'm not in the best place to judge how much work it's worth to get the line count right. But that said, here's a tricksy patch that triggers an assertion failure if the expected_lines is wrong. I intended it to trigger in the regression tests so it only checks if the pager is actually off. It wouldn't be hard to make it always check though. -- greg
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c index cab9e6e..5e2ff09 100644 --- a/src/bin/psql/print.c +++ b/src/bin/psql/print.c @@ -2725,6 +2725,12 @@ print_troff_ms_vertical(const printTableContent *cont, FILE *fout) /* Public functions */ /********************************/ +#ifdef USE_ASSERT_CHECKING +#define MAX_PAGEOUTPUT_LINES 1000 +#define MAX_PAGEOUTPUT_BUF (1000 * 80 * 2) +static char *PageOutputBuf; +static int expected_lines; +#endif /* * PageOutput @@ -2770,6 +2776,21 @@ PageOutput(int lines, const printTableOpt *topt) #endif } +#ifdef USE_ASSERT_CHECKING + if (lines < MAX_PAGEOUTPUT_LINES) + { + fflush(stdout); + expected_lines = lines; + if (!PageOutputBuf) + { + PageOutputBuf = pg_malloc0(MAX_PAGEOUTPUT_BUF); + setvbuf(stdout, PageOutputBuf, _IOFBF, MAX_PAGEOUTPUT_BUF); + } else { + memset(PageOutputBuf, MAX_PAGEOUTPUT_BUF, 0); + } + } +#endif + return stdout; } @@ -2799,6 +2820,33 @@ ClosePager(FILE *pagerpipe) pqsignal(SIGPIPE, SIG_DFL); #endif } + +#ifdef USE_ASSERT_CHECKING + if (PageOutputBuf && expected_lines) + { + int n_newlines = 0; + int i; + for (i=0;i<MAX_PAGEOUTPUT_BUF;i++) + { + if (PageOutputBuf[i]=='\n') + { + n_newlines++; + } + } + fprintf(stderr, "Expected %d lines, actually printed %d lines\n", + expected_lines, n_newlines); + Assert(n_newlines == expected_lines); + + expected_lines = 0; + fflush(stdout); + + /* XXX leave this set? not sure. */ + setbuffer(stdout, NULL, 0); + pg_free(PageOutputBuf); + PageOutputBuf = 0; + + } +#endif } /*
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers