On Mon, Apr 1, 2024 at 3:15 AM Tom Lane <t...@sss.pgh.pa.us> wrote:
>
>
> > The format "%d-%s" is not ideal. I suggesst "%d (%s)".
>
> I didn't like that either, for two reasons: if we have a column name
> it ought to be the prominent label, but we might not have one if the
> TupleDesc came from some anonymous source (possibly that case explains
> the test crash? Although I think the attname would be an empty string
> rather than missing entirely in such cases). I think it'd be worth
> providing two distinct message strings:
>
> "Returned type %s does not match expected type %s in column \"%s\" (position
> %d)."
> "Returned type %s does not match expected type %s in column position %d."
>
> I'd suggest dropping the column number entirely in the first case,
> were it not that the attnames might well not be unique if we're
> dealing with an anonymous record type such as a SELECT result.
>
please check the attached POC, hope the output is what you expected.
now we can output these two message.
> "Returned type %s does not match expected type %s in column \"%s\" (position
> %d)."
> "Returned type %s does not match expected type %s in column position %d."
create type compostype as (x int, y varchar);
create or replace function compos() returns compostype as $$
begin return (1, 'hello'); end;
$$ language plpgsql;
select compos();
HEAD error message is
ERROR: returned record type does not match expected record type
DETAIL: Returned type unknown does not match expected type character
varying in column 2.
CONTEXT: PL/pgSQL function compos() while casting return value to
function's return type
if we print out NameStr(att->attname) then error becomes:
+DETAIL: Returned type unknown does not match expected type character
varying in column "f2" (position 2).
In this case, printing out {column \"%s\"} is not helpful at all.
---------------case1
create function my_f(a integer, b integer)
returns table(first_col integer, lots_of_cols_later numeric) language plpgsql as
$function$
begin
return query select a, b;
end;
$function$;
-----------------case2
create or replace function returnsrecord(int) returns record language plpgsql as
$$ begin return row($1,$1+1); end $$;
select * from my_f(1,1);
select * from returnsrecord(42) as r(x int, y bigint);
In the first case, we want to print out the column \"%s\",
but in the second case, we don't.
in plpgsql_exec_function
first case, return first tuple values then check tuple attributes
in the second case, check the tuple attribute error out immediately.
build_attrmap_by_position both indesc->tdtypeid = 2249, outdesc->tdtypeid = 2249
so build_attrmap_by_position itself cannot distinguish these two cases.
To solve this,
we can add a boolean argument to convert_tuples_by_position.
Also this error
ERROR: structure of query does not match function result type
occurred quite often on the internet, see [1]
but there are no tests for it.
so we can add a test in src/test/regress/sql/plpgsql.sql
[1]
https://stackoverflow.com/search?q=structure+of+query+does+not+match+function+result+type
From b37ad3adc4534dd5dfb8bc78f58a6b81a6c078cf Mon Sep 17 00:00:00 2001
From: jian he <jian.universal...@gmail.com>
Date: Fri, 13 Sep 2024 15:54:43 +0800
Subject: [PATCH v2 1/1] improve error message in build_attrmap_by_position
make build_attrmap_by_position error message more helpful.
error message print out the column name conditionally.
minor refactor error message, change from "column %d" to "column position %d".
also add tests for "ERROR: structure of query does not match function result type"
discussion: https://postgr.es/m/cab-jlwanky28gjamdnmh1cjyo1b2zldr6uoa1-oy9g7pvl9...@mail.gmail.com
---
src/backend/access/common/attmap.c | 30 ++++++++++++++-----
src/backend/access/common/tupconvert.c | 5 ++--
src/backend/executor/tstoreReceiver.c | 3 +-
src/include/access/attmap.h | 3 +-
src/include/access/tupconvert.h | 3 +-
.../plpgsql/src/expected/plpgsql_record.out | 12 ++++----
src/pl/plpgsql/src/pl_exec.c | 18 +++++++----
src/test/regress/expected/plpgsql.out | 17 ++++++++++-
src/test/regress/sql/plpgsql.sql | 12 ++++++++
9 files changed, 78 insertions(+), 25 deletions(-)
diff --git a/src/backend/access/common/attmap.c b/src/backend/access/common/attmap.c
index b0fe27ef57..c55e587efd 100644
--- a/src/backend/access/common/attmap.c
+++ b/src/backend/access/common/attmap.c
@@ -74,7 +74,8 @@ free_attrmap(AttrMap *map)
AttrMap *
build_attrmap_by_position(TupleDesc indesc,
TupleDesc outdesc,
- const char *msg)
+ const char *msg,
+ bool extra)
{
AttrMap *attrMap;
int nincols;
@@ -115,15 +116,30 @@ build_attrmap_by_position(TupleDesc indesc,
/* Found matching column, now check type */
if (atttypid != att->atttypid ||
(atttypmod != att->atttypmod && atttypmod >= 0))
+ {
+ char *returned_type;
+ char *expected_type;
+
+ returned_type = format_type_with_typemod(att->atttypid,
+ att->atttypmod);
+ expected_type = format_type_with_typemod(atttypid,
+ atttypmod);
+
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg_internal("%s", _(msg)),
- errdetail("Returned type %s does not match expected type %s in column %d.",
- format_type_with_typemod(att->atttypid,
- att->atttypmod),
- format_type_with_typemod(atttypid,
- atttypmod),
- noutcols)));
+ extra ?
+ errdetail("Returned type %s does not match expected type %s in column \"%s\" (position %d).",
+ returned_type,
+ expected_type,
+ pstrdup(NameStr(att->attname)),
+ noutcols)
+ :
+ errdetail("Returned type %s does not match expected type %s in column position %d.",
+ returned_type,
+ expected_type,
+ noutcols)));
+ }
attrMap->attnums[i] = (AttrNumber) (j + 1);
j++;
break;
diff --git a/src/backend/access/common/tupconvert.c b/src/backend/access/common/tupconvert.c
index 4d596c4bef..e975cba879 100644
--- a/src/backend/access/common/tupconvert.c
+++ b/src/backend/access/common/tupconvert.c
@@ -58,14 +58,15 @@
TupleConversionMap *
convert_tuples_by_position(TupleDesc indesc,
TupleDesc outdesc,
- const char *msg)
+ const char *msg,
+ bool extra)
{
TupleConversionMap *map;
int n;
AttrMap *attrMap;
/* Verify compatibility and prepare attribute-number map */
- attrMap = build_attrmap_by_position(indesc, outdesc, msg);
+ attrMap = build_attrmap_by_position(indesc, outdesc, msg, extra);
if (attrMap == NULL)
{
diff --git a/src/backend/executor/tstoreReceiver.c b/src/backend/executor/tstoreReceiver.c
index de4646b5c2..e97ac62444 100644
--- a/src/backend/executor/tstoreReceiver.c
+++ b/src/backend/executor/tstoreReceiver.c
@@ -81,7 +81,8 @@ tstoreStartupReceiver(DestReceiver *self, int operation, TupleDesc typeinfo)
if (myState->target_tupdesc)
myState->tupmap = convert_tuples_by_position(typeinfo,
myState->target_tupdesc,
- myState->map_failure_msg);
+ myState->map_failure_msg,
+ true);
else
myState->tupmap = NULL;
diff --git a/src/include/access/attmap.h b/src/include/access/attmap.h
index 123ec45c13..3afdde5eeb 100644
--- a/src/include/access/attmap.h
+++ b/src/include/access/attmap.h
@@ -49,6 +49,7 @@ extern AttrMap *build_attrmap_by_name_if_req(TupleDesc indesc,
bool missing_ok);
extern AttrMap *build_attrmap_by_position(TupleDesc indesc,
TupleDesc outdesc,
- const char *msg);
+ const char *msg,
+ bool extra);
#endif /* ATTMAP_H */
diff --git a/src/include/access/tupconvert.h b/src/include/access/tupconvert.h
index 62a6d12761..2d8182782b 100644
--- a/src/include/access/tupconvert.h
+++ b/src/include/access/tupconvert.h
@@ -35,7 +35,8 @@ typedef struct TupleConversionMap
extern TupleConversionMap *convert_tuples_by_position(TupleDesc indesc,
TupleDesc outdesc,
- const char *msg);
+ const char *msg,
+ bool extra);
extern TupleConversionMap *convert_tuples_by_name(TupleDesc indesc,
TupleDesc outdesc);
diff --git a/src/pl/plpgsql/src/expected/plpgsql_record.out b/src/pl/plpgsql/src/expected/plpgsql_record.out
index 6974c8f4a4..319fd26972 100644
--- a/src/pl/plpgsql/src/expected/plpgsql_record.out
+++ b/src/pl/plpgsql/src/expected/plpgsql_record.out
@@ -28,7 +28,7 @@ create or replace function retc(int) returns two_int8s language plpgsql as
$$ begin return row($1,1); end $$;
select retc(42);
ERROR: returned record type does not match expected record type
-DETAIL: Returned type integer does not match expected type bigint in column 1.
+DETAIL: Returned type integer does not match expected type bigint in column position 1.
CONTEXT: PL/pgSQL function retc(integer) while casting return value to function's return type
-- nor extra columns
create or replace function retc(int) returns two_int8s language plpgsql as
@@ -50,7 +50,7 @@ create or replace function retc(int) returns two_int8s language plpgsql as
$$ declare r record; begin r := row($1,1); return r; end $$;
select retc(42);
ERROR: returned record type does not match expected record type
-DETAIL: Returned type integer does not match expected type bigint in column 1.
+DETAIL: Returned type integer does not match expected type bigint in column position 1.
CONTEXT: PL/pgSQL function retc(integer) while casting return value to function's return type
create or replace function retc(int) returns two_int8s language plpgsql as
$$ declare r record; begin r := row($1::int8, 1::int8, 42); return r; end $$;
@@ -386,7 +386,7 @@ DETAIL: Number of returned columns (2) does not match expected column count (3)
CONTEXT: PL/pgSQL function returnsrecord(integer) while casting return value to function's return type
select * from returnsrecord(42) as r(x int, y bigint); -- fail
ERROR: returned record type does not match expected record type
-DETAIL: Returned type integer does not match expected type bigint in column 2.
+DETAIL: Returned type integer does not match expected type bigint in column position 2.
CONTEXT: PL/pgSQL function returnsrecord(integer) while casting return value to function's return type
-- same with an intermediate record variable
create or replace function returnsrecord(int) returns record language plpgsql as
@@ -409,7 +409,7 @@ DETAIL: Number of returned columns (2) does not match expected column count (3)
CONTEXT: PL/pgSQL function returnsrecord(integer) while casting return value to function's return type
select * from returnsrecord(42) as r(x int, y bigint); -- fail
ERROR: returned record type does not match expected record type
-DETAIL: Returned type integer does not match expected type bigint in column 2.
+DETAIL: Returned type integer does not match expected type bigint in column position 2.
CONTEXT: PL/pgSQL function returnsrecord(integer) while casting return value to function's return type
-- should work the same with a missing column in the actual result value
create table has_hole(f1 int, f2 int, f3 int);
@@ -434,7 +434,7 @@ DETAIL: Number of returned columns (2) does not match expected column count (3)
CONTEXT: PL/pgSQL function returnsrecord(integer) while casting return value to function's return type
select * from returnsrecord(42) as r(x int, y bigint); -- fail
ERROR: returned record type does not match expected record type
-DETAIL: Returned type integer does not match expected type bigint in column 2.
+DETAIL: Returned type integer does not match expected type bigint in column position 2.
CONTEXT: PL/pgSQL function returnsrecord(integer) while casting return value to function's return type
-- same with an intermediate record variable
create or replace function returnsrecord(int) returns record language plpgsql as
@@ -457,7 +457,7 @@ DETAIL: Number of returned columns (2) does not match expected column count (3)
CONTEXT: PL/pgSQL function returnsrecord(integer) while casting return value to function's return type
select * from returnsrecord(42) as r(x int, y bigint); -- fail
ERROR: returned record type does not match expected record type
-DETAIL: Returned type integer does not match expected type bigint in column 2.
+DETAIL: Returned type integer does not match expected type bigint in column position 2.
CONTEXT: PL/pgSQL function returnsrecord(integer) while casting return value to function's return type
-- check access to a field of an argument declared "record"
create function getf1(x record) returns int language plpgsql as
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index ea9740e3f8..d7fda18f16 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -831,7 +831,8 @@ coerce_function_result_tuple(PLpgSQL_execstate *estate, TupleDesc tupdesc)
/* check rowtype compatibility */
tupmap = convert_tuples_by_position(retdesc,
tupdesc,
- gettext_noop("returned record type does not match expected record type"));
+ gettext_noop("returned record type does not match expected record type"),
+ false);
/* it might need conversion */
if (tupmap)
@@ -895,7 +896,8 @@ coerce_function_result_tuple(PLpgSQL_execstate *estate, TupleDesc tupdesc)
/* check rowtype compatibility */
tupmap = convert_tuples_by_position(retdesc,
tupdesc,
- gettext_noop("returned record type does not match expected record type"));
+ gettext_noop("returned record type does not match expected record type"),
+ false);
/* it might need conversion */
if (tupmap)
@@ -1091,7 +1093,8 @@ plpgsql_exec_trigger(PLpgSQL_function *func,
/* check rowtype compatibility */
tupmap = convert_tuples_by_position(retdesc,
RelationGetDescr(trigdata->tg_relation),
- gettext_noop("returned row structure does not match the structure of the triggering table"));
+ gettext_noop("returned row structure does not match the structure of the triggering table"),
+ false);
/* it might need conversion */
if (tupmap)
rettup = execute_attr_map_tuple(rettup, tupmap);
@@ -1119,7 +1122,8 @@ plpgsql_exec_trigger(PLpgSQL_function *func,
/* check rowtype compatibility */
tupmap = convert_tuples_by_position(retdesc,
RelationGetDescr(trigdata->tg_relation),
- gettext_noop("returned row structure does not match the structure of the triggering table"));
+ gettext_noop("returned row structure does not match the structure of the triggering table"),
+ false);
/* it might need conversion */
if (tupmap)
rettup = execute_attr_map_tuple(rettup, tupmap);
@@ -3415,7 +3419,8 @@ exec_stmt_return_next(PLpgSQL_execstate *estate,
rec_tupdesc = expanded_record_get_tupdesc(rec->erh);
tupmap = convert_tuples_by_position(rec_tupdesc,
tupdesc,
- gettext_noop("wrong record type supplied in RETURN NEXT"));
+ gettext_noop("wrong record type supplied in RETURN NEXT"),
+ false);
tuple = expanded_record_get_tuple(rec->erh);
if (tupmap)
tuple = execute_attr_map_tuple(tuple, tupmap);
@@ -3479,7 +3484,8 @@ exec_stmt_return_next(PLpgSQL_execstate *estate,
retvaldesc = deconstruct_composite_datum(retval, &tmptup);
tuple = &tmptup;
tupmap = convert_tuples_by_position(retvaldesc, tupdesc,
- gettext_noop("returned record type does not match expected record type"));
+ gettext_noop("returned record type does not match expected record type"),
+ false);
if (tupmap)
tuple = execute_attr_map_tuple(tuple, tupmap);
tuplestore_puttuple(estate->tuple_store, tuple);
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 0a6945581b..9bee00ded5 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -3777,7 +3777,7 @@ end;
$$ language plpgsql;
select compos();
ERROR: returned record type does not match expected record type
-DETAIL: Returned type unknown does not match expected type character varying in column 2.
+DETAIL: Returned type unknown does not match expected type character varying in column position 2.
CONTEXT: PL/pgSQL function compos() while casting return value to function's return type
-- ... but this does
create or replace function compos() returns compostype as $$
@@ -5852,3 +5852,18 @@ END; $$ LANGUAGE plpgsql;
ERROR: "x" is not a scalar variable
LINE 3: GET DIAGNOSTICS x = ROW_COUNT;
^
+--
+-- Check returned type with the declared type
+--
+create function my_f(a integer, b integer)
+returns table(first_col integer, lots_of_cols_later numeric) language plpgsql as
+$function$
+begin
+ return query select a, b;
+end;
+$function$;
+select * from my_f(1,1);
+ERROR: structure of query does not match function result type
+DETAIL: Returned type integer does not match expected type numeric in column "b" (position 2).
+CONTEXT: SQL statement "select a, b"
+PL/pgSQL function my_f(integer,integer) line 3 at RETURN QUERY
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index 18c91572ae..d63afc8b97 100644
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -4774,3 +4774,15 @@ BEGIN
GET DIAGNOSTICS x = ROW_COUNT;
RETURN;
END; $$ LANGUAGE plpgsql;
+
+--
+-- Check returned type with the declared type
+--
+create function my_f(a integer, b integer)
+returns table(first_col integer, lots_of_cols_later numeric) language plpgsql as
+$function$
+begin
+ return query select a, b;
+end;
+$function$;
+select * from my_f(1,1);
\ No newline at end of file
base-commit: 2b67bdca529c6aed4303eb6963d09d1b540137b8
--
2.34.1
