From a3aef88e6a9c5822eb4a5ad0744b15dc6e8a5d86 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 25 Dec 2016 16:04:31 -0500 Subject: [PATCH] Fix incorrect error reporting for duplicate data in \crosstabview. \crosstabview's complaint about multiple entries for the same crosstab cell quoted the wrong row and/or column values. It would accidentally appear to work if the data had been in strcmp() order to start with, which probably explains how we missed noticing this during development. This could be fixed in more than one way, but the way I chose was to hang onto both result pointers from bsearch() and use those to get at the value names. In passing, avoid casting away const in the bsearch comparison functions. No bug there, just poor style. Per bug #14476 from Tomonari Katsumata. Back-patch to 9.6 where \crosstabview was introduced. Report: https://postgr.es/m/20161225021519.10139.45460@wrigleys.postgresql.org --- src/bin/psql/crosstabview.c | 48 ++++++++++----------- src/test/regress/expected/psql_crosstab.out | 13 ++++++ src/test/regress/sql/psql_crosstab.sql | 11 +++++ 3 files changed, 48 insertions(+), 24 deletions(-) diff --git a/src/bin/psql/crosstabview.c b/src/bin/psql/crosstabview.c index b283c24e3c..945cb2a3ff 100644 --- a/src/bin/psql/crosstabview.c +++ b/src/bin/psql/crosstabview.c @@ -352,7 +352,8 @@ printCrosstab(const PGresult *results, { int row_number; int col_number; - pivot_field *p; + pivot_field *rp, + *cp; pivot_field elt; /* Find target row */ @@ -360,13 +361,13 @@ printCrosstab(const PGresult *results, elt.name = PQgetvalue(results, rn, field_for_rows); else elt.name = NULL; - p = (pivot_field *) bsearch(&elt, - piv_rows, - num_rows, - sizeof(pivot_field), - pivotFieldCompare); - Assert(p != NULL); - row_number = p->rank; + rp = (pivot_field *) bsearch(&elt, + piv_rows, + num_rows, + sizeof(pivot_field), + pivotFieldCompare); + Assert(rp != NULL); + row_number = rp->rank; /* Find target column */ if (!PQgetisnull(results, rn, field_for_columns)) @@ -374,13 +375,13 @@ printCrosstab(const PGresult *results, else elt.name = NULL; - p = (pivot_field *) bsearch(&elt, - piv_columns, - num_columns, - sizeof(pivot_field), - pivotFieldCompare); - Assert(p != NULL); - col_number = p->rank; + cp = (pivot_field *) bsearch(&elt, + piv_columns, + num_columns, + sizeof(pivot_field), + pivotFieldCompare); + Assert(cp != NULL); + col_number = cp->rank; /* Place value into cell */ if (col_number >= 0 && row_number >= 0) @@ -396,10 +397,10 @@ printCrosstab(const PGresult *results, if (cont.cells[idx] != NULL) { psql_error("\\crosstabview: query result contains multiple data values for row \"%s\", column \"%s\"\n", - piv_rows[row_number].name ? piv_rows[row_number].name : - popt.nullPrint ? popt.nullPrint : "(null)", - piv_columns[col_number].name ? piv_columns[col_number].name : - popt.nullPrint ? popt.nullPrint : "(null)"); + rp->name ? rp->name : + (popt.nullPrint ? popt.nullPrint : "(null)"), + cp->name ? cp->name : + (popt.nullPrint ? popt.nullPrint : "(null)")); goto error; } @@ -694,8 +695,8 @@ indexOfColumn(char *arg, const PGresult *res) static int pivotFieldCompare(const void *a, const void *b) { - pivot_field *pa = (pivot_field *) a; - pivot_field *pb = (pivot_field *) b; + const pivot_field *pa = (const pivot_field *) a; + const pivot_field *pb = (const pivot_field *) b; /* test null values */ if (!pb->name) @@ -704,12 +705,11 @@ pivotFieldCompare(const void *a, const void *b) return 1; /* non-null values */ - return strcmp(((pivot_field *) a)->name, - ((pivot_field *) b)->name); + return strcmp(pa->name, pb->name); } static int rankCompare(const void *a, const void *b) { - return *((int *) a) - *((int *) b); + return *((const int *) a) - *((const int *) b); } diff --git a/src/test/regress/expected/psql_crosstab.out b/src/test/regress/expected/psql_crosstab.out index b583323a9e..eae6fbd051 100644 --- a/src/test/regress/expected/psql_crosstab.out +++ b/src/test/regress/expected/psql_crosstab.out @@ -201,3 +201,16 @@ SELECT a,a,1 FROM generate_series(1,3000) AS a SELECT 1 \crosstabview \crosstabview: query must return at least three columns DROP TABLE ctv_data; +-- check error reporting (bug #14476) +CREATE TABLE ctv_data (x int, y int, v text); +INSERT INTO ctv_data SELECT 1, x, '*' || x FROM generate_series(1,10) x; +SELECT * FROM ctv_data \crosstabview + x | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 +---+----+----+----+----+----+----+----+----+----+----- + 1 | *1 | *2 | *3 | *4 | *5 | *6 | *7 | *8 | *9 | *10 +(1 row) + +INSERT INTO ctv_data VALUES (1, 10, '*'); -- duplicate data to cause error +SELECT * FROM ctv_data \crosstabview +\crosstabview: query result contains multiple data values for row "1", column "10" +DROP TABLE ctv_data; diff --git a/src/test/regress/sql/psql_crosstab.sql b/src/test/regress/sql/psql_crosstab.sql index 1237e82f2d..5a4511389d 100644 --- a/src/test/regress/sql/psql_crosstab.sql +++ b/src/test/regress/sql/psql_crosstab.sql @@ -111,3 +111,14 @@ SELECT a,a,1 FROM generate_series(1,3000) AS a SELECT 1 \crosstabview DROP TABLE ctv_data; + +-- check error reporting (bug #14476) +CREATE TABLE ctv_data (x int, y int, v text); + +INSERT INTO ctv_data SELECT 1, x, '*' || x FROM generate_series(1,10) x; +SELECT * FROM ctv_data \crosstabview + +INSERT INTO ctv_data VALUES (1, 10, '*'); -- duplicate data to cause error +SELECT * FROM ctv_data \crosstabview + +DROP TABLE ctv_data; -- 2.40.0