From 436b9559d58886b55a75624ae7b740d974d6a0d6 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 6 Oct 2025 15:12:53 -0400 Subject: [PATCH v5 1/3] 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 vertical output modes did not. * In particular, since vertical 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.) Also, we failed to count the default 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. In passing, move the "flog" output step to the bottom of printTable(), rather than running it when we've already opened the pager in some modes. In principle it shouldn't interfere with the pager because flog should always point to a non-interactive file; but it seems silly to risk any interference, especially when the existing positioning seems to have been chosen with the aid of a dartboard. Author: Erik Wienhold Reviewed-by: Tom Lane Discussion: https://postgr.es/m/2dd2430f-dd20-4c89-97fd-242616a3d768@ewie.name --- src/fe_utils/print.c | 225 ++++++++++++++++++++++++++++++------------- 1 file changed, 159 insertions(+), 66 deletions(-) diff --git a/src/fe_utils/print.c b/src/fe_utils/print.c index 4af0f32f2fc..709b1254c8b 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 vertical, FILE **fout, bool *is_pager); +static int count_table_lines(const printTableContent *cont, + const unsigned int *width_wrap, + bool vertical); 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; } @@ -1351,6 +1316,11 @@ print_aligned_vertical(const printTableContent *cont, if (opt_border > 2) opt_border = 2; + /* + * Kluge for totally empty table: use the default footer even though + * vertical modes normally don't. Otherwise we'd print nothing at all, + * which isn't terribly friendly. Assume pager will not be needed. + */ if (cont->cells[0] == NULL && cont->opt->start_table && cont->opt->stop_table) { @@ -1376,7 +1346,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 +3368,160 @@ 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 + * vertical: vertical 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 vertical, FILE **fout, bool *is_pager) { if (*fout == stdout) { int lines; - if (expanded) - lines = (cont->ncolumns + 1) * cont->nrows; + lines = count_table_lines(cont, width_wrap, vertical); + + *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 + * vertical: vertical mode? + * + * The result is currently only fully accurate for aligned_text and + * aligned_vertical formats; otherwise it's an approximation. + * + * Note: while cont->opt will tell us most formatting details, we need the + * separate "vertical" flag because of the possibility of a dynamic switch + * from aligned_text to aligned_vertical format. + */ +static int +count_table_lines(const printTableContent *cont, + const unsigned int *width_wrap, + bool vertical) +{ + 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 vertical 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); + } + + /* + * Vertical mode writes one separator line per record. Normal mode writes + * a single separator line between header and rows. + */ + lines = vertical ? 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 (vertical) + { + /* 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 (!vertical) + { + /* 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) + { + printTableFooter *f; + + 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 (!vertical) + { + /* 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. Vertical mode does not use the default + * footer, but we must include that in normal mode. + */ + for (f = vertical ? cont->footers : footers_with_default(cont); + f != NULL; 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 +3549,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; } @@ -3464,10 +3557,6 @@ printTable(const printTableContent *cont, clearerr(fout); /* print the stuff */ - - if (flog) - print_aligned_text(cont, flog, false); - switch (cont->opt->format) { case PRINT_UNALIGNED: @@ -3534,6 +3623,10 @@ printTable(const printTableContent *cont, if (is_local_pager) ClosePager(fout); + + /* also produce log output if wanted */ + if (flog) + print_aligned_text(cont, flog, false); } /* -- 2.43.7