Re: psql: Count all table footer lines in pager setup
От | Erik Wienhold |
---|---|
Тема | Re: psql: Count all table footer lines in pager setup |
Дата | |
Msg-id | d64d4773-fd86-44b4-97f3-48ebf1b58f2d@ewie.name обсуждение исходный текст |
Ответ на | Re: psql: Count all table footer lines in pager setup (Tom Lane <tgl@sss.pgh.pa.us>) |
Ответы |
Re: psql: Count all table footer lines in pager setup
|
Список | pgsql-hackers |
On 2025-10-02 00:25 +0200, Tom Lane wrote: > Erik Wienhold <ewie@ewie.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. +1 to factoring out the line counting. > 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). Yeah, I've noticed those repeated pg_wcssize calls as well but thought that their overhead might me acceptable given how old the code is. Limiting that overhead with the threshold parameter is a nifty idea. > 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? I only glanced over the patches. Will take a closer look over the weekend. -- Erik Wienhold
В списке pgsql-hackers по дате отправления: