Re: Verbosity of Function Return Type Checks - Mailing list pgsql-hackers

On Fri, 8 Aug 2008, Alvaro Herrera <alvherre@commandprompt.com> writes:
> I think this is a good idea, but the new error messages need more work.
> Have a look at the message style guidelines please,
> http://www.postgresql.org/docs/8.3/static/error-style-guide.html

Right. Done -- I hope.

> Particularly I think you need to keep the original errmsg() and add the
> new messages as errdetail().  (I notice that there's the slight problem
> that the error messages are different for the different callers.)

Done.

> Also, please use context diffs.

Done.


Regards.

Index: src/pl/plpgsql/src/pl_exec.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v
retrieving revision 1.216
diff -r1.216 pl_exec.c
193c193
< static bool compatible_tupdesc(TupleDesc td1, TupleDesc td2);
---
> static void validate_tupdesc_compat(TupleDesc td1, TupleDesc td2);
389,393c389
<                     if (estate.rettupdesc == NULL ||
<                         !compatible_tupdesc(estate.rettupdesc, tupdesc))
<                         ereport(ERROR,
<                                 (errcode(ERRCODE_DATATYPE_MISMATCH),
<                                  errmsg("returned record type does not match expected record type")));
---
>                     validate_tupdesc_compat(tupdesc, estate.rettupdesc);
710,714c706,707
<         if (!compatible_tupdesc(estate.rettupdesc,
<                                 trigdata->tg_relation->rd_att))
<             ereport(ERROR,
<                     (errcode(ERRCODE_DATATYPE_MISMATCH),
<                      errmsg("returned tuple structure does not match table of trigger event")));
---
>         validate_tupdesc_compat(trigdata->tg_relation->rd_att,
>                                 estate.rettupdesc);
2204,2208c2197,2199
<                            errdetail("The tuple structure of a not-yet-assigned record is indeterminate.")));
<                     if (!compatible_tupdesc(tupdesc, rec->tupdesc))
<                         ereport(ERROR,
<                                 (errcode(ERRCODE_DATATYPE_MISMATCH),
<                         errmsg("wrong record type supplied in RETURN NEXT")));
---
>                            errdetail("The tuple structure of a not-yet-assigned"
>                                      " record is indeterminate.")));
>                     validate_tupdesc_compat(rec->tupdesc, tupdesc);
2314,2317c2305
<     if (!compatible_tupdesc(estate->rettupdesc, portal->tupDesc))
<         ereport(ERROR,
<                 (errcode(ERRCODE_DATATYPE_MISMATCH),
<           errmsg("structure of query does not match function result type")));
---
>     validate_tupdesc_compat(portal->tupDesc, estate->rettupdesc);
5141c5129,5130
<  * Check two tupledescs have matching number and types of attributes
---
>  * Validates compatibility of supplied TupleDesc couple by checking # and type
>  * of available arguments.
5143,5144c5132,5133
< static bool
< compatible_tupdesc(TupleDesc td1, TupleDesc td2)
---
> static void
> validate_tupdesc_compat(TupleDesc td1, TupleDesc td2)
5146c5135,5141
<     int            i;
---
>     int i;
>
>     if (!td1 || !td2)
>         ereport(ERROR,
>                 (errcode(ERRCODE_DATATYPE_MISMATCH),
>                  errmsg("returned record type does not match expected "
>                         "record type")));
5149c5144,5150
<         return false;
---
>         ereport(ERROR,
>                 (errcode(ERRCODE_DATATYPE_MISMATCH),
>                  errmsg("returned record type does not match expected "
>                         "record type"),
>                  errdetail("Number of returned columns (%d) does not match "
>                            "expected column count (%d).",
>                            td1->natts, td2->natts)));
5152d5152
<     {
5154,5157c5154,5164
<             return false;
<     }
<
<     return true;
---
>             ereport(ERROR,
>                     (errcode(ERRCODE_DATATYPE_MISMATCH),
>                      errmsg("returned record type does not match expected "
>                             "record type"),
>                      errdetail("Returned record type (%s) does not match "
>                                "expected record type (%s) in column %d (%s).",
>                                format_type_with_typemod(td1->attrs[i]->atttypid,
>                                                         td1->attrs[i]->atttypmod),
>                                format_type_with_typemod(td2->attrs[i]->atttypid,
>                                                         td2->attrs[i]->atttypmod),
>                                (1+i), NameStr(td2->attrs[i]->attname))));