Re: [patch] Proposal for \crosstabview in psql

I've been looking at this patch.  First thing was to rebase on top of
recent psql code restructuring; second, pgindent; third, reordered the
code in crosstabview.c more sensibly (had to add prototypes).  New
version attached.

Then I looked at the docs to try to figure out exactly how it works.
I'm surprised that there's not a single example added to the psql
manpage.  Please add one.

I then tested it a bit, "kick the tires" so to speak.  I noticed that
error handling is broken.  For instance, observe the query prompt after
the error:

regression=# select * from pg_class \crosstabview relnatts
\crosstabview: missing second argument
regression-#

At this point the query buffer contains the query (you can see it with
\e), which seems bogus to me.  The query buffer needs to be reset.
Compare \gexec:
alvherre=# select 1 \gexec
ERROR:  error de sintaxis en o cerca de «1»
LÍNEA 1: 1
         ^
alvherre=#


Also, using bogus column names as arguments cause state to get all
bogus:

alvherre=# select * from pg_class \crosstabview relnatts relkinda
Invalid column name: relkinda
alvherre=# select 1;
The query must return at least two columns to be shown in crosstab

Note that the second query is not crosstab at all, yet the error message
is entirely bogus.  This one is probably the same bug:

alvherre=# select 'one', 'two';
Invalid column name: relnatts

Apparently, once in that state, not even a successful query crosstab
display resets the state correctly:

alvherre=# select * from pg_class \crosstabview relnatts relkinda
Invalid column name: relkinda
alvherre=# select 'one' as relnatts, 'two' as relkinda \crosstabview
 relnatts | two
----------+-----
 one      | X
(1 fila)

alvherre=# select 1;
The query must return at least two columns to be shown in crosstab

Please fix this.


Some additional items:

* A few examples in docs.  The psql manpage should have at least two new
examples showing the crosstab features, one with the simplest case you
can think of, and another one showing all the features.

* Add regression test cases somewhere for the regression database.
Probably use "FROM tenk1 WHERE hundred < 5", which provides you with 500
rows, enough for many interesting games.  Make sure to test all the
provided features.  I would use a new psql.sql file for this.

* How did you come up with the 1600 value?  Whatever it is, please use a
#define instead of hardcoding it.

* In the "if (cont.cells[idx] != NULL && cont.cells[idx][0] != '\0')"
block (line 497 in the attached), can't we do the same thing by using
psprintf?

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services