From 0b6780211acafa3504b47692f87b24f891dbefe0 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 1 Oct 2025 17:25:00 -0400 Subject: [PATCH v4 1/2] Improve psql's ability to select pager mode accurately. The code in print.c that's concerned with counting the number of lines that will be needed missed a lot of edge cases: * While plain aligned mode accounted for embedded newlines in column headers and table cells, unaligned and expanded output modes did not. * In particular, since expanded mode repeats the headers for each record, we need to account for embedded newlines in the headers for each record. * Multi-line table titles were not accounted for. * tuples_only mode (where headers aren't printed) wasn't accounted for. * Footers were accounted for as one line per footer, again missing the possibility of multi-line footers. (In some cases such as "\d+" on a view, there can be many lines in a footer.) To fix, move the entire responsibility for counting lines into IsPagerNeeded (or actually, into a new subroutine count_table_lines), and then expand the logic as appropriate. Also restructure to make it perhaps a bit easier to follow. Author: Erik Wienhold Reviewed-by: Tom Lane Discussion: https://postgr.es/m/2dd2430f-dd20-4c89-97fd-242616a3d768@ewie.name --- src/fe_utils/print.c | 199 ++++++++++++++++++++++++++++++------------- 1 file changed, 138 insertions(+), 61 deletions(-) diff --git a/src/fe_utils/print.c b/src/fe_utils/print.c index 4af0f32f2fc..71bc14d499b 100644 --- a/src/fe_utils/print.c +++ b/src/fe_utils/print.c @@ -266,8 +266,13 @@ static const unicodeStyleFormat unicode_style = { /* Local functions */ static int strlen_max_width(unsigned char *str, int *target_width, int encoding); -static void IsPagerNeeded(const printTableContent *cont, int extra_lines, bool expanded, +static void IsPagerNeeded(const printTableContent *cont, + const unsigned int *width_wrap, + bool expanded, FILE **fout, bool *is_pager); +static int count_table_lines(const printTableContent *cont, + const unsigned int *width_wrap, + bool expanded); static void print_aligned_vertical(const printTableContent *cont, FILE *fout, bool is_pager); @@ -656,8 +661,6 @@ print_aligned_text(const printTableContent *cont, FILE *fout, bool is_pager) unsigned char **format_buf; unsigned int width_total; unsigned int total_header_width; - unsigned int extra_row_output_lines = 0; - unsigned int extra_output_lines = 0; const char *const *ptr; @@ -722,14 +725,9 @@ print_aligned_text(const printTableContent *cont, FILE *fout, bool is_pager) max_nl_lines[i] = nl_lines; if (bytes_required > max_bytes[i]) max_bytes[i] = bytes_required; - if (nl_lines > extra_row_output_lines) - extra_row_output_lines = nl_lines; width_header[i] = width; } - /* Add height of tallest header column */ - extra_output_lines += extra_row_output_lines; - extra_row_output_lines = 0; /* scan all cells, find maximum width, compute cell_count */ for (i = 0, ptr = cont->cells; *ptr; ptr++, i++, cell_count++) @@ -889,43 +887,10 @@ print_aligned_text(const printTableContent *cont, FILE *fout, bool is_pager) is_pager = is_local_pager = true; } - /* Check if newlines or our wrapping now need the pager */ - if (!is_pager && fout == stdout) + /* Check if there are enough lines to require the pager */ + if (!is_pager) { - /* scan all cells, find maximum width, compute cell_count */ - for (i = 0, ptr = cont->cells; *ptr; ptr++, cell_count++) - { - int width, - nl_lines, - bytes_required; - - pg_wcssize((const unsigned char *) *ptr, strlen(*ptr), encoding, - &width, &nl_lines, &bytes_required); - - /* - * A row can have both wrapping and newlines that cause it to - * display across multiple lines. We check for both cases below. - */ - if (width > 0 && width_wrap[i]) - { - unsigned int extra_lines; - - /* don't count the first line of nl_lines - it's not "extra" */ - extra_lines = ((width - 1) / width_wrap[i]) + nl_lines - 1; - if (extra_lines > extra_row_output_lines) - extra_row_output_lines = extra_lines; - } - - /* i is the current column number: increment with wrap */ - if (++i >= col_count) - { - i = 0; - /* At last column of each row, add tallest column height */ - extra_output_lines += extra_row_output_lines; - extra_row_output_lines = 0; - } - } - IsPagerNeeded(cont, extra_output_lines, false, &fout, &is_pager); + IsPagerNeeded(cont, width_wrap, false, &fout, &is_pager); is_local_pager = is_pager; } @@ -1376,7 +1341,7 @@ print_aligned_vertical(const printTableContent *cont, */ if (!is_pager) { - IsPagerNeeded(cont, 0, true, &fout, &is_pager); + IsPagerNeeded(cont, NULL, true, &fout, &is_pager); is_local_pager = is_pager; } @@ -3398,37 +3363,149 @@ printTableCleanup(printTableContent *const content) * IsPagerNeeded * * Setup pager if required + * + * cont: table data to be printed + * width_wrap[]: per-column maximum width, or NULL if caller will not wrap + * expanded: expanded mode? + * fout: where to print to (in/out argument) + * is_pager: output argument + * + * If we decide pager is needed, *fout is modified and *is_pager is set true */ static void -IsPagerNeeded(const printTableContent *cont, int extra_lines, bool expanded, +IsPagerNeeded(const printTableContent *cont, const unsigned int *width_wrap, + bool expanded, FILE **fout, bool *is_pager) { if (*fout == stdout) { int lines; + lines = count_table_lines(cont, width_wrap, expanded); + + *fout = PageOutput(lines, cont->opt); + *is_pager = (*fout != stdout); + } + else + *is_pager = false; +} + +/* + * Count the number of lines needed to print the given table. + * + * cont: table data to be printed + * width_wrap[]: per-column maximum width, or NULL if caller will not wrap + * expanded: expanded mode? + * + * We currently handle only regular and expanded modes here. + */ +static int +count_table_lines(const printTableContent *cont, + const unsigned int *width_wrap, + bool expanded) +{ + int *header_height; + int lines, + max_lines = 0, + nl_lines, + i; + int encoding = cont->opt->encoding; + const char *const *cell; + + /* + * Scan all column headers and determine their heights. Cache the values + * since expanded mode repeats the headers for every record. + */ + header_height = (int *) pg_malloc(cont->ncolumns * sizeof(int)); + for (i = 0; i < cont->ncolumns; i++) + { + pg_wcssize((const unsigned char *) cont->headers[i], + strlen(cont->headers[i]), encoding, + NULL, &header_height[i], NULL); + } + + /* + * Expanded mode writes one separator line per record. Normal mode writes + * a single separator line between header and rows. + */ + lines = expanded ? cont->nrows : 1; + + /* Scan all cells to count their lines */ + for (i = 0, cell = cont->cells; *cell; cell++) + { + int width; + + /* Count the original line breaks */ + pg_wcssize((const unsigned char *) *cell, strlen(*cell), encoding, + &width, &nl_lines, NULL); + + /* Count extra lines due to wrapping */ + if (width > 0 && width_wrap && width_wrap[i]) + nl_lines += (width - 1) / width_wrap[i]; + if (expanded) - lines = (cont->ncolumns + 1) * cont->nrows; + { + /* Pick the height of the header or cell, whichever is taller */ + if (nl_lines > header_height[i]) + lines += nl_lines; + else + lines += header_height[i]; + } else - lines = cont->nrows + 1; + { + /* Remember max height in the current row */ + if (nl_lines > max_lines) + max_lines = nl_lines; + } - if (!cont->opt->tuples_only) + /* i is the current column number: increment with wrap */ + if (++i >= cont->ncolumns) { - printTableFooter *f; + i = 0; + if (!expanded) + { + /* At last column of each row, add tallest column height */ + lines += max_lines; + max_lines = 0; + } + } + } - /* - * FIXME -- this is slightly bogus: it counts the number of - * footers, not the number of lines in them. - */ - for (f = cont->footers; f; f = f->next) - lines++; + /* Account for header and footer decoration */ + if (!cont->opt->tuples_only) + { + if (cont->title) + { + /* Add height of title */ + pg_wcssize((const unsigned char *) cont->title, strlen(cont->title), + encoding, NULL, &nl_lines, NULL); + lines += nl_lines; } - *fout = PageOutput(lines + extra_lines, cont->opt); - *is_pager = (*fout != stdout); + if (!expanded) + { + /* Add height of tallest header column */ + max_lines = 0; + for (i = 0; i < cont->ncolumns; i++) + { + if (header_height[i] > max_lines) + max_lines = header_height[i]; + } + lines += max_lines; + } + + /* Add all footer lines */ + for (printTableFooter *f = cont->footers; f; f = f->next) + { + pg_wcssize((const unsigned char *) f->data, strlen(f->data), + encoding, NULL, &nl_lines, NULL); + lines += nl_lines; + } } - else - *is_pager = false; + + free(header_height); + + return lines; } /* @@ -3456,7 +3533,7 @@ printTable(const printTableContent *cont, cont->opt->format != PRINT_ALIGNED && cont->opt->format != PRINT_WRAPPED) { - IsPagerNeeded(cont, 0, (cont->opt->expanded == 1), &fout, &is_pager); + IsPagerNeeded(cont, NULL, (cont->opt->expanded == 1), &fout, &is_pager); is_local_pager = is_pager; } -- 2.43.7