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.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 13:47:07 -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 expected, TupleDesc returned,
! 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,392 ----
{
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));
}
--- 703,711 ----
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;
--- 2195,2204 ----
(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(tupdesc, rec->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)
{
--- 2304,2311 ----
stmt->params);
}

! validate_tupdesc_compat(estate->rettupdesc, portal->tupDesc,
! "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;
}

/* ----------
--- 5138,5174 ----
}

/*
! * Validates compatibility of supplied TupleDesc pair by checking number and type
! * of attributes.
*/
! static void
! validate_tupdesc_compat(TupleDesc expected, TupleDesc returned, const char *msg)
{
! int i;

! if (!expected || !returned)
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg("%s", msg)));

! if (expected->natts != returned->natts)
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg("%s", msg),
! errdetail("Number of returned columns (%d) does not match expected column count (%d).",
! returned->natts, expected->natts)));

! for (i = 0; i < expected->natts; i++)
! if (expected->attrs[i]->atttypid != returned->attrs[i]->atttypid)
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg("%s", msg),
! errdetail("Returned type \"%s\" does not match expected type \"%s\" in column \"%s\".",
! format_type_with_typemod(returned->attrs[i]->atttypid,
! returned->attrs[i]->atttypmod),
! format_type_with_typemod(expected->attrs[i]->atttypid,
! expected->attrs[i]->atttypmod),
! NameStr(expected->attrs[i]->attname))));
}

/* ----------
On Fri, 5 Sep 2008, Alvaro Herrera <alvherre@commandprompt.com> writes:
> Please use the patch I posted yesterday, as it had all the issues I
> found fixed. There were other changes in that patch too.

My bad. Patch is modified with respect to suggestions[1][2] from
Tom. (All 115 tests passed in cvs tip.)

Regards.

[1] "char *msg" is replaced with "const char *msg".

[2] "errmsg(msg)" is replaced with 'errmsg("%s", msg)'.