Improving contrib/tablefunc's error reporting - Mailing list pgsql-hackers
| From | Tom Lane |
|---|---|
| Subject | Improving contrib/tablefunc's error reporting |
| Date | |
| Msg-id | 18937.1709676295@sss.pgh.pa.us Whole thread Raw |
| Responses |
Re: Improving contrib/tablefunc's error reporting
Re: Improving contrib/tablefunc's error reporting |
| List | pgsql-hackers |
After reading the thread at [1], I could not escape the feeling
that contrib/tablefunc's error reporting is very confusing.
Looking into the source code, I soon found that it is also
very inconsistent, with similar error reports being phrased
quite differently. The terminology for column names doesn't
match the SGML docs either. And there are some places that are
so confused about whether they are complaining about the calling
query or the called query that the output is flat-out backwards.
So at that point my nascent OCD wouldn't let me rest without
fixing it. Here's a quick patch series to do that.
For review purposes, I split this into two patches. 0001 simply
adds some more test cases to reach currently-unexercised error
reports. Then 0002 makes my proposed code changes and shows
how the existing error messages change.
I'm not necessarily wedded to the phrasings I used here,
in case anyone has better ideas.
BTW, while I didn't touch it here, it seems fairly bogus that
connectby() checks both type OID and typmod for its output
columns while crosstab() only checks type OID. I think
crosstab() is in the wrong and needs to be checking typmod.
That might be fit material for a separate patch though.
regards, tom lane
[1]
https://www.postgresql.org/message-id/flat/DM4PR19MB597886696589C5CE33F5D58AD3222%40DM4PR19MB5978.namprd19.prod.outlook.com
From 7fba23c28e9e7558f54c52dd9407ce50f75b0e1d Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Tue, 5 Mar 2024 16:37:35 -0500
Subject: [PATCH v1 1/2] Add more test coverage for contrib/tablefunc.
Add test cases that exercise all the error reports in the module,
excluding those that are for failures elsewhere such as SPI errors.
---
contrib/tablefunc/expected/tablefunc.out | 63 ++++++++++++++++++++++++
contrib/tablefunc/sql/tablefunc.sql | 42 ++++++++++++++++
2 files changed, 105 insertions(+)
diff --git a/contrib/tablefunc/expected/tablefunc.out b/contrib/tablefunc/expected/tablefunc.out
index 464c210f42..0c4e114aba 100644
--- a/contrib/tablefunc/expected/tablefunc.out
+++ b/contrib/tablefunc/expected/tablefunc.out
@@ -145,6 +145,21 @@ SELECT * FROM crosstab_out('SELECT rowid, attribute, val FROM ct where rowclass
| val9 | val10 | val11
(3 rows)
+-- check error reporting
+SELECT * FROM crosstab('SELECT rowid, val FROM ct where rowclass = ''group1'' and (attribute = ''att2'' or attribute =
''att3'')ORDER BY 1,2;')
+ AS ct(row_name text, category_1 text, category_2 text);
+ERROR: invalid source data SQL statement
+DETAIL: The provided SQL must return 3 columns: rowid, category, and values.
+SELECT * FROM crosstab('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' and (attribute = ''att2'' or
attribute= ''att3'') ORDER BY 1,2;')
+ AS ct(row_name text);
+ERROR: return and sql tuple descriptions are incompatible
+SELECT * FROM crosstab('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' and (attribute = ''att2'' or
attribute= ''att3'') ORDER BY 1,2;')
+ AS ct(row_name int, category_1 text, category_2 text);
+ERROR: invalid return type
+DETAIL: SQL rowid datatype does not match return rowid datatype.
+SELECT * FROM crosstab('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' and (attribute = ''att2'' or
attribute= ''att3'') ORDER BY 1,2;')
+ AS ct(row_name text, category_1 text, category_2 int);
+ERROR: return and sql tuple descriptions are incompatible
--
-- hash based crosstab
--
@@ -223,6 +238,12 @@ SELECT * FROM crosstab(
'SELECT DISTINCT rowdt, attribute FROM cth ORDER BY 2')
AS c(rowid text, rowdt timestamp, temperature int4, test_result text, test_startdate timestamp, volts float8);
ERROR: provided "categories" SQL must return 1 column of at least one row
+-- if category query generates a NULL value, get expected error
+SELECT * FROM crosstab(
+ 'SELECT rowid, rowdt, attribute, val FROM cth ORDER BY 1',
+ 'SELECT NULL::text')
+AS c(rowid text, rowdt timestamp, temperature int4, test_result text, test_startdate timestamp, volts float8);
+ERROR: provided "categories" SQL must not return NULL values
-- if source query returns zero rows, get zero rows returned
SELECT * FROM crosstab(
'SELECT rowid, rowdt, attribute, val FROM cth WHERE false ORDER BY 1',
@@ -241,6 +262,25 @@ AS c(rowid text, rowdt timestamp, temperature text, test_result text, test_start
-------+-------+-------------+-------------+----------------+-------
(0 rows)
+-- check errors with inappropriate input rowtype
+SELECT * FROM crosstab(
+ 'SELECT rowid, attribute FROM cth ORDER BY 1',
+ 'SELECT DISTINCT attribute FROM cth ORDER BY 1')
+AS c(rowid text, temperature text, test_result text, test_startdate text, volts text);
+ERROR: invalid source data SQL statement
+DETAIL: The provided SQL must return 3 columns; rowid, category, and values.
+SELECT * FROM crosstab(
+ 'SELECT rowid, rowdt, rowdt, attribute, val FROM cth ORDER BY 1',
+ 'SELECT DISTINCT attribute FROM cth ORDER BY 1')
+AS c(rowid text, rowdt timestamp, temperature int4, test_result text, test_startdate timestamp, volts float8);
+ERROR: invalid return type
+DETAIL: Query-specified return tuple has 6 columns but crosstab returns 7.
+-- check errors with inappropriate result rowtype
+SELECT * FROM crosstab(
+ 'SELECT rowid, attribute, val FROM cth ORDER BY 1',
+ 'SELECT DISTINCT attribute FROM cth ORDER BY 1')
+AS c(rowid text);
+ERROR: query-specified return tuple and crosstab function are not compatible
-- check it works with a named result rowtype
create type my_crosstab_result as (
rowid text, rowdt timestamp,
@@ -387,6 +427,29 @@ DETAIL: First two columns must be the same type.
SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0, '~') AS t(keyid float8, parent_keyid float8,
levelint, branch text);
ERROR: invalid return type
DETAIL: SQL key field type double precision does not match return key field type integer.
+SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0, '~') AS t(keyid int, parent_keyid float8,
levelint, branch text);
+ERROR: invalid return type
+DETAIL: First two columns must be the same type.
+-- check other rowtype mismatch cases
+SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0) AS t(keyid int, parent_keyid int, level int,
branchtext);
+ERROR: invalid return type
+DETAIL: Query-specified return tuple has wrong number of columns.
+SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0, '~') AS t(keyid int, parent_keyid int, level
int);
+ERROR: invalid return type
+DETAIL: Query-specified return tuple has wrong number of columns.
+SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0) AS t(keyid int, parent_keyid text, level
int);
+ERROR: invalid return type
+DETAIL: First two columns must be the same type.
+SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0, '~') AS t(keyid int, parent_keyid int, level
float,branch float);
+ERROR: invalid return type
+DETAIL: Third column must be type integer.
+SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0, '~') AS t(keyid int, parent_keyid int, level
int,branch float);
+ERROR: invalid return type
+DETAIL: Fourth column must be type text.
+SELECT * FROM connectby('connectby_text', 'keyid', 'parent_keyid', 'pos', 'row2', 0, '~') AS t(keyid text,
parent_keyidtext, level int, branch text, pos text);
+ERROR: query-specified return tuple not valid for Connectby: fifth column must be type integer
+SELECT * FROM connectby('connectby_text', 'keyid', 'parent_keyid', 'pos', 'row2', 0) AS t(keyid text, parent_keyid
text,level int, pos text);
+ERROR: query-specified return tuple not valid for Connectby: fourth column must be type integer
-- tests for values using custom queries
-- query with one column - failed
SELECT * FROM connectby('connectby_int', '1; --', 'parent_keyid', '2', 0) AS t(keyid int, parent_keyid int, level
int);
diff --git a/contrib/tablefunc/sql/tablefunc.sql b/contrib/tablefunc/sql/tablefunc.sql
index 02e8a98c73..0fb8e40de2 100644
--- a/contrib/tablefunc/sql/tablefunc.sql
+++ b/contrib/tablefunc/sql/tablefunc.sql
@@ -44,6 +44,16 @@ LANGUAGE C STABLE STRICT;
SELECT * FROM crosstab_out('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' ORDER BY 1,2;');
+-- check error reporting
+SELECT * FROM crosstab('SELECT rowid, val FROM ct where rowclass = ''group1'' and (attribute = ''att2'' or attribute =
''att3'')ORDER BY 1,2;')
+ AS ct(row_name text, category_1 text, category_2 text);
+SELECT * FROM crosstab('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' and (attribute = ''att2'' or
attribute= ''att3'') ORDER BY 1,2;')
+ AS ct(row_name text);
+SELECT * FROM crosstab('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' and (attribute = ''att2'' or
attribute= ''att3'') ORDER BY 1,2;')
+ AS ct(row_name int, category_1 text, category_2 text);
+SELECT * FROM crosstab('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' and (attribute = ''att2'' or
attribute= ''att3'') ORDER BY 1,2;')
+ AS ct(row_name text, category_1 text, category_2 int);
+
--
-- hash based crosstab
--
@@ -99,6 +109,12 @@ SELECT * FROM crosstab(
'SELECT DISTINCT rowdt, attribute FROM cth ORDER BY 2')
AS c(rowid text, rowdt timestamp, temperature int4, test_result text, test_startdate timestamp, volts float8);
+-- if category query generates a NULL value, get expected error
+SELECT * FROM crosstab(
+ 'SELECT rowid, rowdt, attribute, val FROM cth ORDER BY 1',
+ 'SELECT NULL::text')
+AS c(rowid text, rowdt timestamp, temperature int4, test_result text, test_startdate timestamp, volts float8);
+
-- if source query returns zero rows, get zero rows returned
SELECT * FROM crosstab(
'SELECT rowid, rowdt, attribute, val FROM cth WHERE false ORDER BY 1',
@@ -111,6 +127,22 @@ SELECT * FROM crosstab(
'SELECT DISTINCT attribute FROM cth WHERE false ORDER BY 1')
AS c(rowid text, rowdt timestamp, temperature text, test_result text, test_startdate text, volts text);
+-- check errors with inappropriate input rowtype
+SELECT * FROM crosstab(
+ 'SELECT rowid, attribute FROM cth ORDER BY 1',
+ 'SELECT DISTINCT attribute FROM cth ORDER BY 1')
+AS c(rowid text, temperature text, test_result text, test_startdate text, volts text);
+SELECT * FROM crosstab(
+ 'SELECT rowid, rowdt, rowdt, attribute, val FROM cth ORDER BY 1',
+ 'SELECT DISTINCT attribute FROM cth ORDER BY 1')
+AS c(rowid text, rowdt timestamp, temperature int4, test_result text, test_startdate timestamp, volts float8);
+
+-- check errors with inappropriate result rowtype
+SELECT * FROM crosstab(
+ 'SELECT rowid, attribute, val FROM cth ORDER BY 1',
+ 'SELECT DISTINCT attribute FROM cth ORDER BY 1')
+AS c(rowid text);
+
-- check it works with a named result rowtype
create type my_crosstab_result as (
@@ -186,6 +218,16 @@ SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0, '~') A
-- should fail as key field datatype should match return datatype
SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0, '~') AS t(keyid float8, parent_keyid float8,
levelint, branch text);
+SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0, '~') AS t(keyid int, parent_keyid float8,
levelint, branch text);
+
+-- check other rowtype mismatch cases
+SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0) AS t(keyid int, parent_keyid int, level int,
branchtext);
+SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0, '~') AS t(keyid int, parent_keyid int, level
int);
+SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0) AS t(keyid int, parent_keyid text, level
int);
+SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0, '~') AS t(keyid int, parent_keyid int, level
float,branch float);
+SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0, '~') AS t(keyid int, parent_keyid int, level
int,branch float);
+SELECT * FROM connectby('connectby_text', 'keyid', 'parent_keyid', 'pos', 'row2', 0, '~') AS t(keyid text,
parent_keyidtext, level int, branch text, pos text);
+SELECT * FROM connectby('connectby_text', 'keyid', 'parent_keyid', 'pos', 'row2', 0) AS t(keyid text, parent_keyid
text,level int, pos text);
-- tests for values using custom queries
-- query with one column - failed
--
2.39.3
From 93440770c38c14b926cd1e93e192549ad95fb52d Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Tue, 5 Mar 2024 16:49:23 -0500
Subject: [PATCH v1 2/2] Improve contrib/tablefunc's error reporting.
Try to make these messages less confusing, ambiguous, and
inconsistent. Notably, avoid saying "query" where it's not clear
whether that means the calling query's AS clause or the query
that crosstab/connectby is invoking. Also, mention in each
error message that it's from crosstab or connectby, because
otherwise even that much is far from clear.
---
contrib/tablefunc/expected/tablefunc.out | 74 +++++-----
contrib/tablefunc/tablefunc.c | 169 ++++++++++-------------
2 files changed, 116 insertions(+), 127 deletions(-)
diff --git a/contrib/tablefunc/expected/tablefunc.out b/contrib/tablefunc/expected/tablefunc.out
index 0c4e114aba..ddece79029 100644
--- a/contrib/tablefunc/expected/tablefunc.out
+++ b/contrib/tablefunc/expected/tablefunc.out
@@ -148,18 +148,20 @@ SELECT * FROM crosstab_out('SELECT rowid, attribute, val FROM ct where rowclass
-- check error reporting
SELECT * FROM crosstab('SELECT rowid, val FROM ct where rowclass = ''group1'' and (attribute = ''att2'' or attribute =
''att3'')ORDER BY 1,2;')
AS ct(row_name text, category_1 text, category_2 text);
-ERROR: invalid source data SQL statement
-DETAIL: The provided SQL must return 3 columns: rowid, category, and values.
+ERROR: invalid crosstab source data query
+DETAIL: The query must return 3 columns: row_name, category, and value.
SELECT * FROM crosstab('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' and (attribute = ''att2'' or
attribute= ''att3'') ORDER BY 1,2;')
AS ct(row_name text);
-ERROR: return and sql tuple descriptions are incompatible
+ERROR: invalid crosstab return type
+DETAIL: Return row must have at least two columns.
SELECT * FROM crosstab('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' and (attribute = ''att2'' or
attribute= ''att3'') ORDER BY 1,2;')
AS ct(row_name int, category_1 text, category_2 text);
-ERROR: invalid return type
-DETAIL: SQL rowid datatype does not match return rowid datatype.
+ERROR: invalid crosstab return type
+DETAIL: Source row_name datatype text does not match return row_name datatype integer.
SELECT * FROM crosstab('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' and (attribute = ''att2'' or
attribute= ''att3'') ORDER BY 1,2;')
AS ct(row_name text, category_1 text, category_2 int);
-ERROR: return and sql tuple descriptions are incompatible
+ERROR: invalid crosstab return type
+DETAIL: Source value datatype text does not match return value datatype integer in column 3.
--
-- hash based crosstab
--
@@ -231,19 +233,20 @@ SELECT * FROM crosstab(
'SELECT rowid, rowdt, attribute, val FROM cth ORDER BY 1',
'SELECT DISTINCT attribute FROM cth WHERE attribute = ''a'' ORDER BY 1')
AS c(rowid text, rowdt timestamp, temperature int4, test_result text, test_startdate timestamp, volts float8);
-ERROR: provided "categories" SQL must return 1 column of at least one row
+ERROR: crosstab categories query must return at least one row
-- if category query generates more than one column, get expected error
SELECT * FROM crosstab(
'SELECT rowid, rowdt, attribute, val FROM cth ORDER BY 1',
'SELECT DISTINCT rowdt, attribute FROM cth ORDER BY 2')
AS c(rowid text, rowdt timestamp, temperature int4, test_result text, test_startdate timestamp, volts float8);
-ERROR: provided "categories" SQL must return 1 column of at least one row
+ERROR: invalid crosstab categories query
+DETAIL: The query must return one column.
-- if category query generates a NULL value, get expected error
SELECT * FROM crosstab(
'SELECT rowid, rowdt, attribute, val FROM cth ORDER BY 1',
'SELECT NULL::text')
AS c(rowid text, rowdt timestamp, temperature int4, test_result text, test_startdate timestamp, volts float8);
-ERROR: provided "categories" SQL must not return NULL values
+ERROR: crosstab category value must not be null
-- if source query returns zero rows, get zero rows returned
SELECT * FROM crosstab(
'SELECT rowid, rowdt, attribute, val FROM cth WHERE false ORDER BY 1',
@@ -267,20 +270,21 @@ SELECT * FROM crosstab(
'SELECT rowid, attribute FROM cth ORDER BY 1',
'SELECT DISTINCT attribute FROM cth ORDER BY 1')
AS c(rowid text, temperature text, test_result text, test_startdate text, volts text);
-ERROR: invalid source data SQL statement
-DETAIL: The provided SQL must return 3 columns; rowid, category, and values.
+ERROR: invalid crosstab source data query
+DETAIL: The query must return at least 3 columns: row_name, category, and value.
SELECT * FROM crosstab(
'SELECT rowid, rowdt, rowdt, attribute, val FROM cth ORDER BY 1',
'SELECT DISTINCT attribute FROM cth ORDER BY 1')
AS c(rowid text, rowdt timestamp, temperature int4, test_result text, test_startdate timestamp, volts float8);
-ERROR: invalid return type
-DETAIL: Query-specified return tuple has 6 columns but crosstab returns 7.
+ERROR: invalid crosstab return type
+DETAIL: Return row must have 7 columns, not 6.
-- check errors with inappropriate result rowtype
SELECT * FROM crosstab(
'SELECT rowid, attribute, val FROM cth ORDER BY 1',
'SELECT DISTINCT attribute FROM cth ORDER BY 1')
AS c(rowid text);
-ERROR: query-specified return tuple and crosstab function are not compatible
+ERROR: invalid crosstab return type
+DETAIL: Return row must have at least two columns.
-- check it works with a named result rowtype
create type my_crosstab_result as (
rowid text, rowdt timestamp,
@@ -421,40 +425,42 @@ SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 4, '~') A
-- should fail as first two columns must have the same type
SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0, '~') AS t(keyid text, parent_keyid int,
levelint, branch text);
-ERROR: invalid return type
-DETAIL: First two columns must be the same type.
+ERROR: invalid connectby return type
+DETAIL: Source key type integer does not match return key type text.
-- should fail as key field datatype should match return datatype
SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0, '~') AS t(keyid float8, parent_keyid float8,
levelint, branch text);
-ERROR: invalid return type
-DETAIL: SQL key field type double precision does not match return key field type integer.
+ERROR: invalid connectby return type
+DETAIL: Source key type integer does not match return key type double precision.
SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0, '~') AS t(keyid int, parent_keyid float8,
levelint, branch text);
-ERROR: invalid return type
-DETAIL: First two columns must be the same type.
+ERROR: invalid connectby return type
+DETAIL: Source parent key type integer does not match return parent key type double precision.
-- check other rowtype mismatch cases
SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0) AS t(keyid int, parent_keyid int, level int,
branchtext);
-ERROR: invalid return type
-DETAIL: Query-specified return tuple has wrong number of columns.
+ERROR: invalid connectby return type
+DETAIL: Return row must have 3 columns, not 4.
SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0, '~') AS t(keyid int, parent_keyid int, level
int);
-ERROR: invalid return type
-DETAIL: Query-specified return tuple has wrong number of columns.
+ERROR: invalid connectby return type
+DETAIL: Return row must have 4 columns, not 3.
SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0) AS t(keyid int, parent_keyid text, level
int);
-ERROR: invalid return type
-DETAIL: First two columns must be the same type.
+ERROR: invalid connectby return type
+DETAIL: Source parent key type integer does not match return parent key type text.
SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0, '~') AS t(keyid int, parent_keyid int, level
float,branch float);
-ERROR: invalid return type
-DETAIL: Third column must be type integer.
+ERROR: invalid connectby return type
+DETAIL: Third return column (depth) must be type integer.
SELECT * FROM connectby('connectby_int', 'keyid', 'parent_keyid', '2', 0, '~') AS t(keyid int, parent_keyid int, level
int,branch float);
-ERROR: invalid return type
-DETAIL: Fourth column must be type text.
+ERROR: invalid connectby return type
+DETAIL: Fourth return column (branch) must be type text.
SELECT * FROM connectby('connectby_text', 'keyid', 'parent_keyid', 'pos', 'row2', 0, '~') AS t(keyid text,
parent_keyidtext, level int, branch text, pos text);
-ERROR: query-specified return tuple not valid for Connectby: fifth column must be type integer
+ERROR: invalid connectby return type
+DETAIL: Fifth return column (serial) must be type integer.
SELECT * FROM connectby('connectby_text', 'keyid', 'parent_keyid', 'pos', 'row2', 0) AS t(keyid text, parent_keyid
text,level int, pos text);
-ERROR: query-specified return tuple not valid for Connectby: fourth column must be type integer
+ERROR: invalid connectby return type
+DETAIL: Fourth return column (serial) must be type integer.
-- tests for values using custom queries
-- query with one column - failed
SELECT * FROM connectby('connectby_int', '1; --', 'parent_keyid', '2', 0) AS t(keyid int, parent_keyid int, level
int);
-ERROR: invalid return type
-DETAIL: Query must return at least two columns.
+ERROR: invalid connectby source data query
+DETAIL: The query must return at least two columns.
-- query with two columns first value as NULL
SELECT * FROM connectby('connectby_int', 'NULL::int, 1::int; --', 'parent_keyid', '2', 0) AS t(keyid int, parent_keyid
int,level int);
keyid | parent_keyid | level
diff --git a/contrib/tablefunc/tablefunc.c b/contrib/tablefunc/tablefunc.c
index ba17c9ba8e..8009becefe 100644
--- a/contrib/tablefunc/tablefunc.c
+++ b/contrib/tablefunc/tablefunc.c
@@ -52,7 +52,7 @@ static Tuplestorestate *get_crosstab_tuplestore(char *sql,
TupleDesc tupdesc,
bool randomAccess);
static void validateConnectbyTupleDesc(TupleDesc td, bool show_branch, bool show_serial);
-static bool compatCrosstabTupleDescs(TupleDesc ret_tupdesc, TupleDesc sql_tupdesc);
+static void compatCrosstabTupleDescs(TupleDesc ret_tupdesc, TupleDesc sql_tupdesc);
static void compatConnectbyTupleDescs(TupleDesc ret_tupdesc, TupleDesc sql_tupdesc);
static void get_normal_pair(float8 *x1, float8 *x2);
static Tuplestorestate *connectby(char *relname,
@@ -418,9 +418,8 @@ crosstab(PG_FUNCTION_ARGS)
if (spi_tupdesc->natts != 3)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("invalid source data SQL statement"),
- errdetail("The provided SQL must return 3 "
- "columns: rowid, category, and values.")));
+ errmsg("invalid crosstab source data query"),
+ errdetail("The query must return 3 columns: row_name, category, and value.")));
/* get a tuple descriptor for our result type */
switch (get_call_result_type(fcinfo, NULL, &tupdesc))
@@ -447,11 +446,7 @@ crosstab(PG_FUNCTION_ARGS)
* Check that return tupdesc is compatible with the data we got from SPI,
* at least based on number and type of attributes
*/
- if (!compatCrosstabTupleDescs(tupdesc, spi_tupdesc))
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("return and sql tuple descriptions are " \
- "incompatible")));
+ compatCrosstabTupleDescs(tupdesc, spi_tupdesc);
/*
* switch to long-lived memory context
@@ -673,9 +668,9 @@ crosstab_hash(PG_FUNCTION_ARGS)
*/
if (tupdesc->natts < 2)
ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("query-specified return tuple and " \
- "crosstab function are not compatible")));
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("invalid crosstab return type"),
+ errdetail("Return row must have at least two columns.")));
/* load up the categories hash table */
crosstab_hash = load_categories_hash(cats_sql, per_query_ctx);
@@ -750,9 +745,9 @@ load_categories_hash(char *cats_sql, MemoryContext per_query_ctx)
*/
if (spi_tupdesc->natts != 1)
ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("provided \"categories\" SQL must " \
- "return 1 column of at least one row")));
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid crosstab categories query"),
+ errdetail("The query must return one column.")));
for (i = 0; i < proc; i++)
{
@@ -767,9 +762,8 @@ load_categories_hash(char *cats_sql, MemoryContext per_query_ctx)
catname = SPI_getvalue(spi_tuple, spi_tupdesc, 1);
if (catname == NULL)
ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("provided \"categories\" SQL must " \
- "not return NULL values")));
+ (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+ errmsg("crosstab category value must not be null")));
SPIcontext = MemoryContextSwitchTo(per_query_ctx);
@@ -837,9 +831,8 @@ get_crosstab_tuplestore(char *sql,
{
/* no qualifying category tuples */
ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("provided \"categories\" SQL must " \
- "return 1 column of at least one row")));
+ (errcode(ERRCODE_CARDINALITY_VIOLATION),
+ errmsg("crosstab categories query must return at least one row")));
}
/*
@@ -858,20 +851,18 @@ get_crosstab_tuplestore(char *sql,
if (ncols < 3)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("invalid source data SQL statement"),
- errdetail("The provided SQL must return 3 " \
- " columns; rowid, category, and values.")));
+ errmsg("invalid crosstab source data query"),
+ errdetail("The query must return at least 3 columns: row_name, category, and value.")));
result_ncols = (ncols - 2) + num_categories;
- /* Recheck to make sure we tuple descriptor still looks reasonable */
+ /* Recheck to make sure output tuple descriptor looks reasonable */
if (tupdesc->natts != result_ncols)
ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("invalid return type"),
- errdetail("Query-specified return " \
- "tuple has %d columns but crosstab " \
- "returns %d.", tupdesc->natts, result_ncols)));
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("invalid crosstab return type"),
+ errdetail("Return row must have %d columns, not %d.",
+ result_ncols, tupdesc->natts)));
/* allocate space and make sure it's clear */
values = (char **) palloc0(result_ncols * sizeof(char *));
@@ -1422,77 +1413,62 @@ build_tuplestore_recursively(char *key_fld,
static void
validateConnectbyTupleDesc(TupleDesc td, bool show_branch, bool show_serial)
{
- int serial_column = 0;
-
- if (show_serial)
- serial_column = 1;
+ int expected_cols;
/* are there the correct number of columns */
if (show_branch)
- {
- if (td->natts != (CONNECTBY_NCOLS + serial_column))
- ereport(ERROR,
- (errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("invalid return type"),
- errdetail("Query-specified return tuple has " \
- "wrong number of columns.")));
- }
+ expected_cols = CONNECTBY_NCOLS;
else
- {
- if (td->natts != CONNECTBY_NCOLS_NOBRANCH + serial_column)
- ereport(ERROR,
- (errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("invalid return type"),
- errdetail("Query-specified return tuple has " \
- "wrong number of columns.")));
- }
+ expected_cols = CONNECTBY_NCOLS_NOBRANCH;
+ if (show_serial)
+ expected_cols++;
- /* check that the types of the first two columns match */
- if (TupleDescAttr(td, 0)->atttypid != TupleDescAttr(td, 1)->atttypid)
+ if (td->natts != expected_cols)
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("invalid return type"),
- errdetail("First two columns must be the same type.")));
+ errmsg("invalid connectby return type"),
+ errdetail("Return row must have %d columns, not %d.",
+ expected_cols, td->natts)));
+
+ /* the first two columns will be checked against the input tuples later */
/* check that the type of the third column is INT4 */
if (TupleDescAttr(td, 2)->atttypid != INT4OID)
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("invalid return type"),
- errdetail("Third column must be type %s.",
+ errmsg("invalid connectby return type"),
+ errdetail("Third return column (depth) must be type %s.",
format_type_be(INT4OID))));
- /* check that the type of the fourth column is TEXT if applicable */
+ /* check that the type of the branch column is TEXT if applicable */
if (show_branch && TupleDescAttr(td, 3)->atttypid != TEXTOID)
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("invalid return type"),
- errdetail("Fourth column must be type %s.",
+ errmsg("invalid connectby return type"),
+ errdetail("Fourth return column (branch) must be type %s.",
format_type_be(TEXTOID))));
- /* check that the type of the fifth column is INT4 */
+ /* check that the type of the serial column is INT4 if applicable */
if (show_branch && show_serial &&
TupleDescAttr(td, 4)->atttypid != INT4OID)
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("query-specified return tuple not valid for Connectby: "
- "fifth column must be type %s",
- format_type_be(INT4OID))));
-
- /* check that the type of the fourth column is INT4 */
+ errmsg("invalid connectby return type"),
+ errdetail("Fifth return column (serial) must be type %s.",
+ format_type_be(INT4OID))));
if (!show_branch && show_serial &&
TupleDescAttr(td, 3)->atttypid != INT4OID)
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("query-specified return tuple not valid for Connectby: "
- "fourth column must be type %s",
- format_type_be(INT4OID))));
+ errmsg("invalid connectby return type"),
+ errdetail("Fourth return column (serial) must be type %s.",
+ format_type_be(INT4OID))));
/* OK, the tupdesc is valid for our purposes */
}
/*
- * Check if spi sql tupdesc and return tupdesc are compatible
+ * Check if output tupdesc and SQL query's tupdesc are compatible
*/
static void
compatConnectbyTupleDescs(TupleDesc ret_tupdesc, TupleDesc sql_tupdesc)
@@ -1503,13 +1479,13 @@ compatConnectbyTupleDescs(TupleDesc ret_tupdesc, TupleDesc sql_tupdesc)
int32 sql_atttypmod;
/*
- * Result must have at least 2 columns.
+ * Query result must have at least 2 columns.
*/
if (sql_tupdesc->natts < 2)
ereport(ERROR,
- (errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("invalid return type"),
- errdetail("Query must return at least two columns.")));
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid connectby source data query"),
+ errdetail("The query must return at least two columns.")));
/*
* These columns must match the result type indicated by the calling
@@ -1523,11 +1499,10 @@ compatConnectbyTupleDescs(TupleDesc ret_tupdesc, TupleDesc sql_tupdesc)
(ret_atttypmod >= 0 && ret_atttypmod != sql_atttypmod))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("invalid return type"),
- errdetail("SQL key field type %s does " \
- "not match return key field type %s.",
- format_type_with_typemod(ret_atttypid, ret_atttypmod),
- format_type_with_typemod(sql_atttypid, sql_atttypmod))));
+ errmsg("invalid connectby return type"),
+ errdetail("Source key type %s does not match return key type %s.",
+ format_type_with_typemod(sql_atttypid, sql_atttypmod),
+ format_type_with_typemod(ret_atttypid, ret_atttypmod))));
ret_atttypid = TupleDescAttr(ret_tupdesc, 1)->atttypid;
sql_atttypid = TupleDescAttr(sql_tupdesc, 1)->atttypid;
@@ -1537,19 +1512,18 @@ compatConnectbyTupleDescs(TupleDesc ret_tupdesc, TupleDesc sql_tupdesc)
(ret_atttypmod >= 0 && ret_atttypmod != sql_atttypmod))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("invalid return type"),
- errdetail("SQL parent key field type %s does " \
- "not match return parent key field type %s.",
- format_type_with_typemod(ret_atttypid, ret_atttypmod),
- format_type_with_typemod(sql_atttypid, sql_atttypmod))));
+ errmsg("invalid connectby return type"),
+ errdetail("Source parent key type %s does not match return parent key type %s.",
+ format_type_with_typemod(sql_atttypid, sql_atttypmod),
+ format_type_with_typemod(ret_atttypid, ret_atttypmod))));
/* OK, the two tupdescs are compatible for our purposes */
}
/*
- * Check if two tupdescs match in type of attributes
+ * Check if crosstab output tupdesc agrees with input tupdesc
*/
-static bool
+static void
compatCrosstabTupleDescs(TupleDesc ret_tupdesc, TupleDesc sql_tupdesc)
{
int i;
@@ -1558,9 +1532,12 @@ compatCrosstabTupleDescs(TupleDesc ret_tupdesc, TupleDesc sql_tupdesc)
Form_pg_attribute sql_attr;
Oid sql_atttypid;
- if (ret_tupdesc->natts < 2 ||
- sql_tupdesc->natts < 3)
- return false;
+ if (ret_tupdesc->natts < 2)
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("invalid crosstab return type"),
+ errdetail("Return row must have at least two columns.")));
+ Assert(sql_tupdesc->natts == 3); /* already checked by caller */
/* check the rowid types match */
ret_atttypid = TupleDescAttr(ret_tupdesc, 0)->atttypid;
@@ -1568,9 +1545,10 @@ compatCrosstabTupleDescs(TupleDesc ret_tupdesc, TupleDesc sql_tupdesc)
if (ret_atttypid != sql_atttypid)
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("invalid return type"),
- errdetail("SQL rowid datatype does not match " \
- "return rowid datatype.")));
+ errmsg("invalid crosstab return type"),
+ errdetail("Source row_name datatype %s does not match return row_name datatype %s.",
+ format_type_be(sql_atttypid),
+ format_type_be(ret_atttypid))));
/*
* - attribute [1] of the sql tuple is the category; no need to check it -
@@ -1583,9 +1561,14 @@ compatCrosstabTupleDescs(TupleDesc ret_tupdesc, TupleDesc sql_tupdesc)
ret_attr = TupleDescAttr(ret_tupdesc, i);
if (ret_attr->atttypid != sql_attr->atttypid)
- return false;
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("invalid crosstab return type"),
+ errdetail("Source value datatype %s does not match return value datatype %s in column %d.",
+ format_type_be(sql_attr->atttypid),
+ format_type_be(ret_attr->atttypid),
+ i + 1)));
}
/* OK, the two tupdescs are compatible for our purposes */
- return true;
}
--
2.39.3
pgsql-hackers by date: