> On Jun 25, 2026, at 21:21, Álvaro Herrera <[email protected]> wrote:
Thank you very much for the comments.
>
> 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.
Agreed.
>
> 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?
>
Yes, I didn’t include \pset null in the new test case because I didn’t intend
this patch to change NULL display behavior, so I assumed other test cases
already covered that. I didn’t check that carefully enough.
I didn’t take this part from your proposal, so an empty cell still shows
nothing:
```
@@ -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, "");
}
```
Your proposed test has NULL in the row and column headers, but not in the value
column. I added a NULL value too, so the test case explicitly demonstrates the
display difference between a NULL value and an empty cell.
PFA v4: addressed Álvaro's comments.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
v4-0001-Make-crosstabview-honor-boolean-display-settings.patch
Description: Binary data
