From: | Erik Wienhold <ewie(at)ewie(dot)name> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Greg Sabino Mullane <htamfids(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: psql: Count all table footer lines in pager setup |
Date: | 2025-10-06 00:40:31 |
Message-ID: | bf75a064-7617-446f-8b62-8afc06a1519a@ewie.name |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
v4-0001 LGTM. It still gives me the same pager line count as my v2.
On 2025-10-02 23:00 +0200, Tom Lane wrote:
> Erik Wienhold <ewie(at)ewie(dot)name> writes:
> > On 2025-10-02 00:25 +0200, Tom Lane wrote:
> >> 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.
I see larger gains for queries that produce cells with many newlines,
benchmarked with a variation of my test-psql-pager.py script from
upthread. It measures the mtime of a file created before running psql
and a second file created by the pager using PAGER='touch somefile'
(that should cover the query execution time plus the formatting
overhead). With that I consistently measure these times for psql's
normal output format:
Query: SELECT repeat(e'foo\n', :n_lines) FROM generate_series(1, :n_rows)
n_lines | n_rows | time[ms] (master) | time[ms] (v4) | diff[%]
---------|----------|---------------------|---------------|---------
1 | 100000 | 30.000 | 26.667 | -11.11
1 | 1000000 | 200.000 | 173.333 | -13.33
1 | 10000000 | 1889.998 | 1613.332 | -14.63
10 | 10000 | 16.667 | 13.333 | -20.00
10 | 100000 | 73.333 | 50.000 | -31.82
10 | 1000000 | 583.333 | 390.000 | -33.14
100 | 1000 | 16.667 | 10.000 | -40.00
100 | 10000 | 60.000 | 36.667 | -38.89
100 | 100000 | 490.000 | 280.000 | -42.86
1000 | 100 | 16.667 | 13.333 | -20.00
1000 | 1000 | 56.667 | 33.333 | -41.18
1000 | 10000 | 483.333 | 260.000 | -46.21
Based on that I think you should push v4-0002.
> >> 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.
>
> I realized that we could address that if we really wanted to, using
> the same infrastructure as for tab-completion testing. 0001 and 0002
> attached are the same as before, and then I added 0003 which adds a
> draft-quality TAP test. Code coverage checks show that this adds only
> about 10 more lines of coverage in psql proper, but in print.c most of
> the pager-related logic is now covered.
+1 on the TAP tests in general. But I'm not too familiar with the TAP
infrastructure to provide any meaningful review, given that you consider
it just draft-quality.
> >> 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.
Can we use something along the lines of my test-psql-pager.py to binary
search the line count at which psql uses the pager. Because it would
only apply to systems that provide termios.h, we might be less
constrained to find a portable solution that tests whether the pager
triggers or not (which you noted in v4-0003.)
--
Erik Wienhold
From | Date | Subject | |
---|---|---|---|
Next Message | Richard Guo | 2025-10-06 00:56:25 | Re: Eager aggregation, take 3 |
Previous Message | Tatsuo Ishii | 2025-10-06 00:28:13 | Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options |