Alvaro Herrera wrote:

>On Wed, Jan 12, 2005 at 09:23:26AM +1100, Brendan Jurd wrote:
>
>
>>This patch to src/backend/executor/nodeFunctionscan.c is intended to
>>make life a little easier for people using row-returning functions, by
>>increasing the level of detail in the error messages thrown when
>>tupledesc_match fails.
>>
>>
>
>You should get rid of the returns, because ereport(ERROR) will never
>return control to the function and they are thus dead code. And make
>the function return void rather than bool.
>
>Also follow the style: use "if (foo)" rather than "if( foo )". And
>message style stipulates that the errdetail() message should start with
>a capital (upper case?) letter.
>
>
>
Thanks Alvaro, changes made and new patch attached.

Index: src/backend/executor/nodeFunctionscan.c
================================================== =================
RCS file: /projects/cvsroot/pgsql/src/backend/executor/nodeFunctionscan.c,v
retrieving revision 1.29
diff -c -r1.29 nodeFunctionscan.c
*** src/backend/executor/nodeFunctionscan.c 31 Dec 2004 21:59:45 -0000 1.29
--- src/backend/executor/nodeFunctionscan.c 12 Jan 2005 06:36:53 -0000
***************
*** 36,42 ****


static TupleTableSlot *FunctionNext(FunctionScanState *node);
! static bool tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc);

/* ----------------------------------------------------------------
* Scan Support
--- 36,42 ----


static TupleTableSlot *FunctionNext(FunctionScanState *node);
! static void tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc);

/* ----------------------------------------------------------------
* Scan Support
***************
*** 87,96 ****
* need to do this for functions returning RECORD, but might as
* well do it always.
*/
! if (funcTupdesc && !tupledesc_match(node->tupdesc, funcTupdesc))
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg("query-specified return row and actual function return row do not match")));
}

/*
--- 87,94 ----
* need to do this for functions returning RECORD, but might as
* well do it always.
*/
! if (funcTupdesc)
! tupledesc_match(node->tupdesc, funcTupdesc);
}

/*
***************
*** 357,369 ****
* destination type, so long as the physical storage matches. This is
* helpful in some cases involving out-of-date cached plans.
*/
! static bool
tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc)
{
int i;

if (dst_tupdesc->natts != src_tupdesc->natts)
! return false;

for (i = 0; i < dst_tupdesc->natts; i++)
{
--- 355,370 ----
* destination type, so long as the physical storage matches. This is
* helpful in some cases involving out-of-date cached plans.
*/
! static void
tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc)
{
int i;

if (dst_tupdesc->natts != src_tupdesc->natts)
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg("function return row and query-specified return row do not match"),
! errdetail("Function-returned row contains %d attributes, but query expects %d.", src_tupdesc->natts, dst_tupdesc->natts)));

for (i = 0; i < dst_tupdesc->natts; i++)
{
***************
*** 373,382 ****
if (dattr->atttypid == sattr->atttypid)
continue; /* no worries */
if (!dattr->attisdropped)
! return false;
if (dattr->attlen != sattr->attlen ||
dattr->attalign != sattr->attalign)
! return false;
}

return true;
--- 374,393 ----
if (dattr->atttypid == sattr->atttypid)
continue; /* no worries */
if (!dattr->attisdropped)
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg("function return row and query-specified return row do not match"),
! errdetail("Function returned type %s at ordinal position %d, but query expects %s.",
! format_type_be(sattr->atttypid),
! i+1,
! format_type_be(dattr->atttypid))));
!
if (dattr->attlen != sattr->attlen ||
dattr->attalign != sattr->attalign)
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg("function return row and query-specified return row do not match"),
! errdetail("Physical storage mismatch on dropped attribute at ordinal position %d.", i+1)));
}

return true;


---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faqs/FAQ.html

This has been saved for the 8.1 release:

http:/momjian.postgresql.org/cgi-bin/pgpatches2

---------------------------------------------------------------------------

Brendan Jurd wrote:
> Alvaro Herrera wrote:
>
> >On Wed, Jan 12, 2005 at 09:23:26AM +1100, Brendan Jurd wrote:
> >
> >
> >>This patch to src/backend/executor/nodeFunctionscan.c is intended to
> >>make life a little easier for people using row-returning functions, by
> >>increasing the level of detail in the error messages thrown when
> >>tupledesc_match fails.
> >>
> >>
> >
> >You should get rid of the returns, because ereport(ERROR) will never
> >return control to the function and they are thus dead code. And make
> >the function return void rather than bool.
> >
> >Also follow the style: use "if (foo)" rather than "if( foo )". And
> >message style stipulates that the errdetail() message should start with
> >a capital (upper case?) letter.
> >
> >
> >
> Thanks Alvaro, changes made and new patch attached.

> Index: src/backend/executor/nodeFunctionscan.c
> ================================================== =================
> RCS file: /projects/cvsroot/pgsql/src/backend/executor/nodeFunctionscan.c,v
> retrieving revision 1.29
> diff -c -r1.29 nodeFunctionscan.c
> *** src/backend/executor/nodeFunctionscan.c 31 Dec 2004 21:59:45 -0000 1.29
> --- src/backend/executor/nodeFunctionscan.c 12 Jan 2005 06:36:53 -0000
> ***************
> *** 36,42 ****
>
>
> static TupleTableSlot *FunctionNext(FunctionScanState *node);
> ! static bool tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc);
>
> /* ----------------------------------------------------------------
> * Scan Support
> --- 36,42 ----
>
>
> static TupleTableSlot *FunctionNext(FunctionScanState *node);
> ! static void tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc);
>
> /* ----------------------------------------------------------------
> * Scan Support
> ***************
> *** 87,96 ****
> * need to do this for functions returning RECORD, but might as
> * well do it always.
> */
> ! if (funcTupdesc && !tupledesc_match(node->tupdesc, funcTupdesc))
> ! ereport(ERROR,
> ! (errcode(ERRCODE_DATATYPE_MISMATCH),
> ! errmsg("query-specified return row and actual function return row do not match")));
> }
>
> /*
> --- 87,94 ----
> * need to do this for functions returning RECORD, but might as
> * well do it always.
> */
> ! if (funcTupdesc)
> ! tupledesc_match(node->tupdesc, funcTupdesc);
> }
>
> /*
> ***************
> *** 357,369 ****
> * destination type, so long as the physical storage matches. This is
> * helpful in some cases involving out-of-date cached plans.
> */
> ! static bool
> tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc)
> {
> int i;
>
> if (dst_tupdesc->natts != src_tupdesc->natts)
> ! return false;
>
> for (i = 0; i < dst_tupdesc->natts; i++)
> {
> --- 355,370 ----
> * destination type, so long as the physical storage matches. This is
> * helpful in some cases involving out-of-date cached plans.
> */
> ! static void
> tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc)
> {
> int i;
>
> if (dst_tupdesc->natts != src_tupdesc->natts)
> ! ereport(ERROR,
> ! (errcode(ERRCODE_DATATYPE_MISMATCH),
> ! errmsg("function return row and query-specified return row do not match"),
> ! errdetail("Function-returned row contains %d attributes, but query expects %d.", src_tupdesc->natts, dst_tupdesc->natts)));
>
> for (i = 0; i < dst_tupdesc->natts; i++)
> {
> ***************
> *** 373,382 ****
> if (dattr->atttypid == sattr->atttypid)
> continue; /* no worries */
> if (!dattr->attisdropped)
> ! return false;
> if (dattr->attlen != sattr->attlen ||
> dattr->attalign != sattr->attalign)
> ! return false;
> }
>
> return true;
> --- 374,393 ----
> if (dattr->atttypid == sattr->atttypid)
> continue; /* no worries */
> if (!dattr->attisdropped)
> ! ereport(ERROR,
> ! (errcode(ERRCODE_DATATYPE_MISMATCH),
> ! errmsg("function return row and query-specified return row do not match"),
> ! errdetail("Function returned type %s at ordinal position %d, but query expects %s.",
> ! format_type_be(sattr->atttypid),
> ! i+1,
> ! format_type_be(dattr->atttypid))));
> !
> if (dattr->attlen != sattr->attlen ||
> dattr->attalign != sattr->attalign)
> ! ereport(ERROR,
> ! (errcode(ERRCODE_DATATYPE_MISMATCH),
> ! errmsg("function return row and query-specified return row do not match"),
> ! errdetail("Physical storage mismatch on dropped attribute at ordinal position %d.", i+1)));
> }
>
> return true;

>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/docs/faqs/FAQ.html

--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faqs/FAQ.html