Saturday, August 9, 2008

Re: [HACKERS] Verbosity of Function Return Type Checks

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))));
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.

No comments: