Daniel Verite wrote:
> > 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.
Works for me.
> > * 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.
Ok. Seems a bit too short to me, and I don't like the fact that you
can't actually run it because you need to create the my_table
beforehand. I think it'd be better if you used a VALUES clause there,
so that the reader can cut'n paste into psql to start to play with the
feature.
> > * 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.
I don't know what you mean, but here's what I meant.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/bin/psql/crosstabview.c b/src/bin/psql/crosstabview.c
index b3510b9..0216dae 100644
--- a/src/bin/psql/crosstabview.c
+++ b/src/bin/psql/crosstabview.c
@@ -496,12 +496,12 @@ printCrosstab(const PGresult *results, int num_columns,
{
src_col = colsG[i];
- content = (!PQgetisnull(results, rn, src_col)) ?
+ content = !PQgetisnull(results, rn, src_col) ?
PQgetvalue(results, rn, src_col) :
(popt.nullPrint ? popt.nullPrint : "");
}
- if (cont.cells[idx] != NULL && cont.cells[idx][0] != '\0')
+ if (cont.cells[idx] != NULL)
{
/*
* Multiple values for the same (row,col) are projected
@@ -509,12 +509,9 @@ printCrosstab(const PGresult *results, int num_columns,
* previous content of the cell from the new value by a
* newline.
*/
- int content_size;
char *new_content;
int idx2;
- content_size = strlen(cont.cells[idx]) + 2 + strlen(content) + 1;
-
/*
* idx2 is an index into allocated_cells. It differs from
* idx (index into cont.cells), because vertical and
@@ -524,34 +521,17 @@ printCrosstab(const PGresult *results, int num_columns,
idx2 = (row_number * num_columns) + col_number;
if (allocated_cells[idx2] != NULL)
- {
- new_content = pg_realloc(allocated_cells[idx2], content_size);
- }
+ new_content = psprintf("%s%s%s",
+ allocated_cells[idx2],
+ i == 0 ? "\n" : " ",
+ content);
else
- {
- /*
- * At this point, cont.cells[idx] still contains a
- * PQgetvalue() pointer. Just after, it will contain
- * a new pointer maintained in allocated_cells[], and
- * freed at the end of this function.
- */
- new_content = pg_malloc(content_size);
- strcpy(new_content, cont.cells[idx]);
- }
- cont.cells[idx] = new_content;
- allocated_cells[idx2] = new_content;
+ new_content = psprintf("%s", content);
- /*
- * Contents that are on adjacent columns in the source
- * results get separated by one space in the target.
- * Contents that are on different rows in the source get
- * separated by newlines in the target.
- */
- if (i == 0)
- strcat(new_content, "\n");
- else
- strcat(new_content, " ");
- strcat(new_content, content);
+ cont.cells[idx] = new_content;
+ if (allocated_cells[idx2] != NULL)
+ pg_free(allocated_cells[idx2]);
+ allocated_cells[idx2] = new_content;
}
else
{
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
