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-08-19 01:52:15 |
Message-ID: | f0b1582d-b6e8-4c46-bc35-b87ffa8b863f@ewie.name |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2025-08-17 17:19 +0200, Tom Lane wrote:
> Erik Wienhold <ewie(at)ewie(dot)name> writes:
> > On 2025-08-11 20:28 +0200, Greg Sabino Mullane wrote:
> >> Patch looks good, applies and works. Needs a pgindent run:
>
> > Thanks for the review. Here's v2 with proper formatting.
>
> 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. Tested with the following definitions and
patch v2 applied:
create function generate_lines(n int)
returns table (lines text)
language sql
as $$ select string_agg(s::text, e'\n') from generate_series(1, n) s $$;
-- This creates a view name and column name with 24 line breaks when truncated
-- to the default NAMEDATALEN (63 = 9*2 + 15*3)
select format('create view %I (c) as select null;'
'create view nl_column (%1$I) as select null;',
string_agg(s::text, e'\n'))
from generate_series(1, current_setting('max_identifier_length')::int) s
\gexec
Test results:
\pset format aligned \pset expanded off
1a) select * from generate_lines(25); -- uses pager (<= 27 lines)
1b) select * from nl_column; -- uses pager (<= 27 lines)
1c) \d nl_column -- uses pager (<= 27 lines)
1d) \d+ nl_column -- uses pager (<= 53 lines)
1e) \d "1<TAB> -- no pager
1f) \d+ "1<TAB> -- no pager
\pset format unaligned \pset expanded off
2a) select * from generate_lines(25); -- no pager
2b) select * from nl_column; -- no pager
2c) \d nl_column -- no pager
2d) \d+ nl_column -- uses pager (<= 28 lines)
2e) \d "1<TAB> -- no pager
2f) \d+ "1<TAB> -- no pager
\pset format unaligned \pset expanded on
3a) select * from generate_lines(25); -- no pager
3b) select * from nl_column; -- no pager
-- Expanded mode does not apply to \d -> same output as 2c to 2f
3c) \d nl_column -- no pager
3d) \d+ nl_column -- uses pager (<= 28 lines)
3e) \d "1<TAB> -- no pager
3f) \d+ "1<TAB> -- no pager
\pset format aligned \pset expanded on
4a) select * from generate_lines(25); -- no pager
4b) select * from nl_column -- no pager
-- Here as well: same output as 1c to 1f
4c) \d nl_column -- uses pager (<= 27 lines)
4d) \d+ nl_column -- uses pager (<= 53 lines)
4e) \d "1<TAB> -- no pager
4f) \d+ "1<TAB> -- no 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. I've seen some
annoying uses of quoted identifiers in the past but so far haven't come
across identifiers with line breaks. (That would really take the cake!)
But of course, fixing those as well would tie up loose ends.
> Am I missing something about why those cases need not consider this
> issue?
I don't know either. The commits that added the IsPagerNeeded calls
with extra_lines=0 don't say anything about that.
> 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. 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.
--
Erik Wienhold
From | Date | Subject | |
---|---|---|---|
Next Message | Chao Li | 2025-08-19 02:16:54 | Re: Enhance Makefiles to rebuild objects on map file changes |
Previous Message | Man Zeng | 2025-08-19 01:44:03 | Re: When deleting the plpgsql function, release the CachedPlan of the function |