Re: [patch] Proposal for \crosstabview in psql

    Alvaro Herrera wrote:


Thanks for looking into that patch!

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

Fixed. This was modelled after the behavior of:
 select 1 \badcommand
but I've changed to mimic what happens with:
  select 1 \g /some/invalid/path
the query buffer is not discarded by the error but the prompt
is ready for a fresh new command.

> 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

Definitely a bug. Fixed.

Also fixed a one-off bug with quoted columns: in parseColumnRefs(),
first call to PQmblen(), I wrongly assumed that
PQmblen("", ..) returns 0, whereas in fact it returns 1.

> * 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.

Added that in the EXAMPLES section at the very end of the manpage.

> * 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.

Looking into regression tests, not yet done.

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

Done with accompanying comment in crosstabview.h

> * 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?

In that block, we can't pass a cell contents as a valist and be done with
that cell, because duplicates of (col value,row value) may happen
at any iteration of the upper loop over PQntuples(results). Any cell really
may need reallocation unpredictably until that loop is done, whereas
psprintf starts by allocating a new buffer unconditionally, so it doesn't
look
to me like it could help to simplify that block.


Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite