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-09-20 21:58:54 |
Message-ID: | 1fdbc01e-ac18-40dd-afe4-9767dee2b08e@ewie.name |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2025-08-19 03:52 +0200, Erik Wienhold wrote:
> On 2025-08-17 17:19 +0200, Tom Lane wrote:
> > This appears to fix the problem it sets out to fix, but it looks
> > to me like there are adjacent problems of the same ilk; do you
> > feel like looking at those?
> >
> > Specifically, I wondered whether we had the same bug with table
> > headers or cells that contain newlines. It looks like
> > print_aligned_text() is okay, because it counts up those newlines
> > and includes them in the extra_output_lines value that it passes
> > to IsPagerNeeded(). But print_aligned_vertical() and printTable()
> > have no such logic.
>
> I looked into these cases now and the unaligned and expanded modes
> indeed miss to use the pager.
>
> Fixing the generate_lines cases goes without saying since that one is
> about cells. But I'm not so sure if fixing the header cases (nl_column
> and view u&"1\000a2\000a3...") is worth the effort.
>
> > If not, can we fix that by moving all the "extra_lines" logic into
> > IsPagerNeeded(), and if not what shall we do about it?
>
> Looks doable. I'll prepare patch.
Here's v3 to address all of this. I split it into three separate
patches:
* v3-0001 is just a refactoring of the extra line counting, moving it
into IsPagerNeeded. This fixes the line counting for unaligned and
expanded output which, until now, called IsPagerNeeded with zero
extra_lines.
* v3-0002 fixes the line count in expanded mode when headers
contain more line breaks than the respective cells. This must be
checked for every cell because expanded mode repeats the headers for
every record. The header line counts are calculated once before
looping over all cells.
* v3-0003 is the original fix for the footer lines with two additions:
Here I also fix the counting of header lines in normal output mode
because I noticed that header lines are always counted right now even
when the header is not printed (in tuples_only mode or expanded mode.)
This now also considers the table title, if present.
> Manually testing this with the various printing options won't be fun
> though. Or are there tools to simulate the terminal size? I'm using
> tmux so it's fairly easy to create a horizonal split and resize the
> pane to be N lines high. But a script to run psql and check if the
> pager was invoked for a terminal with N lines would be cool.
I also came up with the attached test-psql-pager.py script to run the
test cases from my previous post against different output modes to count
the maximum number of lines for which psql sill triggers the pager.
This uses termios to control the terminal size. The script compares the
actual output (pager line count, output mode, test case) against a *.out
file with the expected output (similar to pg_regress) which I've also
attached. The *.out files correspond the respective patch numbers to
show how each patch improves the pager setup. The expected output in
0-master.out is the baseline on master.
--
Erik Wienhold
Attachment | Content-Type | Size |
---|---|---|
v3-0001-psql-Fix-pager-setup-for-unaligned-and-expanded-o.patch | text/plain | 6.8 KB |
v3-0002-psql-Fix-counting-of-cell-lines-in-expanded-mode.patch | text/plain | 4.1 KB |
v3-0003-psql-Fix-counting-of-header-and-footer-lines-in-p.patch | text/plain | 2.1 KB |
test-psql-pager.py | text/plain | 4.9 KB |
0-master.out | text/plain | 2.5 KB |
1-refactor.out | text/plain | 2.5 KB |
2-expanded.out | text/plain | 2.5 KB |
3-header-footer.out | text/plain | 2.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Daniel Gustafsson | 2025-09-20 22:03:51 | Re: encode/decode support for base64url |
Previous Message | Tom Lane | 2025-09-20 21:21:19 | Re: allow benign typedef redefinitions (C11) |