On Fri, 8 Aug 2008, Alvaro Herrera <[EMAIL PROTECTED]> 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))));
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to