On Thu, 04 Sep 2008, Tom Lane <[EMAIL PROTECTED]> writes:
> This is not ready to go: you've lost the ability to localize most of
> the error message strings.

How can I make this available? What's your suggestion?

> Also, "char *msg" should be "const char *msg"

Done.

> if you're going to pass literal constants to it, and this gives me the
> willies even though the passed-in strings are supposedly all fixed:
>               errmsg(msg),
> Use
>               errmsg("%s", msg),
> to be safe.

Done.

> Actually, the entire concept of varying the main message to suit the
> context exactly, while the detail messages are *not* changing, seems
> pretty bogus...

I share your concerns but couldn't come up with a better approach. I'd
be happy to hear your suggestions.

> Another problem with it is it's likely going to fail completely on
> dropped columns (which will have atttypid = 0).

Is it ok if I'd replace

  if (td1->attrs[i]->atttypid != td2->attrs[i]->atttypid)

line in validate_tupdesc_compat with

  if (td1->attrs[i]->atttypid &&
      td2->attrs[i]->atttypid &&
      td1->attrs[i]->atttypid != td2->attrs[i]->atttypid)

expression to fix this?


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.219
diff -c -r1.219 pl_exec.c
*** src/pl/plpgsql/src/pl_exec.c	1 Sep 2008 22:30:33 -0000	1.219
--- src/pl/plpgsql/src/pl_exec.c	5 Sep 2008 06:13:18 -0000
***************
*** 188,194 ****
  					   Oid reqtype, int32 reqtypmod,
  					   bool isnull);
  static void exec_init_tuple_store(PLpgSQL_execstate *estate);
! static bool compatible_tupdesc(TupleDesc td1, TupleDesc td2);
  static void exec_set_found(PLpgSQL_execstate *estate, bool state);
  static void plpgsql_create_econtext(PLpgSQL_execstate *estate);
  static void free_var(PLpgSQL_var *var);
--- 188,195 ----
  					   Oid reqtype, int32 reqtypmod,
  					   bool isnull);
  static void exec_init_tuple_store(PLpgSQL_execstate *estate);
! static void validate_tupdesc_compat(TupleDesc td1, TupleDesc td2,
! 									const char *msg);
  static void exec_set_found(PLpgSQL_execstate *estate, bool state);
  static void plpgsql_create_econtext(PLpgSQL_execstate *estate);
  static void free_var(PLpgSQL_var *var);
***************
*** 384,394 ****
  			{
  				case TYPEFUNC_COMPOSITE:
  					/* got the expected result rowtype, now check it */
! 					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")));
  					break;
  				case TYPEFUNC_RECORD:
  
--- 385,393 ----
  			{
  				case TYPEFUNC_COMPOSITE:
  					/* got the expected result rowtype, now check it */
! 					validate_tupdesc_compat(tupdesc, estate.rettupdesc,
! 											"returned record type does not "
! 											"match expected record type");
  					break;
  				case TYPEFUNC_RECORD:
  
***************
*** 705,715 ****
  		rettup = NULL;
  	else
  	{
! 		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")));
  		/* Copy tuple to upper executor memory */
  		rettup = SPI_copytuple((HeapTuple) DatumGetPointer(estate.retval));
  	}
--- 704,713 ----
  		rettup = NULL;
  	else
  	{
! 		validate_tupdesc_compat(trigdata->tg_relation->rd_att,
! 								estate.rettupdesc,
! 								"returned tuple structure does not match "
! 								"table of trigger event");
  		/* Copy tuple to upper executor memory */
  		rettup = SPI_copytuple((HeapTuple) DatumGetPointer(estate.retval));
  	}
***************
*** 2199,2209 ****
  						  (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
  						   errmsg("record \"%s\" is not assigned yet",
  								  rec->refname),
! 						   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")));
  					tuple = rec->tup;
  				}
  				break;
--- 2197,2207 ----
  						  (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
  						   errmsg("record \"%s\" is not assigned yet",
  								  rec->refname),
! 						   errdetail("The tuple structure of a not-yet-assigned"
! 									 " record is indeterminate.")));
! 					validate_tupdesc_compat(rec->tupdesc, tupdesc,
! 						                    "wrong record type supplied in "
! 											"RETURN NEXT");
  					tuple = rec->tup;
  				}
  				break;
***************
*** 2309,2318 ****
  										   stmt->params);
  	}
  
! 	if (!compatible_tupdesc(estate->rettupdesc, portal->tupDesc))
! 		ereport(ERROR,
! 				(errcode(ERRCODE_DATATYPE_MISMATCH),
! 		  errmsg("structure of query does not match function result type")));
  
  	while (true)
  	{
--- 2307,2315 ----
  										   stmt->params);
  	}
  
! 	validate_tupdesc_compat(portal->tupDesc, estate->rettupdesc,
! 							"structure of query does not match function "
! 							"result type");
  
  	while (true)
  	{
***************
*** 5145,5167 ****
  }
  
  /*
!  * Check two tupledescs have matching number and types of attributes
   */
! static bool
! compatible_tupdesc(TupleDesc td1, TupleDesc td2)
  {
! 	int			i;
  
  	if (td1->natts != td2->natts)
! 		return false;
  
  	for (i = 0; i < td1->natts; i++)
- 	{
  		if (td1->attrs[i]->atttypid != td2->attrs[i]->atttypid)
! 			return false;
! 	}
! 
! 	return true;
  }
  
  /* ----------
--- 5142,5178 ----
  }
  
  /*
!  * Validates compatibility of supplied TupleDesc couple by checking # and type
!  * of available arguments.
   */
! static void
! validate_tupdesc_compat(TupleDesc td1, TupleDesc td2, const char *msg)
  {
! 	int i;
! 
! 	if (!td1 || !td2)
! 		ereport(ERROR, (errcode(ERRCODE_DATATYPE_MISMATCH), errmsg(msg)));
  
  	if (td1->natts != td2->natts)
! 		ereport(ERROR,
! 				(errcode(ERRCODE_DATATYPE_MISMATCH),
! 				 errmsg("%s", msg),
! 				 errdetail("Number of returned columns (%d) does not match "
! 						   "expected column count (%d).",
! 						   td1->natts, td2->natts)));
  
  	for (i = 0; i < td1->natts; i++)
  		if (td1->attrs[i]->atttypid != td2->attrs[i]->atttypid)
! 			ereport(ERROR,
! 					(errcode(ERRCODE_DATATYPE_MISMATCH),
! 					 errmsg("%s", msg),
! 					 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