Re: Fix \crosstabview to honor \pset display_true/display_false

On 2026-Jun-21, Chao Li wrote:

> Thank you both very much for reviewing and for the valuable comments.
>
> I have made the helper private to crosstabview.c and left print.c
> unchanged in v3. I will see if I can improve print.h/c for v20.

I mostly agree with this, but I don't think we need to make that
function name that long. It's a private function after all. But it's
definitely in the wrong spot in the file, so I propose to move it down.

Now, your new test case doesn't exercise "\pset null" under
\crosstabview, and I'm not sure that just leaving the cell empty for a
NULL is really the thing to do, when \pset null has set it to some other
value. Simply printing the \pset null string doesn't seem to cut it
though (here implemented as the change in printCrosstab line 421ff), as
seen in how this patch affects the test case prior to the new one.
Should we have a separate array of bools to distinguish cells that must
remain empty (because the query didn't provide a value for them) from
cells that must have the \pset null value?

Thanks

diff --git a/src/bin/psql/crosstabview.c b/src/bin/psql/crosstabview.c
index aaa9b8a2451..75d5563b0b1 100644
--- a/src/bin/psql/crosstabview.c
+++ b/src/bin/psql/crosstabview.c
@@ -47,28 +47,6 @@ typedef struct _pivot_field
int rank;
} pivot_field;

-/*
- * Return the display representation of a crosstab value, following pset
- * substitutions. The returned pointer should not be freed.
- *
- * TODO: In v20, consider whether this should become part of the public print.h
- * interface, and with what name.
- */
-static char *
-printQueryOptDisplayValue(char *value, Oid ftype, const printQueryOpt *opt,
- const char *default_null)
-{
- if (value == NULL)
- return opt->nullPrint ? opt->nullPrint :
- unconstify(char *, default_null);
-
- if (ftype == BOOLOID)
- return value[0] == 't' ?
- (opt->truePrint ? opt->truePrint : "t") :
- (opt->falsePrint ? opt->falsePrint : "f");
-
- return value;
-}

/* Node in avl_tree */
typedef struct _avl_node
@@ -106,6 +84,8 @@ static bool printCrosstab(const PGresult *result,
int num_columns, pivot_field *piv_columns, int field_for_columns,
int num_rows, pivot_field *piv_rows, int field_for_rows,
int field_for_data);
+static char *printDisplayValue(const printQueryOpt *opt, Oid ftype, char *value,
+ char *default_null);
static void avlInit(avl_tree *tree);
static void avlMergeValue(avl_tree *tree, char *name, char *sort_value);
static int avlCollectFields(avl_tree *tree, avl_node *node,
@@ -349,8 +329,8 @@ printCrosstab(const PGresult *result,
{
char *colname;

- colname = printQueryOptDisplayValue(piv_columns[horiz_map[i]].name,
- col_ftype, &popt, "");
+ colname = printDisplayValue(&popt, col_ftype,
+ piv_columns[horiz_map[i]].name, "");

printTableAddHeader(&cont, colname, false, col_align);
}
@@ -363,8 +343,7 @@ printCrosstab(const PGresult *result,
int idx = k * (num_columns + 1);

cont.cells[idx] =
- printQueryOptDisplayValue(piv_rows[i].name, row_ftype, &popt,
- "");
+ printDisplayValue(&popt, row_ftype, piv_rows[i].name, "");
}
cont.cellsadded = num_rows * (num_columns + 1);

@@ -421,17 +400,17 @@ printCrosstab(const PGresult *result,
if (cont.cells[idx] != NULL)
{
pg_log_error("\\crosstabview: query result contains multiple data values for row \"%s\", column \"%s\"",
- printQueryOptDisplayValue(rp->name, row_ftype,
- &popt, "(null)"),
- printQueryOptDisplayValue(cp->name, col_ftype,
- &popt, "(null)"));
+ printDisplayValue(&popt, row_ftype, rp->name,
+ "(null)"),
+ printDisplayValue(&popt, col_ftype, cp->name,
+ "(null)"));
goto error;
}

if (!PQgetisnull(result, rn, field_for_data))
value = PQgetvalue(result, rn, field_for_data);
cont.cells[idx] =
- printQueryOptDisplayValue(value, data_ftype, &popt, "");
+ printDisplayValue(&popt, data_ftype, value, "ups");
}
}

@@ -442,7 +421,7 @@ printCrosstab(const PGresult *result,
for (i = 0; i < cont.cellsadded; i++)
{
if (cont.cells[i] == NULL)
- cont.cells[i] = "";
+ cont.cells[i] = printDisplayValue(&popt, InvalidOid, NULL, "");
}

printTable(&cont, pset.queryFout, false, pset.logfile);
@@ -454,6 +433,27 @@ error:
return retval;
}

+/*
+ * Return the display representation of one value in the query result,
+ * following pset substitutions.
+ *
+ * The returned pointer is not a copy.
+ */
+static char *
+printDisplayValue(const printQueryOpt *opt, Oid ftype, char *value,
+ char *default_null)
+{
+ if (value == NULL)
+ return opt->nullPrint ? opt->nullPrint : default_null;
+
+ if (ftype == BOOLOID)
+ return value[0] == 't' ?
+ (opt->truePrint ? opt->truePrint : "t") :
+ (opt->falsePrint ? opt->falsePrint : "f");
+
+ return value;
+}
+
/*
* The avl* functions below provide a minimalistic implementation of AVL binary
* trees, to efficiently collect the distinct values that will form the horizontal
diff --git a/src/test/regress/sql/psql_crosstab.sql b/src/test/regress/sql/psql_crosstab.sql
index 00b395d7168..b9f8007b2d6 100644
--- a/src/test/regress/sql/psql_crosstab.sql
+++ b/src/test/regress/sql/psql_crosstab.sql
@@ -70,12 +70,21 @@ GROUP BY v, h ORDER BY h,v
\pset null ''

-- boolean display
-\pset display_true 'true'
-\pset display_false 'false'
-SELECT false as row_key, true as col_key, true as val
+\pset display_true 'Aye'
+\pset display_false 'Nay'
+\pset null 'Wut'
+SELECT false as display_bools, false as col_key, false as val
UNION ALL
-SELECT true, false, false
- \crosstabview row_key col_key val
+SELECT true, true, true
+UNION ALL
+SELECT null, true, false
+UNION ALL
+SELECT null, false, true
+UNION ALL
+SELECT true, null, false
+UNION ALL
+SELECT false, null, true
+ \crosstabview display_bools col_key val
\pset display_true 't'
\pset display_false 'f'

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
Tom: There seems to be something broken here.
Teodor: I'm in sackcloth and ashes... Fixed.
http://postgr.es/m/482D1632.8010507@sigaev.ru