Re: psql: Count all table footer lines in pager setup

Erik Wienhold <ewie(at)ewie(dot)name> writes:
> Here's v3 to address all of this. I split it into three separate
> patches:

Thanks! While reviewing this I decided that splitting it wasn't
such a great idea, because I kept getting distracted by obvious
bugs in the code you were copying around, only to realize that
the next patches fixed those bugs. So in the attached v4 I
merged these into one patch. It's mostly the same as yours
(I did find one outright bug, in a typo'd pg_malloc call),
but I split the line-counting logic into a new subroutine of
its own, which allows IsPagerNeeded to retain pretty much its
previous shape. There's some cosmetic changes too, mostly
expanding comments.

While I was looking at this I got annoyed at how many times we call
pg_wcssize. That's not tremendously cheap, and we're actually doing
it twice for every table cell, which is going to add up to a lot for
large tables. (We've had complaints before about the speed of psql
on big query results...) I'm not sure there is any great way to
avoid that altogether, but I did realize that we could skip the vast
majority of that work in the line-counting path if we recognize that
we can stop as soon as we know the table is longer than screen height.
So 0002 attached is a cut at doing that (which now shows why I wanted
the counting to be in its own subroutine).

I am not entirely sure that we should commit 0002 though; it may be
that the savings is down in the noise anyway once you consider all the
other work that happens while printing a big table. A positive reason
not to take it is something I realized while checking test coverage:
we never execute any of the maybe-use-the-pager branch of PageOutput
in the regression tests, because isatty(stdout) will always fail.
So that lack of coverage would also apply to count_table_lines in this
formulation, which is kind of sad. However, I'm not sure how useful
it really is to have code coverage there, since by this very token
the tests are paying zero attention to the value computed by
count_table_lines. It could be arbitrarily silly and we'd not see
that.

So, while I'm content with v4-0001, I'm not quite convinced about
v4-0002. WDYT?

regards, tom lane