Re: why can't plpgsql return a row-expression?

Started by Alvaro Herreraabout 13 years ago15 messages
#1Alvaro Herrera
alvherre@2ndquadrant.com

Asif Rehman escribió:

Hi,

I have tried to solve this issue. Please see the attached patch.

With this patch, any expression is allowed in the return statement. For any
invalid expression an error is generated without doing any special handling.
When a row expression is used in the return statement then the resultant
tuple will have rowtype in a single column that needed to be extracted.
Hence I have handled that case in exec_stmt_return().

any comments/suggestions?

Hmm. We're running an I/O cast during do_tup_convert() now, and look
up the required functions for each tuple. I think if we're going to
support this operation at that level, we need to look up the necessary
functions at convert_tuples_by_foo level, and then apply unconditionally
if they've been set up.

Also, what are the implicancies for existing users of tupconvert? Do we
want to apply casting during ANALYZE for example? AFAICS the patch
shouldn't break any case that works today, but I don't see that there
has been any analysis of this.

(I looked at the patch posted in the thread started by Pavel elsewhere.
I'm replying to both emails in the interest of keeping things properly
linked.)

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Asif Rehman
asifr.rehman@gmail.com
In reply to: Alvaro Herrera (#1)
1 attachment(s)

Hi,

Thanks for the review. Please see the updated patch.

Hmm. We're running an I/O cast during do_tup_convert() now, and look

up the required functions for each tuple. I think if we're going to
support this operation at that level, we need to look up the necessary
functions at convert_tuples_by_foo level, and then apply unconditionally
if they've been set up.

Done. TupleConversionMap struct should keep the array of functions oid's
that
needs to be applied. Though only for those cases where both attribute type
id's
do not match. This way no unnecessary casting will happen.

Also, what are the implicancies for existing users of tupconvert? Do we
want to apply casting during ANALYZE for example? AFAICS the patch
shouldn't break any case that works today, but I don't see that there
has been any analysis of this.

I believe this part of the code should not impact existing users of
tupconvert.
As this part of code is relaxing a restriction upon which an error would
have been
generated. Though it could have a little impact on performance but should
be only for
cases where attribute type id's are different and are implicitly cast able.

Besides existing users involve tupconvert in case of inheritance. And in
that case
an exact match is expected.

Regards,
--Asif

Attachments:

return_rowtype.v2.patchapplication/octet-stream; name=return_rowtype.v2.patchDownload
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 07fba57..58b1489 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -1571,11 +1571,9 @@ RETURN <replaceable>expression</replaceable>;
      </para>
 
      <para>
-      When returning a scalar type, any expression can be used. The
-      expression's result will be automatically cast into the
-      function's return type as described for assignments. To return a
-      composite (row) value, you must write a record or row variable
-      as the <replaceable>expression</replaceable>.
+      In <command>RETURN</command> statement you can use any expression.
+       The expression's result will be automatically cast into the
+      function's return type as described for assignments.
      </para>
 
      <para>
@@ -1600,6 +1598,21 @@ RETURN <replaceable>expression</replaceable>;
       however.  In those cases a <command>RETURN</command> statement is
       automatically executed if the top-level block finishes.
      </para>
+
+     <para>
+      Some examples:
+
+<programlisting>
+-- functions returning a scalar type
+RETURN 1 + 2;
+RETURN scalar_var;
+
+-- functions returning a composite type
+RETURN composite_type_var;
+RETURN (1, 2, 'three');
+</programlisting>
+
+     </para>
     </sect3>
 
     <sect3>
diff --git a/src/backend/access/common/tupconvert.c b/src/backend/access/common/tupconvert.c
index f813432..0f5ac50 100644
--- a/src/backend/access/common/tupconvert.c
+++ b/src/backend/access/common/tupconvert.c
@@ -22,7 +22,9 @@
 
 #include "access/htup_details.h"
 #include "access/tupconvert.h"
+#include "parser/parse_coerce.h"
 #include "utils/builtins.h"
+#include "utils/lsyscache.h"
 
 
 /*
@@ -69,6 +71,9 @@ convert_tuples_by_position(TupleDesc indesc,
 {
 	TupleConversionMap *map;
 	AttrNumber *attrMap;
+	Oid		   *typoutput;
+	Oid		   *typinput;
+	Oid		   *typioparam;
 	int			nincols;
 	int			noutcols;
 	int			n;
@@ -78,7 +83,10 @@ convert_tuples_by_position(TupleDesc indesc,
 
 	/* Verify compatibility and prepare attribute-number map */
 	n = outdesc->natts;
-	attrMap = (AttrNumber *) palloc0(n * sizeof(AttrNumber));
+	attrMap 	= (AttrNumber *) palloc0(n * sizeof(AttrNumber));
+	typinput 	= (Oid *) palloc0(n * sizeof(Oid));
+	typioparam	= (Oid *) palloc(n * sizeof(Oid));
+	typoutput 	= (Oid *) palloc((indesc->natts + 1) * sizeof(Oid)); /* +1 for NULL */
 	j = 0;						/* j is next physical input attribute */
 	nincols = noutcols = 0;		/* these count non-dropped attributes */
 	same = true;
@@ -100,8 +108,8 @@ convert_tuples_by_position(TupleDesc indesc,
 				continue;
 			nincols++;
 			/* Found matching column, check type */
-			if (atttypid != att->atttypid ||
-				(atttypmod != att->atttypmod && atttypmod >= 0))
+			if ((atttypmod != att->atttypmod && atttypmod >= 0) ||
+				!can_coerce_type(1, &atttypid, &(att->atttypid), COERCE_IMPLICIT_CAST))
 				ereport(ERROR,
 						(errcode(ERRCODE_DATATYPE_MISMATCH),
 						 errmsg_internal("%s", _(msg)),
@@ -111,6 +119,17 @@ convert_tuples_by_position(TupleDesc indesc,
 								   format_type_with_typemod(atttypid,
 															atttypmod),
 								   noutcols)));
+			/*
+			 * get the needed I/O functions to perform cast later in
+			 * do_convert_tuple, no need if type id's are same.
+			 */
+			if (atttypid != att->atttypid)
+			{
+				bool typIsVarlena;
+
+				getTypeOutputInfo(att->atttypid, &typoutput[j + 1], &typIsVarlena);
+				getTypeInputInfo(atttypid, &typinput[i], &typioparam[i]);
+			}
 			attrMap[i] = (AttrNumber) (j + 1);
 			j++;
 			break;
@@ -183,9 +202,12 @@ convert_tuples_by_position(TupleDesc indesc,
 	/* preallocate workspace for Datum arrays */
 	map->outvalues = (Datum *) palloc(n * sizeof(Datum));
 	map->outisnull = (bool *) palloc(n * sizeof(bool));
+	map->typinput = typinput;
+	map->typioparam = typioparam;
 	n = indesc->natts + 1;		/* +1 for NULL */
 	map->invalues = (Datum *) palloc(n * sizeof(Datum));
 	map->inisnull = (bool *) palloc(n * sizeof(bool));
+	map->typoutput = typoutput;
 	map->invalues[0] = (Datum) 0;		/* set up the NULL entry */
 	map->inisnull[0] = true;
 
@@ -206,6 +228,9 @@ convert_tuples_by_name(TupleDesc indesc,
 {
 	TupleConversionMap *map;
 	AttrNumber *attrMap;
+	Oid		   *typoutput;
+	Oid		   *typinput;
+	Oid		   *typioparam;
 	int			n;
 	int			i;
 	bool		same;
@@ -213,6 +238,9 @@ convert_tuples_by_name(TupleDesc indesc,
 	/* Verify compatibility and prepare attribute-number map */
 	n = outdesc->natts;
 	attrMap = (AttrNumber *) palloc0(n * sizeof(AttrNumber));
+	typinput 	= (Oid *) palloc0(n * sizeof(Oid));
+	typioparam	= (Oid *) palloc(n * sizeof(Oid));
+	typoutput 	= (Oid *) palloc((indesc->natts + 1) * sizeof(Oid)); /* +1 for NULL */
 	for (i = 0; i < n; i++)
 	{
 		Form_pg_attribute att = outdesc->attrs[i];
@@ -234,7 +262,8 @@ convert_tuples_by_name(TupleDesc indesc,
 			if (strcmp(attname, NameStr(att->attname)) == 0)
 			{
 				/* Found it, check type */
-				if (atttypid != att->atttypid || atttypmod != att->atttypmod)
+				if (atttypmod != att->atttypmod ||
+					!can_coerce_type(1, &atttypid, &(att->atttypid), COERCE_IMPLICIT_CAST))
 					ereport(ERROR,
 							(errcode(ERRCODE_DATATYPE_MISMATCH),
 							 errmsg_internal("%s", _(msg)),
@@ -242,6 +271,17 @@ convert_tuples_by_name(TupleDesc indesc,
 									   attname,
 									   format_type_be(outdesc->tdtypeid),
 									   format_type_be(indesc->tdtypeid))));
+				/*
+				 * get the needed I/O functions to perform cast later in
+				 * do_convert_tuple, no need if type id's are same.
+				 */
+				if (atttypid != att->atttypid)
+				{
+					bool typIsVarlena;
+
+					getTypeOutputInfo(att->atttypid, &typoutput[j + 1], &typIsVarlena);
+					getTypeInputInfo(atttypid, &typinput[i], &typioparam[i]);
+				}
 				attrMap[i] = (AttrNumber) (j + 1);
 				break;
 			}
@@ -303,9 +343,12 @@ convert_tuples_by_name(TupleDesc indesc,
 	/* preallocate workspace for Datum arrays */
 	map->outvalues = (Datum *) palloc(n * sizeof(Datum));
 	map->outisnull = (bool *) palloc(n * sizeof(bool));
+	map->typinput = typinput;
+	map->typioparam = typioparam;
 	n = indesc->natts + 1;		/* +1 for NULL */
 	map->invalues = (Datum *) palloc(n * sizeof(Datum));
 	map->inisnull = (bool *) palloc(n * sizeof(bool));
+	map->typoutput = typoutput;
 	map->invalues[0] = (Datum) 0;		/* set up the NULL entry */
 	map->inisnull[0] = true;
 
@@ -342,6 +385,21 @@ do_convert_tuple(HeapTuple tuple, TupleConversionMap *map)
 
 		outvalues[i] = invalues[j];
 		outisnull[i] = inisnull[j];
+
+		/* Perform the casting, if necessary. */
+		if (!inisnull[j] &&
+			OidIsValid(map->typinput[i]))
+		{
+			Form_pg_attribute att = map->outdesc->attrs[i];
+			char *strval;
+
+			strval = OidOutputFunctionCall(map->typoutput[j], invalues[j]);
+			outvalues[i] = OidInputFunctionCall(map->typinput[i],
+											 strval,
+											 map->typioparam[i],
+											 att->atttypmod);
+			pfree(strval);
+		}
 	}
 
 	/*
@@ -360,7 +418,10 @@ free_conversion_map(TupleConversionMap *map)
 	pfree(map->attrMap);
 	pfree(map->invalues);
 	pfree(map->inisnull);
+	pfree(map->typoutput);
 	pfree(map->outvalues);
 	pfree(map->outisnull);
+	pfree(map->typinput);
+	pfree(map->typioparam);
 	pfree(map);
 }
diff --git a/src/include/access/tupconvert.h b/src/include/access/tupconvert.h
index 342dbb4..61ef7ae 100644
--- a/src/include/access/tupconvert.h
+++ b/src/include/access/tupconvert.h
@@ -25,8 +25,11 @@ typedef struct TupleConversionMap
 	AttrNumber *attrMap;		/* indexes of input fields, or 0 for null */
 	Datum	   *invalues;		/* workspace for deconstructing source */
 	bool	   *inisnull;
+	Oid		   *typoutput;		/* output functions for source type */
 	Datum	   *outvalues;		/* workspace for constructing result */
 	bool	   *outisnull;
+	Oid		   *typinput;		/* input functions for result type */
+	Oid		   *typioparam;
 } TupleConversionMap;
 
 
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 3b5b3bb..d1c0acd 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -214,6 +214,7 @@ static void free_params_data(PreparedParamsData *ppd);
 static Portal exec_dynquery_with_params(PLpgSQL_execstate *estate,
 						  PLpgSQL_expr *dynquery, List *params,
 						  const char *portalname, int cursorOptions);
+static HeapTuple get_tuple_from_datum(Datum value, TupleDesc *rettupdesc);
 
 
 /* ----------
@@ -275,23 +276,11 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo)
 
 					if (!fcinfo->argnull[i])
 					{
-						HeapTupleHeader td;
-						Oid			tupType;
-						int32		tupTypmod;
 						TupleDesc	tupdesc;
-						HeapTupleData tmptup;
-
-						td = DatumGetHeapTupleHeader(fcinfo->arg[i]);
-						/* Extract rowtype info and find a tupdesc */
-						tupType = HeapTupleHeaderGetTypeId(td);
-						tupTypmod = HeapTupleHeaderGetTypMod(td);
-						tupdesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
-						/* Build a temporary HeapTuple control structure */
-						tmptup.t_len = HeapTupleHeaderGetDatumLength(td);
-						ItemPointerSetInvalid(&(tmptup.t_self));
-						tmptup.t_tableOid = InvalidOid;
-						tmptup.t_data = td;
-						exec_move_row(&estate, NULL, row, &tmptup, tupdesc);
+						HeapTuple	tuple;
+
+						tuple = get_tuple_from_datum(fcinfo->arg[i], &tupdesc);
+						exec_move_row(&estate, NULL, row, tuple, tupdesc);
 						ReleaseTupleDesc(tupdesc);
 					}
 					else
@@ -2449,24 +2438,30 @@ exec_stmt_return(PLpgSQL_execstate *estate, PLpgSQL_stmt_return *stmt)
 
 	if (stmt->expr != NULL)
 	{
+		estate->retval = exec_eval_expr(estate, stmt->expr,
+										&(estate->retisnull),
+										&(estate->rettype));
 		if (estate->retistuple)
 		{
-			exec_run_select(estate, stmt->expr, 1, NULL);
-			if (estate->eval_processed > 0)
+			/* Source must be of RECORD or composite type */
+			if (!type_is_rowtype(estate->rettype))
+				ereport(ERROR,
+						(errcode(ERRCODE_DATATYPE_MISMATCH),
+						errmsg("cannot return non-composite value from composite type returning function")));
+
+			if (!estate->retisnull)
 			{
-				estate->retval = PointerGetDatum(estate->eval_tuptable->vals[0]);
-				estate->rettupdesc = estate->eval_tuptable->tupdesc;
-				estate->retisnull = false;
+				HeapTuple	tuple;
+				TupleDesc	tupdesc;
+
+				tuple = get_tuple_from_datum(estate->retval, &tupdesc);
+				estate->retval = PointerGetDatum(tuple);
+				estate->rettupdesc = CreateTupleDescCopy(tupdesc);
+				ReleaseTupleDesc(tupdesc);
 			}
 		}
-		else
-		{
-			/* Normal case for scalar results */
-			estate->retval = exec_eval_expr(estate, stmt->expr,
-											&(estate->retisnull),
-											&(estate->rettype));
-		}
 
+		/* Else, the expr is of scalar type and has been evaluated. simply return. */
 		return PLPGSQL_RC_RETURN;
 	}
 
@@ -2593,26 +2588,51 @@ exec_stmt_return_next(PLpgSQL_execstate *estate,
 		bool		isNull;
 		Oid			rettype;
 
-		if (natts != 1)
-			ereport(ERROR,
-					(errcode(ERRCODE_DATATYPE_MISMATCH),
-					 errmsg("wrong result type supplied in RETURN NEXT")));
-
 		retval = exec_eval_expr(estate,
 								stmt->expr,
 								&isNull,
 								&rettype);
 
-		/* coerce type if needed */
-		retval = exec_simple_cast_value(estate,
-										retval,
-										rettype,
-										tupdesc->attrs[0]->atttypid,
-										tupdesc->attrs[0]->atttypmod,
-										isNull);
+		/* Check if expr is of RECORD or composite type */
+		if (type_is_rowtype(rettype))
+		{
+			TupleConversionMap *tupmap;
+			TupleDesc retdesc;
+
+			tuple = get_tuple_from_datum(retval, &retdesc);
+			tupmap = convert_tuples_by_position(retdesc,
+												estate->rettupdesc,
+												gettext_noop("returned record type does not match expected record type"));
+			/* it might need conversion */
+			if (tupmap)
+			{
+				tuple = do_convert_tuple(tuple, tupmap);
+				free_conversion_map(tupmap);
+				free_tuple = true;
+			}
+
+			ReleaseTupleDesc(retdesc);
+		}
+		else
+		{
+			/* Normal case for scalar results */
+
+			if (natts != 1)
+				ereport(ERROR,
+						(errcode(ERRCODE_DATATYPE_MISMATCH),
+						errmsg("wrong result type supplied in RETURN NEXT")));
+
+			/* coerce type if needed */
+			retval = exec_simple_cast_value(estate,
+											retval,
+											rettype,
+											tupdesc->attrs[0]->atttypid,
+											tupdesc->attrs[0]->atttypmod,
+											isNull);
 
-		tuplestore_putvalues(estate->tuple_store, tupdesc,
-							 &retval, &isNull);
+			tuplestore_putvalues(estate->tuple_store, tupdesc,
+								&retval, &isNull);
+		}
 	}
 	else
 	{
@@ -3901,29 +3921,16 @@ exec_assign_value(PLpgSQL_execstate *estate,
 				}
 				else
 				{
-					HeapTupleHeader td;
-					Oid			tupType;
-					int32		tupTypmod;
 					TupleDesc	tupdesc;
-					HeapTupleData tmptup;
+					HeapTuple	tuple;
 
 					/* Source must be of RECORD or composite type */
 					if (!type_is_rowtype(valtype))
 						ereport(ERROR,
 								(errcode(ERRCODE_DATATYPE_MISMATCH),
 								 errmsg("cannot assign non-composite value to a row variable")));
-					/* Source is a tuple Datum, so safe to do this: */
-					td = DatumGetHeapTupleHeader(value);
-					/* Extract rowtype info and find a tupdesc */
-					tupType = HeapTupleHeaderGetTypeId(td);
-					tupTypmod = HeapTupleHeaderGetTypMod(td);
-					tupdesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
-					/* Build a temporary HeapTuple control structure */
-					tmptup.t_len = HeapTupleHeaderGetDatumLength(td);
-					ItemPointerSetInvalid(&(tmptup.t_self));
-					tmptup.t_tableOid = InvalidOid;
-					tmptup.t_data = td;
-					exec_move_row(estate, NULL, row, &tmptup, tupdesc);
+					tuple = get_tuple_from_datum(value, &tupdesc);
+					exec_move_row(estate, NULL, row, tuple, tupdesc);
 					ReleaseTupleDesc(tupdesc);
 				}
 				break;
@@ -3943,11 +3950,8 @@ exec_assign_value(PLpgSQL_execstate *estate,
 				}
 				else
 				{
-					HeapTupleHeader td;
-					Oid			tupType;
-					int32		tupTypmod;
 					TupleDesc	tupdesc;
-					HeapTupleData tmptup;
+					HeapTuple	tuple;
 
 					/* Source must be of RECORD or composite type */
 					if (!type_is_rowtype(valtype))
@@ -3955,18 +3959,8 @@ exec_assign_value(PLpgSQL_execstate *estate,
 								(errcode(ERRCODE_DATATYPE_MISMATCH),
 								 errmsg("cannot assign non-composite value to a record variable")));
 
-					/* Source is a tuple Datum, so safe to do this: */
-					td = DatumGetHeapTupleHeader(value);
-					/* Extract rowtype info and find a tupdesc */
-					tupType = HeapTupleHeaderGetTypeId(td);
-					tupTypmod = HeapTupleHeaderGetTypMod(td);
-					tupdesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
-					/* Build a temporary HeapTuple control structure */
-					tmptup.t_len = HeapTupleHeaderGetDatumLength(td);
-					ItemPointerSetInvalid(&(tmptup.t_self));
-					tmptup.t_tableOid = InvalidOid;
-					tmptup.t_data = td;
-					exec_move_row(estate, rec, NULL, &tmptup, tupdesc);
+					tuple = get_tuple_from_datum(value, &tupdesc);
+					exec_move_row(estate, rec, NULL, tuple, tupdesc);
 					ReleaseTupleDesc(tupdesc);
 				}
 				break;
@@ -6280,3 +6274,38 @@ exec_dynquery_with_params(PLpgSQL_execstate *estate,
 
 	return portal;
 }
+
+/* ----------
+ * get_tuple_from_datum		get a tuple from the rowtype datum
+ *
+ * If rettupdesc isn't NULL, it will receive a pointer to TupleDesc
+ * of rowtype.
+ *
+ * Note: its caller's responsibility to check 'value' is rowtype datum.
+ * ----------
+ */
+static HeapTuple
+get_tuple_from_datum(Datum value, TupleDesc *rettupdesc)
+{
+	HeapTupleHeader td;
+	Oid			tupType;
+	int32		tupTypmod;
+	HeapTupleData tmptup;
+
+	td = DatumGetHeapTupleHeader(value);
+	/* Extract rowtype info and find a tupdesc */
+	if (rettupdesc)
+	{
+		tupType = HeapTupleHeaderGetTypeId(td);
+		tupTypmod = HeapTupleHeaderGetTypMod(td);
+		*rettupdesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
+	}
+
+	/* Build a temporary HeapTuple control structure */
+	tmptup.t_len = HeapTupleHeaderGetDatumLength(td);
+	ItemPointerSetInvalid(&(tmptup.t_self));
+	tmptup.t_tableOid = InvalidOid;
+	tmptup.t_data = td;
+
+	return heap_copytuple(&tmptup);
+}
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index cf164d0..c3b59e8 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -2926,7 +2926,8 @@ make_return_stmt(int location)
 	}
 	else if (plpgsql_curr_compile->fn_retistuple)
 	{
-		switch (yylex())
+		int tok = yylex();
+		switch (tok)
 		{
 			case K_NULL:
 				/* we allow this to support RETURN NULL in triggers */
@@ -2944,10 +2945,13 @@ make_return_stmt(int location)
 				break;
 
 			default:
-				ereport(ERROR,
-						(errcode(ERRCODE_DATATYPE_MISMATCH),
-						 errmsg("RETURN must specify a record or row variable in function returning row"),
-						 parser_errposition(yylloc)));
+				/*
+				 * Allow use of expression in return statement for functions returning
+				 * row types.
+				 */
+				plpgsql_push_back_token(tok);
+				new->expr = read_sql_expression(';', ";");
+				return (PLpgSQL_stmt *) new;
 				break;
 		}
 		if (yylex() != ';')
@@ -2994,7 +2998,8 @@ make_return_next_stmt(int location)
 	}
 	else if (plpgsql_curr_compile->fn_retistuple)
 	{
-		switch (yylex())
+		int tok = yylex();
+		switch (tok)
 		{
 			case T_DATUM:
 				if (yylval.wdatum.datum->dtype == PLPGSQL_DTYPE_ROW ||
@@ -3008,10 +3013,13 @@ make_return_next_stmt(int location)
 				break;
 
 			default:
-				ereport(ERROR,
-						(errcode(ERRCODE_DATATYPE_MISMATCH),
-						 errmsg("RETURN NEXT must specify a record or row variable in function returning row"),
-						 parser_errposition(yylloc)));
+				/*
+				 * Allow use of expression in return statement for functions returning
+				 * row types.
+				 */
+				plpgsql_push_back_token(tok);
+				new->expr = read_sql_expression(';', ";");
+				return (PLpgSQL_stmt *) new;
 				break;
 		}
 		if (yylex() != ';')
diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule
index be789e3..7ea2a26 100644
--- a/src/test/regress/serial_schedule
+++ b/src/test/regress/serial_schedule
@@ -134,3 +134,4 @@ test: largeobject
 test: with
 test: xml
 test: stats
+test: return_rowtype
diff --git a/src/test/regress/expected/return_rowtype.out b/src/test/regress/expected/return_rowtype.out
new file mode 100644
index 0000000..b008538
--- /dev/null
+++ b/src/test/regress/expected/return_rowtype.out
@@ -0,0 +1,135 @@
+--create an composite type
+create type footype as (x int, y varchar);
+--test: use of variable of composite type in return statement
+create or replace function foo() returns footype as $$
+declare
+  v footype;
+begin
+  v := (1, 'hello');
+  return v;
+end;
+$$ language plpgsql;
+select foo();
+    foo    
+-----------
+ (1,hello)
+(1 row)
+
+--test: use row expr in return statement
+create or replace function foo() returns footype as $$
+begin
+  return (1, 'hello'::varchar);
+end;
+$$ language plpgsql;
+select foo();
+    foo    
+-----------
+ (1,hello)
+(1 row)
+
+DO $$
+declare
+  v footype;
+begin
+  v := foo();
+  raise info 'x = %', v.x;
+  raise info 'y = %', v.y;
+end;
+$$;
+INFO:  x = 1
+INFO:  y = hello
+drop function foo();
+--create a table
+create table footab(x int, y varchar(10));
+--test: return a row expr
+create or replace function foorec() returns footab as $$
+begin
+  return (1, 'hello'::varchar(10));
+end;
+$$ language plpgsql;
+DO $$
+declare
+  v footab%rowtype;
+begin
+  v := foorec();
+  raise info 'x = %', v.x;
+  raise info 'y = %', v.y;
+end; $$;
+INFO:  x = 1
+INFO:  y = hello
+drop function foorec();
+drop table footab;
+--test: return a row expr as record, (ensure record behavior is not changed)
+create or replace function foorec() returns record as $$
+declare
+  v record;
+begin
+  v := (1, 'hello');
+  return v;
+end;
+$$ language plpgsql;
+select foorec();
+  foorec   
+-----------
+ (1,hello)
+(1 row)
+
+DO $$
+declare
+  v record;
+begin
+  v := foorec();
+  raise info 'rec = %', v;
+end; $$;
+INFO:  rec = (1,hello)
+--test: return row expr in return statement.
+create or replace function foorec() returns record as $$
+begin
+  return (1, 'hello');
+end;
+$$ language plpgsql;
+select foorec();
+  foorec   
+-----------
+ (1,hello)
+(1 row)
+
+DO $$
+declare
+  v record;
+begin
+  v := foorec();
+  raise info 'rec = %', v;
+end; $$;
+INFO:  rec = (1,hello)
+drop function foorec();
+--test: row expr in RETURN NEXT statement.
+create or replace function foo() returns setof footype as $$
+begin
+  for i in 1..10
+  loop
+    return next (1, 'hello');
+  end loop;
+  return;
+end;
+$$ language plpgsql;
+drop function foo();
+--test: use invalid expr in return statement.
+create or replace function foo() returns footype as $$
+begin
+  return 1 + 1;
+end;
+$$ language plpgsql;
+DO $$
+declare
+  v footype;
+begin
+  v := foo();
+  raise info 'x = %', v.x;
+  raise info 'y = %', v.y;
+end; $$;
+ERROR:  cannot return non-composite value from composite type returning function
+CONTEXT:  PL/pgSQL function foo() line 3 at RETURN
+PL/pgSQL function inline_code_block line 5 at assignment
+drop function foo();
+drop type footype;
diff --git a/src/test/regress/sql/return_rowtype.sql b/src/test/regress/sql/return_rowtype.sql
new file mode 100644
index 0000000..79a6553
--- /dev/null
+++ b/src/test/regress/sql/return_rowtype.sql
@@ -0,0 +1,128 @@
+--create an composite type
+create type footype as (x int, y varchar);
+
+--test: use of variable of composite type in return statement
+create or replace function foo() returns footype as $$
+declare
+  v footype;
+begin
+  v := (1, 'hello');
+  return v;
+end;
+$$ language plpgsql;
+
+select foo();
+
+--test: use row expr in return statement
+create or replace function foo() returns footype as $$
+begin
+  return (1, 'hello'::varchar);
+end;
+$$ language plpgsql;
+
+select foo();
+
+DO $$
+declare
+  v footype;
+begin
+  v := foo();
+  raise info 'x = %', v.x;
+  raise info 'y = %', v.y;
+end;
+$$;
+
+drop function foo();
+
+--create a table
+create table footab(x int, y varchar(10));
+
+--test: return a row expr
+create or replace function foorec() returns footab as $$
+begin
+  return (1, 'hello'::varchar(10));
+end;
+$$ language plpgsql;
+
+DO $$
+declare
+  v footab%rowtype;
+begin
+  v := foorec();
+  raise info 'x = %', v.x;
+  raise info 'y = %', v.y;
+end; $$;
+
+drop function foorec();
+drop table footab;
+
+--test: return a row expr as record, (ensure record behavior is not changed)
+create or replace function foorec() returns record as $$
+declare
+  v record;
+begin
+  v := (1, 'hello');
+  return v;
+end;
+$$ language plpgsql;
+
+select foorec();
+
+DO $$
+declare
+  v record;
+begin
+  v := foorec();
+  raise info 'rec = %', v;
+end; $$;
+
+--test: return row expr in return statement.
+create or replace function foorec() returns record as $$
+begin
+  return (1, 'hello');
+end;
+$$ language plpgsql;
+
+select foorec();
+
+DO $$
+declare
+  v record;
+begin
+  v := foorec();
+  raise info 'rec = %', v;
+end; $$;
+
+drop function foorec();
+
+--test: row expr in RETURN NEXT statement.
+create or replace function foo() returns setof footype as $$
+begin
+  for i in 1..10
+  loop
+    return next (1, 'hello');
+  end loop;
+  return;
+end;
+$$ language plpgsql;
+
+drop function foo();
+
+--test: use invalid expr in return statement.
+create or replace function foo() returns footype as $$
+begin
+  return 1 + 1;
+end;
+$$ language plpgsql;
+
+DO $$
+declare
+  v footype;
+begin
+  v := foo();
+  raise info 'x = %', v.x;
+  raise info 'y = %', v.y;
+end; $$;
+
+drop function foo();
+drop type footype;
#3Pavel Stehule
pavel.stehule@gmail.com
In reply to: Asif Rehman (#2)

Hello

I fully agree with Asif's arguments

previous tupconvert implementation was really strict - so using
enhanced tupconver has very minimal impact for old user - and I
checked same speed for plpgsql function - patched and unpatched pg.

tested

CREATE OR REPLACE FUNCTION public.foo(i integer)
RETURNS SETOF record
LANGUAGE plpgsql
AS $function$
declare r record;
begin
r := (10.1,20.1); for i in 1..10 loop return next r; end loop; return;
end;
$function$

select sum(a) from generate_series(1,3000) g(i), lateral foo(i) as (a
numeric, b numeric);

More - everywhere where we use tupconvert internally, then we use
cached conversion map - so I am sure, so speed of ANALYZE cannot be
impacted by this patch

There are other two issue:

it allow to write new differnt slow code - IO cast is about 5-10-20%
slower, and with this path anybody has new possibilities for new "bad"
code. But it is not a problem of this patch. It is related to plpgsql
design and probably we should to move some conversions to outside
plpgsql to be possible reuse conversion map and enhance plpgsql. I
have a simple half solutions - plpgsql_check_function can detect this
situation in almost typical use cases and can raises a performance
warning. So this issue is not stopper for me - because it is not new
issue in plpgsql.

Second issue is more significant:

there are bug:

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral
foo(i) as (a numeric, b numeric);
sum
----------
303000.0
(1 row)

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral
foo(i) as (a float, b numeric);
sum
-----------------------
7.33675699577682e-232
(1 row)

it produces wrong result

And with minimal change it kill session

create or replace function foo(i integer)
returns setof record as $$
declare r record;
begin
r := (10,20); for i in 1..10 loop return next r; end loop; return;
end;
$$ language plpgsql;

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral
foo(i) as (a numeric, b numeric);
The connection to the server was lost. Attempting reset: Failed.

create or replace function fo(i integer)
returns record as $$
declare r record;
begin
r := (10,20); return r;
end;
$$ language plpgsql;

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral
fo(i) as (a int, b numeric);
sum
-------
30000
(1 row)

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral
fo(i) as (a numeric, b numeric);
The connection to the server was lost. Attempting reset: Failed.

Regards

Pavel Stehule

2012/12/3 Asif Rehman <asifr.rehman@gmail.com>:

Hi,

Thanks for the review. Please see the updated patch.

Hmm. We're running an I/O cast during do_tup_convert() now, and look
up the required functions for each tuple. I think if we're going to
support this operation at that level, we need to look up the necessary
functions at convert_tuples_by_foo level, and then apply unconditionally
if they've been set up.

Done. TupleConversionMap struct should keep the array of functions oid's
that
needs to be applied. Though only for those cases where both attribute type
id's
do not match. This way no unnecessary casting will happen.

Also, what are the implicancies for existing users of tupconvert? Do we
want to apply casting during ANALYZE for example? AFAICS the patch
shouldn't break any case that works today, but I don't see that there
has been any analysis of this.

I believe this part of the code should not impact existing users of
tupconvert.
As this part of code is relaxing a restriction upon which an error would
have been
generated. Though it could have a little impact on performance but should be
only for
cases where attribute type id's are different and are implicitly cast able.

Besides existing users involve tupconvert in case of inheritance. And in
that case
an exact match is expected.

Regards,
--Asif

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Asif Rehman
asifr.rehman@gmail.com
In reply to: Pavel Stehule (#3)
1 attachment(s)

Hi,

Here is the updated patch. I overlooked the loop, checking to free the
conversions map. Here are the results now.

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral foo(i)
as (a numeric, b numeric);
sum
----------
303000.0
(1 row)

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral foo(i)
as (a float, b numeric);
sum
------------------
303000.000000012

Regards,
--Asif

On Wed, Dec 5, 2012 at 12:40 AM, Pavel Stehule <pavel.stehule@gmail.com>wrote:

Show quoted text

Hello

I fully agree with Asif's arguments

previous tupconvert implementation was really strict - so using
enhanced tupconver has very minimal impact for old user - and I
checked same speed for plpgsql function - patched and unpatched pg.

tested

CREATE OR REPLACE FUNCTION public.foo(i integer)
RETURNS SETOF record
LANGUAGE plpgsql
AS $function$
declare r record;
begin
r := (10.1,20.1); for i in 1..10 loop return next r; end loop; return;
end;
$function$

select sum(a) from generate_series(1,3000) g(i), lateral foo(i) as (a
numeric, b numeric);

More - everywhere where we use tupconvert internally, then we use
cached conversion map - so I am sure, so speed of ANALYZE cannot be
impacted by this patch

There are other two issue:

it allow to write new differnt slow code - IO cast is about 5-10-20%
slower, and with this path anybody has new possibilities for new "bad"
code. But it is not a problem of this patch. It is related to plpgsql
design and probably we should to move some conversions to outside
plpgsql to be possible reuse conversion map and enhance plpgsql. I
have a simple half solutions - plpgsql_check_function can detect this
situation in almost typical use cases and can raises a performance
warning. So this issue is not stopper for me - because it is not new
issue in plpgsql.

Second issue is more significant:

there are bug:

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral
foo(i) as (a numeric, b numeric);
sum
----------
303000.0
(1 row)

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral
foo(i) as (a float, b numeric);
sum
-----------------------
7.33675699577682e-232
(1 row)

it produces wrong result

And with minimal change it kill session

create or replace function foo(i integer)
returns setof record as $$
declare r record;
begin
r := (10,20); for i in 1..10 loop return next r; end loop; return;
end;
$$ language plpgsql;

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral
foo(i) as (a numeric, b numeric);
The connection to the server was lost. Attempting reset: Failed.

create or replace function fo(i integer)
returns record as $$
declare r record;
begin
r := (10,20); return r;
end;
$$ language plpgsql;

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral
fo(i) as (a int, b numeric);
sum
-------
30000
(1 row)

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral
fo(i) as (a numeric, b numeric);
The connection to the server was lost. Attempting reset: Failed.

Regards

Pavel Stehule

2012/12/3 Asif Rehman <asifr.rehman@gmail.com>:

Hi,

Thanks for the review. Please see the updated patch.

Hmm. We're running an I/O cast during do_tup_convert() now, and look
up the required functions for each tuple. I think if we're going to
support this operation at that level, we need to look up the necessary
functions at convert_tuples_by_foo level, and then apply unconditionally
if they've been set up.

Done. TupleConversionMap struct should keep the array of functions oid's
that
needs to be applied. Though only for those cases where both attribute

type

id's
do not match. This way no unnecessary casting will happen.

Also, what are the implicancies for existing users of tupconvert? Do we
want to apply casting during ANALYZE for example? AFAICS the patch
shouldn't break any case that works today, but I don't see that there
has been any analysis of this.

I believe this part of the code should not impact existing users of
tupconvert.
As this part of code is relaxing a restriction upon which an error would
have been
generated. Though it could have a little impact on performance but

should be

only for
cases where attribute type id's are different and are implicitly cast

able.

Besides existing users involve tupconvert in case of inheritance. And in
that case
an exact match is expected.

Regards,
--Asif

Attachments:

return_rowtype.v3.patchapplication/octet-stream; name=return_rowtype.v3.patchDownload
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 07fba57..58b1489 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -1571,11 +1571,9 @@ RETURN <replaceable>expression</replaceable>;
      </para>
 
      <para>
-      When returning a scalar type, any expression can be used. The
-      expression's result will be automatically cast into the
-      function's return type as described for assignments. To return a
-      composite (row) value, you must write a record or row variable
-      as the <replaceable>expression</replaceable>.
+      In <command>RETURN</command> statement you can use any expression.
+       The expression's result will be automatically cast into the
+      function's return type as described for assignments.
      </para>
 
      <para>
@@ -1600,6 +1598,21 @@ RETURN <replaceable>expression</replaceable>;
       however.  In those cases a <command>RETURN</command> statement is
       automatically executed if the top-level block finishes.
      </para>
+
+     <para>
+      Some examples:
+
+<programlisting>
+-- functions returning a scalar type
+RETURN 1 + 2;
+RETURN scalar_var;
+
+-- functions returning a composite type
+RETURN composite_type_var;
+RETURN (1, 2, 'three');
+</programlisting>
+
+     </para>
     </sect3>
 
     <sect3>
diff --git a/src/backend/access/common/tupconvert.c b/src/backend/access/common/tupconvert.c
index f813432..6a9cc64 100644
--- a/src/backend/access/common/tupconvert.c
+++ b/src/backend/access/common/tupconvert.c
@@ -22,7 +22,9 @@
 
 #include "access/htup_details.h"
 #include "access/tupconvert.h"
+#include "parser/parse_coerce.h"
 #include "utils/builtins.h"
+#include "utils/lsyscache.h"
 
 
 /*
@@ -69,6 +71,9 @@ convert_tuples_by_position(TupleDesc indesc,
 {
 	TupleConversionMap *map;
 	AttrNumber *attrMap;
+	Oid		   *typoutput;
+	Oid		   *typinput;
+	Oid		   *typioparam;
 	int			nincols;
 	int			noutcols;
 	int			n;
@@ -78,7 +83,10 @@ convert_tuples_by_position(TupleDesc indesc,
 
 	/* Verify compatibility and prepare attribute-number map */
 	n = outdesc->natts;
-	attrMap = (AttrNumber *) palloc0(n * sizeof(AttrNumber));
+	attrMap 	= (AttrNumber *) palloc0(n * sizeof(AttrNumber));
+	typinput 	= (Oid *) palloc0(n * sizeof(Oid));
+	typioparam	= (Oid *) palloc(n * sizeof(Oid));
+	typoutput 	= (Oid *) palloc((indesc->natts + 1) * sizeof(Oid)); /* +1 for NULL */
 	j = 0;						/* j is next physical input attribute */
 	nincols = noutcols = 0;		/* these count non-dropped attributes */
 	same = true;
@@ -100,8 +108,8 @@ convert_tuples_by_position(TupleDesc indesc,
 				continue;
 			nincols++;
 			/* Found matching column, check type */
-			if (atttypid != att->atttypid ||
-				(atttypmod != att->atttypmod && atttypmod >= 0))
+			if ((atttypmod != att->atttypmod && atttypmod >= 0) ||
+				!can_coerce_type(1, &atttypid, &(att->atttypid), COERCE_IMPLICIT_CAST))
 				ereport(ERROR,
 						(errcode(ERRCODE_DATATYPE_MISMATCH),
 						 errmsg_internal("%s", _(msg)),
@@ -111,6 +119,17 @@ convert_tuples_by_position(TupleDesc indesc,
 								   format_type_with_typemod(atttypid,
 															atttypmod),
 								   noutcols)));
+			/*
+			 * get the needed I/O functions to perform cast later in
+			 * do_convert_tuple, no need if type id's are same.
+			 */
+			if (atttypid != att->atttypid)
+			{
+				bool typIsVarlena;
+
+				getTypeOutputInfo(att->atttypid, &typoutput[j + 1], &typIsVarlena);
+				getTypeInputInfo(atttypid, &typinput[i], &typioparam[i]);
+			}
 			attrMap[i] = (AttrNumber) (j + 1);
 			j++;
 			break;
@@ -147,6 +166,13 @@ convert_tuples_by_position(TupleDesc indesc,
 	{
 		for (i = 0; i < n; i++)
 		{
+			/* If typinput is set, it also means we need to convert */
+			if (OidIsValid(typinput[i]))
+			{
+				same = false;
+				break;	
+			}
+
 			if (attrMap[i] == (i + 1))
 				continue;
 
@@ -183,9 +209,12 @@ convert_tuples_by_position(TupleDesc indesc,
 	/* preallocate workspace for Datum arrays */
 	map->outvalues = (Datum *) palloc(n * sizeof(Datum));
 	map->outisnull = (bool *) palloc(n * sizeof(bool));
+	map->typinput = typinput;
+	map->typioparam = typioparam;
 	n = indesc->natts + 1;		/* +1 for NULL */
 	map->invalues = (Datum *) palloc(n * sizeof(Datum));
 	map->inisnull = (bool *) palloc(n * sizeof(bool));
+	map->typoutput = typoutput;
 	map->invalues[0] = (Datum) 0;		/* set up the NULL entry */
 	map->inisnull[0] = true;
 
@@ -206,6 +235,9 @@ convert_tuples_by_name(TupleDesc indesc,
 {
 	TupleConversionMap *map;
 	AttrNumber *attrMap;
+	Oid		   *typoutput;
+	Oid		   *typinput;
+	Oid		   *typioparam;
 	int			n;
 	int			i;
 	bool		same;
@@ -213,6 +245,9 @@ convert_tuples_by_name(TupleDesc indesc,
 	/* Verify compatibility and prepare attribute-number map */
 	n = outdesc->natts;
 	attrMap = (AttrNumber *) palloc0(n * sizeof(AttrNumber));
+	typinput 	= (Oid *) palloc0(n * sizeof(Oid));
+	typioparam	= (Oid *) palloc(n * sizeof(Oid));
+	typoutput 	= (Oid *) palloc((indesc->natts + 1) * sizeof(Oid)); /* +1 for NULL */
 	for (i = 0; i < n; i++)
 	{
 		Form_pg_attribute att = outdesc->attrs[i];
@@ -234,7 +269,8 @@ convert_tuples_by_name(TupleDesc indesc,
 			if (strcmp(attname, NameStr(att->attname)) == 0)
 			{
 				/* Found it, check type */
-				if (atttypid != att->atttypid || atttypmod != att->atttypmod)
+				if (atttypmod != att->atttypmod ||
+					!can_coerce_type(1, &atttypid, &(att->atttypid), COERCE_IMPLICIT_CAST))
 					ereport(ERROR,
 							(errcode(ERRCODE_DATATYPE_MISMATCH),
 							 errmsg_internal("%s", _(msg)),
@@ -242,6 +278,17 @@ convert_tuples_by_name(TupleDesc indesc,
 									   attname,
 									   format_type_be(outdesc->tdtypeid),
 									   format_type_be(indesc->tdtypeid))));
+				/*
+				 * get the needed I/O functions to perform cast later in
+				 * do_convert_tuple, no need if type id's are same.
+				 */
+				if (atttypid != att->atttypid)
+				{
+					bool typIsVarlena;
+
+					getTypeOutputInfo(att->atttypid, &typoutput[j + 1], &typIsVarlena);
+					getTypeInputInfo(atttypid, &typinput[i], &typioparam[i]);
+				}
 				attrMap[i] = (AttrNumber) (j + 1);
 				break;
 			}
@@ -267,6 +314,13 @@ convert_tuples_by_name(TupleDesc indesc,
 		same = true;
 		for (i = 0; i < n; i++)
 		{
+			/* If typinput is set, it also means we need to convert */
+			if (OidIsValid(typinput[i]))
+			{
+				same = false;
+				break;	
+			}
+
 			if (attrMap[i] == (i + 1))
 				continue;
 
@@ -303,9 +357,12 @@ convert_tuples_by_name(TupleDesc indesc,
 	/* preallocate workspace for Datum arrays */
 	map->outvalues = (Datum *) palloc(n * sizeof(Datum));
 	map->outisnull = (bool *) palloc(n * sizeof(bool));
+	map->typinput = typinput;
+	map->typioparam = typioparam;
 	n = indesc->natts + 1;		/* +1 for NULL */
 	map->invalues = (Datum *) palloc(n * sizeof(Datum));
 	map->inisnull = (bool *) palloc(n * sizeof(bool));
+	map->typoutput = typoutput;
 	map->invalues[0] = (Datum) 0;		/* set up the NULL entry */
 	map->inisnull[0] = true;
 
@@ -342,6 +399,21 @@ do_convert_tuple(HeapTuple tuple, TupleConversionMap *map)
 
 		outvalues[i] = invalues[j];
 		outisnull[i] = inisnull[j];
+
+		/* Perform the casting, if necessary. */
+		if (!inisnull[j] &&
+			OidIsValid(map->typinput[i]))
+		{
+			Form_pg_attribute att = map->outdesc->attrs[i];
+			char *strval;
+
+			strval = OidOutputFunctionCall(map->typoutput[j], invalues[j]);
+			outvalues[i] = OidInputFunctionCall(map->typinput[i],
+											 strval,
+											 map->typioparam[i],
+											 att->atttypmod);
+			pfree(strval);
+		}
 	}
 
 	/*
@@ -360,7 +432,10 @@ free_conversion_map(TupleConversionMap *map)
 	pfree(map->attrMap);
 	pfree(map->invalues);
 	pfree(map->inisnull);
+	pfree(map->typoutput);
 	pfree(map->outvalues);
 	pfree(map->outisnull);
+	pfree(map->typinput);
+	pfree(map->typioparam);
 	pfree(map);
 }
diff --git a/src/include/access/tupconvert.h b/src/include/access/tupconvert.h
index 342dbb4..61ef7ae 100644
--- a/src/include/access/tupconvert.h
+++ b/src/include/access/tupconvert.h
@@ -25,8 +25,11 @@ typedef struct TupleConversionMap
 	AttrNumber *attrMap;		/* indexes of input fields, or 0 for null */
 	Datum	   *invalues;		/* workspace for deconstructing source */
 	bool	   *inisnull;
+	Oid		   *typoutput;		/* output functions for source type */
 	Datum	   *outvalues;		/* workspace for constructing result */
 	bool	   *outisnull;
+	Oid		   *typinput;		/* input functions for result type */
+	Oid		   *typioparam;
 } TupleConversionMap;
 
 
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 3b5b3bb..d1c0acd 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -214,6 +214,7 @@ static void free_params_data(PreparedParamsData *ppd);
 static Portal exec_dynquery_with_params(PLpgSQL_execstate *estate,
 						  PLpgSQL_expr *dynquery, List *params,
 						  const char *portalname, int cursorOptions);
+static HeapTuple get_tuple_from_datum(Datum value, TupleDesc *rettupdesc);
 
 
 /* ----------
@@ -275,23 +276,11 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo)
 
 					if (!fcinfo->argnull[i])
 					{
-						HeapTupleHeader td;
-						Oid			tupType;
-						int32		tupTypmod;
 						TupleDesc	tupdesc;
-						HeapTupleData tmptup;
-
-						td = DatumGetHeapTupleHeader(fcinfo->arg[i]);
-						/* Extract rowtype info and find a tupdesc */
-						tupType = HeapTupleHeaderGetTypeId(td);
-						tupTypmod = HeapTupleHeaderGetTypMod(td);
-						tupdesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
-						/* Build a temporary HeapTuple control structure */
-						tmptup.t_len = HeapTupleHeaderGetDatumLength(td);
-						ItemPointerSetInvalid(&(tmptup.t_self));
-						tmptup.t_tableOid = InvalidOid;
-						tmptup.t_data = td;
-						exec_move_row(&estate, NULL, row, &tmptup, tupdesc);
+						HeapTuple	tuple;
+
+						tuple = get_tuple_from_datum(fcinfo->arg[i], &tupdesc);
+						exec_move_row(&estate, NULL, row, tuple, tupdesc);
 						ReleaseTupleDesc(tupdesc);
 					}
 					else
@@ -2449,24 +2438,30 @@ exec_stmt_return(PLpgSQL_execstate *estate, PLpgSQL_stmt_return *stmt)
 
 	if (stmt->expr != NULL)
 	{
+		estate->retval = exec_eval_expr(estate, stmt->expr,
+										&(estate->retisnull),
+										&(estate->rettype));
 		if (estate->retistuple)
 		{
-			exec_run_select(estate, stmt->expr, 1, NULL);
-			if (estate->eval_processed > 0)
+			/* Source must be of RECORD or composite type */
+			if (!type_is_rowtype(estate->rettype))
+				ereport(ERROR,
+						(errcode(ERRCODE_DATATYPE_MISMATCH),
+						errmsg("cannot return non-composite value from composite type returning function")));
+
+			if (!estate->retisnull)
 			{
-				estate->retval = PointerGetDatum(estate->eval_tuptable->vals[0]);
-				estate->rettupdesc = estate->eval_tuptable->tupdesc;
-				estate->retisnull = false;
+				HeapTuple	tuple;
+				TupleDesc	tupdesc;
+
+				tuple = get_tuple_from_datum(estate->retval, &tupdesc);
+				estate->retval = PointerGetDatum(tuple);
+				estate->rettupdesc = CreateTupleDescCopy(tupdesc);
+				ReleaseTupleDesc(tupdesc);
 			}
 		}
-		else
-		{
-			/* Normal case for scalar results */
-			estate->retval = exec_eval_expr(estate, stmt->expr,
-											&(estate->retisnull),
-											&(estate->rettype));
-		}
 
+		/* Else, the expr is of scalar type and has been evaluated. simply return. */
 		return PLPGSQL_RC_RETURN;
 	}
 
@@ -2593,26 +2588,51 @@ exec_stmt_return_next(PLpgSQL_execstate *estate,
 		bool		isNull;
 		Oid			rettype;
 
-		if (natts != 1)
-			ereport(ERROR,
-					(errcode(ERRCODE_DATATYPE_MISMATCH),
-					 errmsg("wrong result type supplied in RETURN NEXT")));
-
 		retval = exec_eval_expr(estate,
 								stmt->expr,
 								&isNull,
 								&rettype);
 
-		/* coerce type if needed */
-		retval = exec_simple_cast_value(estate,
-										retval,
-										rettype,
-										tupdesc->attrs[0]->atttypid,
-										tupdesc->attrs[0]->atttypmod,
-										isNull);
+		/* Check if expr is of RECORD or composite type */
+		if (type_is_rowtype(rettype))
+		{
+			TupleConversionMap *tupmap;
+			TupleDesc retdesc;
+
+			tuple = get_tuple_from_datum(retval, &retdesc);
+			tupmap = convert_tuples_by_position(retdesc,
+												estate->rettupdesc,
+												gettext_noop("returned record type does not match expected record type"));
+			/* it might need conversion */
+			if (tupmap)
+			{
+				tuple = do_convert_tuple(tuple, tupmap);
+				free_conversion_map(tupmap);
+				free_tuple = true;
+			}
+
+			ReleaseTupleDesc(retdesc);
+		}
+		else
+		{
+			/* Normal case for scalar results */
+
+			if (natts != 1)
+				ereport(ERROR,
+						(errcode(ERRCODE_DATATYPE_MISMATCH),
+						errmsg("wrong result type supplied in RETURN NEXT")));
+
+			/* coerce type if needed */
+			retval = exec_simple_cast_value(estate,
+											retval,
+											rettype,
+											tupdesc->attrs[0]->atttypid,
+											tupdesc->attrs[0]->atttypmod,
+											isNull);
 
-		tuplestore_putvalues(estate->tuple_store, tupdesc,
-							 &retval, &isNull);
+			tuplestore_putvalues(estate->tuple_store, tupdesc,
+								&retval, &isNull);
+		}
 	}
 	else
 	{
@@ -3901,29 +3921,16 @@ exec_assign_value(PLpgSQL_execstate *estate,
 				}
 				else
 				{
-					HeapTupleHeader td;
-					Oid			tupType;
-					int32		tupTypmod;
 					TupleDesc	tupdesc;
-					HeapTupleData tmptup;
+					HeapTuple	tuple;
 
 					/* Source must be of RECORD or composite type */
 					if (!type_is_rowtype(valtype))
 						ereport(ERROR,
 								(errcode(ERRCODE_DATATYPE_MISMATCH),
 								 errmsg("cannot assign non-composite value to a row variable")));
-					/* Source is a tuple Datum, so safe to do this: */
-					td = DatumGetHeapTupleHeader(value);
-					/* Extract rowtype info and find a tupdesc */
-					tupType = HeapTupleHeaderGetTypeId(td);
-					tupTypmod = HeapTupleHeaderGetTypMod(td);
-					tupdesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
-					/* Build a temporary HeapTuple control structure */
-					tmptup.t_len = HeapTupleHeaderGetDatumLength(td);
-					ItemPointerSetInvalid(&(tmptup.t_self));
-					tmptup.t_tableOid = InvalidOid;
-					tmptup.t_data = td;
-					exec_move_row(estate, NULL, row, &tmptup, tupdesc);
+					tuple = get_tuple_from_datum(value, &tupdesc);
+					exec_move_row(estate, NULL, row, tuple, tupdesc);
 					ReleaseTupleDesc(tupdesc);
 				}
 				break;
@@ -3943,11 +3950,8 @@ exec_assign_value(PLpgSQL_execstate *estate,
 				}
 				else
 				{
-					HeapTupleHeader td;
-					Oid			tupType;
-					int32		tupTypmod;
 					TupleDesc	tupdesc;
-					HeapTupleData tmptup;
+					HeapTuple	tuple;
 
 					/* Source must be of RECORD or composite type */
 					if (!type_is_rowtype(valtype))
@@ -3955,18 +3959,8 @@ exec_assign_value(PLpgSQL_execstate *estate,
 								(errcode(ERRCODE_DATATYPE_MISMATCH),
 								 errmsg("cannot assign non-composite value to a record variable")));
 
-					/* Source is a tuple Datum, so safe to do this: */
-					td = DatumGetHeapTupleHeader(value);
-					/* Extract rowtype info and find a tupdesc */
-					tupType = HeapTupleHeaderGetTypeId(td);
-					tupTypmod = HeapTupleHeaderGetTypMod(td);
-					tupdesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
-					/* Build a temporary HeapTuple control structure */
-					tmptup.t_len = HeapTupleHeaderGetDatumLength(td);
-					ItemPointerSetInvalid(&(tmptup.t_self));
-					tmptup.t_tableOid = InvalidOid;
-					tmptup.t_data = td;
-					exec_move_row(estate, rec, NULL, &tmptup, tupdesc);
+					tuple = get_tuple_from_datum(value, &tupdesc);
+					exec_move_row(estate, rec, NULL, tuple, tupdesc);
 					ReleaseTupleDesc(tupdesc);
 				}
 				break;
@@ -6280,3 +6274,38 @@ exec_dynquery_with_params(PLpgSQL_execstate *estate,
 
 	return portal;
 }
+
+/* ----------
+ * get_tuple_from_datum		get a tuple from the rowtype datum
+ *
+ * If rettupdesc isn't NULL, it will receive a pointer to TupleDesc
+ * of rowtype.
+ *
+ * Note: its caller's responsibility to check 'value' is rowtype datum.
+ * ----------
+ */
+static HeapTuple
+get_tuple_from_datum(Datum value, TupleDesc *rettupdesc)
+{
+	HeapTupleHeader td;
+	Oid			tupType;
+	int32		tupTypmod;
+	HeapTupleData tmptup;
+
+	td = DatumGetHeapTupleHeader(value);
+	/* Extract rowtype info and find a tupdesc */
+	if (rettupdesc)
+	{
+		tupType = HeapTupleHeaderGetTypeId(td);
+		tupTypmod = HeapTupleHeaderGetTypMod(td);
+		*rettupdesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
+	}
+
+	/* Build a temporary HeapTuple control structure */
+	tmptup.t_len = HeapTupleHeaderGetDatumLength(td);
+	ItemPointerSetInvalid(&(tmptup.t_self));
+	tmptup.t_tableOid = InvalidOid;
+	tmptup.t_data = td;
+
+	return heap_copytuple(&tmptup);
+}
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index cf164d0..c3b59e8 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -2926,7 +2926,8 @@ make_return_stmt(int location)
 	}
 	else if (plpgsql_curr_compile->fn_retistuple)
 	{
-		switch (yylex())
+		int tok = yylex();
+		switch (tok)
 		{
 			case K_NULL:
 				/* we allow this to support RETURN NULL in triggers */
@@ -2944,10 +2945,13 @@ make_return_stmt(int location)
 				break;
 
 			default:
-				ereport(ERROR,
-						(errcode(ERRCODE_DATATYPE_MISMATCH),
-						 errmsg("RETURN must specify a record or row variable in function returning row"),
-						 parser_errposition(yylloc)));
+				/*
+				 * Allow use of expression in return statement for functions returning
+				 * row types.
+				 */
+				plpgsql_push_back_token(tok);
+				new->expr = read_sql_expression(';', ";");
+				return (PLpgSQL_stmt *) new;
 				break;
 		}
 		if (yylex() != ';')
@@ -2994,7 +2998,8 @@ make_return_next_stmt(int location)
 	}
 	else if (plpgsql_curr_compile->fn_retistuple)
 	{
-		switch (yylex())
+		int tok = yylex();
+		switch (tok)
 		{
 			case T_DATUM:
 				if (yylval.wdatum.datum->dtype == PLPGSQL_DTYPE_ROW ||
@@ -3008,10 +3013,13 @@ make_return_next_stmt(int location)
 				break;
 
 			default:
-				ereport(ERROR,
-						(errcode(ERRCODE_DATATYPE_MISMATCH),
-						 errmsg("RETURN NEXT must specify a record or row variable in function returning row"),
-						 parser_errposition(yylloc)));
+				/*
+				 * Allow use of expression in return statement for functions returning
+				 * row types.
+				 */
+				plpgsql_push_back_token(tok);
+				new->expr = read_sql_expression(';', ";");
+				return (PLpgSQL_stmt *) new;
 				break;
 		}
 		if (yylex() != ';')
diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule
index be789e3..7ea2a26 100644
--- a/src/test/regress/serial_schedule
+++ b/src/test/regress/serial_schedule
@@ -134,3 +134,4 @@ test: largeobject
 test: with
 test: xml
 test: stats
+test: return_rowtype
diff --git a/src/test/regress/expected/return_rowtype.out b/src/test/regress/expected/return_rowtype.out
new file mode 100644
index 0000000..effd6ac
--- /dev/null
+++ b/src/test/regress/expected/return_rowtype.out
@@ -0,0 +1,167 @@
+--create an composite type
+create type footype as (x int, y varchar);
+--test: use of variable of composite type in return statement
+create or replace function foo() returns footype as $$
+declare
+  v footype;
+begin
+  v := (1, 'hello');
+  return v;
+end;
+$$ language plpgsql;
+select foo();
+    foo    
+-----------
+ (1,hello)
+(1 row)
+
+--test: use row expr in return statement
+create or replace function foo() returns footype as $$
+begin
+  return (1, 'hello'::varchar);
+end;
+$$ language plpgsql;
+select foo();
+    foo    
+-----------
+ (1,hello)
+(1 row)
+
+DO $$
+declare
+  v footype;
+begin
+  v := foo();
+  raise info 'x = %', v.x;
+  raise info 'y = %', v.y;
+end;
+$$;
+INFO:  x = 1
+INFO:  y = hello
+drop function foo();
+--create a table
+create table footab(x int, y varchar(10));
+--test: return a row expr
+create or replace function foorec() returns footab as $$
+begin
+  return (1, 'hello'::varchar(10));
+end;
+$$ language plpgsql;
+DO $$
+declare
+  v footab%rowtype;
+begin
+  v := foorec();
+  raise info 'x = %', v.x;
+  raise info 'y = %', v.y;
+end; $$;
+INFO:  x = 1
+INFO:  y = hello
+drop function foorec();
+drop table footab;
+--test: return a row expr as record, (ensure record behavior is not changed)
+create or replace function foorec() returns record as $$
+declare
+  v record;
+begin
+  v := (1, 'hello');
+  return v;
+end;
+$$ language plpgsql;
+select foorec();
+  foorec   
+-----------
+ (1,hello)
+(1 row)
+
+DO $$
+declare
+  v record;
+begin
+  v := foorec();
+  raise info 'rec = %', v;
+end; $$;
+INFO:  rec = (1,hello)
+--test: return row expr in return statement.
+create or replace function foorec() returns record as $$
+begin
+  return (1, 'hello');
+end;
+$$ language plpgsql;
+select foorec();
+  foorec   
+-----------
+ (1,hello)
+(1 row)
+
+DO $$
+declare
+  v record;
+begin
+  v := foorec();
+  raise info 'rec = %', v;
+end; $$;
+INFO:  rec = (1,hello)
+drop function foorec();
+--test: row expr in RETURN NEXT statement.
+create or replace function foo() returns setof footype as $$
+begin
+  for i in 1..10
+  loop
+    return next (1, 'hello');
+  end loop;
+  return;
+end;
+$$ language plpgsql;
+drop function foo();
+--test: use invalid expr in return statement.
+create or replace function foo() returns footype as $$
+begin
+  return 1 + 1;
+end;
+$$ language plpgsql;
+DO $$
+declare
+  v footype;
+begin
+  v := foo();
+  raise info 'x = %', v.x;
+  raise info 'y = %', v.y;
+end; $$;
+ERROR:  cannot return non-composite value from composite type returning function
+CONTEXT:  PL/pgSQL function foo() line 3 at RETURN
+PL/pgSQL function inline_code_block line 5 at assignment
+drop function foo();
+drop type footype;
+--test: return record of integers but receive as numeric
+create or replace function foo(i integer)
+returns setof record as $$
+declare r record;
+begin
+  r := (10,20);
+  for i in 1..10 loop
+    return next r;
+  end loop;
+  return;
+end;
+$$ language plpgsql;
+select sum(a) from generate_series(1,3000) g(i), lateral foo(i) as (a numeric, b numeric);
+  sum   
+--------
+ 300000
+(1 row)
+
+create or replace function fo(i integer)
+returns record as $$
+declare r record;
+begin
+  r := (10,20); return r;
+end;
+$$ language plpgsql;
+select sum(a) from generate_series(1,3000) g(i), lateral fo(i) as (a int, b numeric);
+  sum  
+-------
+ 30000
+(1 row)
+
+drop function foo(integer);
diff --git a/src/test/regress/sql/return_rowtype.sql b/src/test/regress/sql/return_rowtype.sql
new file mode 100644
index 0000000..7776262
--- /dev/null
+++ b/src/test/regress/sql/return_rowtype.sql
@@ -0,0 +1,155 @@
+--create an composite type
+create type footype as (x int, y varchar);
+
+--test: use of variable of composite type in return statement
+create or replace function foo() returns footype as $$
+declare
+  v footype;
+begin
+  v := (1, 'hello');
+  return v;
+end;
+$$ language plpgsql;
+
+select foo();
+
+--test: use row expr in return statement
+create or replace function foo() returns footype as $$
+begin
+  return (1, 'hello'::varchar);
+end;
+$$ language plpgsql;
+
+select foo();
+
+DO $$
+declare
+  v footype;
+begin
+  v := foo();
+  raise info 'x = %', v.x;
+  raise info 'y = %', v.y;
+end;
+$$;
+
+drop function foo();
+
+--create a table
+create table footab(x int, y varchar(10));
+
+--test: return a row expr
+create or replace function foorec() returns footab as $$
+begin
+  return (1, 'hello'::varchar(10));
+end;
+$$ language plpgsql;
+
+DO $$
+declare
+  v footab%rowtype;
+begin
+  v := foorec();
+  raise info 'x = %', v.x;
+  raise info 'y = %', v.y;
+end; $$;
+
+drop function foorec();
+drop table footab;
+
+--test: return a row expr as record, (ensure record behavior is not changed)
+create or replace function foorec() returns record as $$
+declare
+  v record;
+begin
+  v := (1, 'hello');
+  return v;
+end;
+$$ language plpgsql;
+
+select foorec();
+
+DO $$
+declare
+  v record;
+begin
+  v := foorec();
+  raise info 'rec = %', v;
+end; $$;
+
+--test: return row expr in return statement.
+create or replace function foorec() returns record as $$
+begin
+  return (1, 'hello');
+end;
+$$ language plpgsql;
+
+select foorec();
+
+DO $$
+declare
+  v record;
+begin
+  v := foorec();
+  raise info 'rec = %', v;
+end; $$;
+
+drop function foorec();
+
+--test: row expr in RETURN NEXT statement.
+create or replace function foo() returns setof footype as $$
+begin
+  for i in 1..10
+  loop
+    return next (1, 'hello');
+  end loop;
+  return;
+end;
+$$ language plpgsql;
+
+drop function foo();
+
+--test: use invalid expr in return statement.
+create or replace function foo() returns footype as $$
+begin
+  return 1 + 1;
+end;
+$$ language plpgsql;
+
+DO $$
+declare
+  v footype;
+begin
+  v := foo();
+  raise info 'x = %', v.x;
+  raise info 'y = %', v.y;
+end; $$;
+
+drop function foo();
+drop type footype;
+
+--test: return record of integers but receive as numeric
+create or replace function foo(i integer)
+returns setof record as $$
+declare r record;
+begin
+  r := (10,20);
+  for i in 1..10 loop
+    return next r;
+  end loop;
+  return;
+end;
+$$ language plpgsql;
+
+select sum(a) from generate_series(1,3000) g(i), lateral foo(i) as (a numeric, b numeric);
+
+create or replace function fo(i integer)
+returns record as $$
+declare r record;
+begin
+  r := (10,20); return r;
+end;
+$$ language plpgsql;
+
+select sum(a) from generate_series(1,3000) g(i), lateral fo(i) as (a int, b numeric);
+
+drop function foo(integer);
#5Pavel Stehule
pavel.stehule@gmail.com
In reply to: Asif Rehman (#4)

2012/12/4 Asif Rehman <asifr.rehman@gmail.com>:

Hi,

Here is the updated patch. I overlooked the loop, checking to free the
conversions map. Here are the results now.

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral foo(i)
as (a numeric, b numeric);
sum
----------
303000.0
(1 row)

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral foo(i)
as (a float, b numeric);
sum
------------------
303000.000000012

Regards,
--Asif

yes, it is fixed.

ok I have no objection

Regards

Pavel Stehule

On Wed, Dec 5, 2012 at 12:40 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Hello

I fully agree with Asif's arguments

previous tupconvert implementation was really strict - so using
enhanced tupconver has very minimal impact for old user - and I
checked same speed for plpgsql function - patched and unpatched pg.

tested

CREATE OR REPLACE FUNCTION public.foo(i integer)
RETURNS SETOF record
LANGUAGE plpgsql
AS $function$
declare r record;
begin
r := (10.1,20.1); for i in 1..10 loop return next r; end loop; return;
end;
$function$

select sum(a) from generate_series(1,3000) g(i), lateral foo(i) as (a
numeric, b numeric);

More - everywhere where we use tupconvert internally, then we use
cached conversion map - so I am sure, so speed of ANALYZE cannot be
impacted by this patch

There are other two issue:

it allow to write new differnt slow code - IO cast is about 5-10-20%
slower, and with this path anybody has new possibilities for new "bad"
code. But it is not a problem of this patch. It is related to plpgsql
design and probably we should to move some conversions to outside
plpgsql to be possible reuse conversion map and enhance plpgsql. I
have a simple half solutions - plpgsql_check_function can detect this
situation in almost typical use cases and can raises a performance
warning. So this issue is not stopper for me - because it is not new
issue in plpgsql.

Second issue is more significant:

there are bug:

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral
foo(i) as (a numeric, b numeric);
sum
----------
303000.0
(1 row)

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral
foo(i) as (a float, b numeric);
sum
-----------------------
7.33675699577682e-232
(1 row)

it produces wrong result

And with minimal change it kill session

create or replace function foo(i integer)
returns setof record as $$
declare r record;
begin
r := (10,20); for i in 1..10 loop return next r; end loop; return;
end;
$$ language plpgsql;

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral
foo(i) as (a numeric, b numeric);
The connection to the server was lost. Attempting reset: Failed.

create or replace function fo(i integer)
returns record as $$
declare r record;
begin
r := (10,20); return r;
end;
$$ language plpgsql;

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral
fo(i) as (a int, b numeric);
sum
-------
30000
(1 row)

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral
fo(i) as (a numeric, b numeric);
The connection to the server was lost. Attempting reset: Failed.

Regards

Pavel Stehule

2012/12/3 Asif Rehman <asifr.rehman@gmail.com>:

Hi,

Thanks for the review. Please see the updated patch.

Hmm. We're running an I/O cast during do_tup_convert() now, and look
up the required functions for each tuple. I think if we're going to
support this operation at that level, we need to look up the necessary
functions at convert_tuples_by_foo level, and then apply
unconditionally
if they've been set up.

Done. TupleConversionMap struct should keep the array of functions oid's
that
needs to be applied. Though only for those cases where both attribute
type
id's
do not match. This way no unnecessary casting will happen.

Also, what are the implicancies for existing users of tupconvert? Do
we
want to apply casting during ANALYZE for example? AFAICS the patch
shouldn't break any case that works today, but I don't see that there
has been any analysis of this.

I believe this part of the code should not impact existing users of
tupconvert.
As this part of code is relaxing a restriction upon which an error would
have been
generated. Though it could have a little impact on performance but
should be
only for
cases where attribute type id's are different and are implicitly cast
able.

Besides existing users involve tupconvert in case of inheritance. And in
that case
an exact match is expected.

Regards,
--Asif

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Asif Rehman (#4)

Asif Rehman <asifr.rehman@gmail.com> writes:

Here is the updated patch. I overlooked the loop, checking to free the
conversions map. Here are the results now.

I looked at this patch briefly. It seems to me to be completely
schizophrenic about the coercion rules. This bit:

-			if (atttypid != att->atttypid ||
-				(atttypmod != att->atttypmod && atttypmod >= 0))
+			if ((atttypmod != att->atttypmod && atttypmod >= 0) ||
+				!can_coerce_type(1, &atttypid, &(att->atttypid), COERCE_IMPLICIT_CAST))

says that we'll allow column types to differ if there is an implicit
cast from the source to the target (or at least I think that's what's
intended, although it's got the source and target backwards). Fine, but
then why don't we use the cast machinery to do the conversion? This
is taking plpgsql's cowboy let's-coerce-via-IO-no-matter-whether-that's-
right-or-not approach and sticking it into a core part of the system.
There's no guarantee at all that applying typoutput then typinput
will match the conversion semantics you get from an actual cast, and
in many practical cases such as int4 to int8 it'll be drastically less
efficient anyway. It would make more sense to do something similar to
coerce_record_to_complex(), that is modify the expression tree to
coerce the columns using the regular cast machinery.

Also, the typmod part of the test seems completely broken. For one
thing, comparing typmods isn't sane if the types themselves aren't
the same. And it's quite unclear to me why we'd want to have an
anything-goes policy for type coercion, or even an implicit-casts-only
policy, but then insist that the typmods match exactly. This coding
will allow varchar(42) to text, but not varchar(42) to varchar(43)
... where's the sense in that?

The patch also seems to go a great deal further than what was asked for
originally, or indeed is mentioned in the documentation patch, namely
fixing the restriction on RETURN to allow composite-typed expressions.
Specifically it's changing the code that accepts composite input
arguments. Do we actually want that? If so, shouldn't it be
documented?

I'm inclined to suggest that we drop all the coercion stuff and just
do what Robert actually asked for originally, which was the mere
ability to return a composite value as long as it matched the function's
result type. I'm not convinced that we want automatic implicit type
coercions here. In any case I'm very much against sticking such a thing
into general-purpose support code like tupconvert.c. That will create a
strong likelihood that plpgsql's poorly-designed coercion semantics will
leak into other aspects of the system.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Pavel Stehule
pavel.stehule@gmail.com
In reply to: Tom Lane (#6)

2012/12/5 Tom Lane <tgl@sss.pgh.pa.us>:

Asif Rehman <asifr.rehman@gmail.com> writes:

Here is the updated patch. I overlooked the loop, checking to free the
conversions map. Here are the results now.

I looked at this patch briefly. It seems to me to be completely
schizophrenic about the coercion rules. This bit:

-                       if (atttypid != att->atttypid ||
-                               (atttypmod != att->atttypmod && atttypmod >= 0))
+                       if ((atttypmod != att->atttypmod && atttypmod >= 0) ||
+                               !can_coerce_type(1, &atttypid, &(att->atttypid), COERCE_IMPLICIT_CAST))

says that we'll allow column types to differ if there is an implicit
cast from the source to the target (or at least I think that's what's
intended, although it's got the source and target backwards). Fine, but
then why don't we use the cast machinery to do the conversion? This
is taking plpgsql's cowboy let's-coerce-via-IO-no-matter-whether-that's-
right-or-not approach and sticking it into a core part of the system.
There's no guarantee at all that applying typoutput then typinput
will match the conversion semantics you get from an actual cast, and
in many practical cases such as int4 to int8 it'll be drastically less
efficient anyway. It would make more sense to do something similar to
coerce_record_to_complex(), that is modify the expression tree to
coerce the columns using the regular cast machinery.

Also, the typmod part of the test seems completely broken. For one
thing, comparing typmods isn't sane if the types themselves aren't
the same. And it's quite unclear to me why we'd want to have an
anything-goes policy for type coercion, or even an implicit-casts-only
policy, but then insist that the typmods match exactly. This coding
will allow varchar(42) to text, but not varchar(42) to varchar(43)
... where's the sense in that?

The patch also seems to go a great deal further than what was asked for
originally, or indeed is mentioned in the documentation patch, namely
fixing the restriction on RETURN to allow composite-typed expressions.
Specifically it's changing the code that accepts composite input
arguments. Do we actually want that? If so, shouldn't it be
documented?

I'm inclined to suggest that we drop all the coercion stuff and just
do what Robert actually asked for originally, which was the mere
ability to return a composite value as long as it matched the function's
result type. I'm not convinced that we want automatic implicit type
coercions here. In any case I'm very much against sticking such a thing
into general-purpose support code like tupconvert.c. That will create a
strong likelihood that plpgsql's poorly-designed coercion semantics will
leak into other aspects of the system.

I think so without some change of coercion this patch is not too
useful because very simply test fail

create type foo(a int, b text);

create or replace function foo_func()
returns foo as $$
begin
...
return (10, 'hello');

end;

but we can limit a implicit coercion in tupconvert via new parameter -
because we would to forward plpgsql behave just from this direction.
Then when this parameter - maybe "allowIOCoercion" will be false, then
tupconvert will have same behave like before.

Regards

Pavel

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8Robert Haas
robertmhaas@gmail.com
In reply to: Pavel Stehule (#7)

On Thu, Dec 6, 2012 at 12:31 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:

2012/12/5 Tom Lane <tgl@sss.pgh.pa.us>:

Asif Rehman <asifr.rehman@gmail.com> writes:

Here is the updated patch. I overlooked the loop, checking to free the
conversions map. Here are the results now.

I looked at this patch briefly. It seems to me to be completely
schizophrenic about the coercion rules. This bit:

-                       if (atttypid != att->atttypid ||
-                               (atttypmod != att->atttypmod && atttypmod >= 0))
+                       if ((atttypmod != att->atttypmod && atttypmod >= 0) ||
+                               !can_coerce_type(1, &atttypid, &(att->atttypid), COERCE_IMPLICIT_CAST))

says that we'll allow column types to differ if there is an implicit
cast from the source to the target (or at least I think that's what's
intended, although it's got the source and target backwards). Fine, but
then why don't we use the cast machinery to do the conversion? This
is taking plpgsql's cowboy let's-coerce-via-IO-no-matter-whether-that's-
right-or-not approach and sticking it into a core part of the system.
There's no guarantee at all that applying typoutput then typinput
will match the conversion semantics you get from an actual cast, and
in many practical cases such as int4 to int8 it'll be drastically less
efficient anyway. It would make more sense to do something similar to
coerce_record_to_complex(), that is modify the expression tree to
coerce the columns using the regular cast machinery.

Also, the typmod part of the test seems completely broken. For one
thing, comparing typmods isn't sane if the types themselves aren't
the same. And it's quite unclear to me why we'd want to have an
anything-goes policy for type coercion, or even an implicit-casts-only
policy, but then insist that the typmods match exactly. This coding
will allow varchar(42) to text, but not varchar(42) to varchar(43)
... where's the sense in that?

The patch also seems to go a great deal further than what was asked for
originally, or indeed is mentioned in the documentation patch, namely
fixing the restriction on RETURN to allow composite-typed expressions.
Specifically it's changing the code that accepts composite input
arguments. Do we actually want that? If so, shouldn't it be
documented?

I'm inclined to suggest that we drop all the coercion stuff and just
do what Robert actually asked for originally, which was the mere
ability to return a composite value as long as it matched the function's
result type. I'm not convinced that we want automatic implicit type
coercions here. In any case I'm very much against sticking such a thing
into general-purpose support code like tupconvert.c. That will create a
strong likelihood that plpgsql's poorly-designed coercion semantics will
leak into other aspects of the system.

I think so without some change of coercion this patch is not too
useful because very simply test fail

create type foo(a int, b text);

create or replace function foo_func()
returns foo as $$
begin
...
return (10, 'hello');

end;

but we can limit a implicit coercion in tupconvert via new parameter -
because we would to forward plpgsql behave just from this direction.
Then when this parameter - maybe "allowIOCoercion" will be false, then
tupconvert will have same behave like before.

It would be nice to make that work, but it could be left for a
separate patch, I suppose. I'm OK with Tom's proposal to go ahead and
commit the core mechanic first, if doing more than that is
controversial.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#8)

Robert Haas <robertmhaas@gmail.com> writes:

On Thu, Dec 6, 2012 at 12:31 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:

but we can limit a implicit coercion in tupconvert via new parameter -
because we would to forward plpgsql behave just from this direction.
Then when this parameter - maybe "allowIOCoercion" will be false, then
tupconvert will have same behave like before.

It would be nice to make that work, but it could be left for a
separate patch, I suppose. I'm OK with Tom's proposal to go ahead and
commit the core mechanic first, if doing more than that is
controversial.

I'm against putting I/O coercion semantics into tupconvert, period. Ever.
If plpgsql wants that behavior rather than something more consistent
with the rest of the system, it needs to implement it for itself.

If the per-column conversions were cast-based, it wouldn't be such a
flagrantly bad idea to implement it in tupconvert. I'm still not
convinced that that's the best place for it though. tupconvert is about
rearranging columns, not about changing their contents.

It might be more appropriate to invent an expression evaluation
structure that could handle such nontrivial composite-type coercions.
I'm imagining that somehow we disassemble a composite value (produced by
some initial expression node), pass its fields through coercion nodes as
required, and then reassemble them in a toplevel RowExpr.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#9)

On Thu, Dec 6, 2012 at 1:36 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm against putting I/O coercion semantics into tupconvert, period. Ever.
If plpgsql wants that behavior rather than something more consistent
with the rest of the system, it needs to implement it for itself.

I'm sure that can be done. I don't think anyone is objecting to that,
just trying to get useful behavior out of the system.

Are you going to commit a stripped-down version of the patch?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#11Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#10)

Robert Haas <robertmhaas@gmail.com> writes:

Are you going to commit a stripped-down version of the patch?

I set it back to "waiting on author" --- don't know if he wants to
produce a stripped-down version with no type coercions, or try to use
cast-based coercions.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#12Asif Rehman
asifr.rehman@gmail.com
In reply to: Tom Lane (#11)
1 attachment(s)

Hi,

I have attached the stripped-down version. I will leave the type coercions
support for a separate patch.

Regards,
--Asif

On Fri, Dec 7, 2012 at 1:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Show quoted text

Robert Haas <robertmhaas@gmail.com> writes:

Are you going to commit a stripped-down version of the patch?

I set it back to "waiting on author" --- don't know if he wants to
produce a stripped-down version with no type coercions, or try to use
cast-based coercions.

regards, tom lane

Attachments:

return_rowtype.v4.patchapplication/octet-stream; name=return_rowtype.v4.patchDownload
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 07fba57..338f7c2 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -1571,11 +1571,10 @@ RETURN <replaceable>expression</replaceable>;
      </para>
 
      <para>
-      When returning a scalar type, any expression can be used. The
-      expression's result will be automatically cast into the
-      function's return type as described for assignments. To return a
-      composite (row) value, you must write a record or row variable
-      as the <replaceable>expression</replaceable>.
+      In <command>RETURN</command> statement you can use any expression.
+      The expression's result will be automatically cast into the
+      function's return type as described for assignments. However if
+      row expression is used, you may have to do explicit casting.
      </para>
 
      <para>
@@ -1600,6 +1599,21 @@ RETURN <replaceable>expression</replaceable>;
       however.  In those cases a <command>RETURN</command> statement is
       automatically executed if the top-level block finishes.
      </para>
+
+     <para>
+      Some examples:
+
+<programlisting>
+-- functions returning a scalar type
+RETURN 1 + 2;
+RETURN scalar_var;
+
+-- functions returning a composite type
+RETURN composite_type_var;
+RETURN (1, 2, 'three');
+</programlisting>
+
+     </para>
     </sect3>
 
     <sect3>
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 3b5b3bb..d1c0acd 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -214,6 +214,7 @@ static void free_params_data(PreparedParamsData *ppd);
 static Portal exec_dynquery_with_params(PLpgSQL_execstate *estate,
 						  PLpgSQL_expr *dynquery, List *params,
 						  const char *portalname, int cursorOptions);
+static HeapTuple get_tuple_from_datum(Datum value, TupleDesc *rettupdesc);
 
 
 /* ----------
@@ -275,23 +276,11 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo)
 
 					if (!fcinfo->argnull[i])
 					{
-						HeapTupleHeader td;
-						Oid			tupType;
-						int32		tupTypmod;
 						TupleDesc	tupdesc;
-						HeapTupleData tmptup;
-
-						td = DatumGetHeapTupleHeader(fcinfo->arg[i]);
-						/* Extract rowtype info and find a tupdesc */
-						tupType = HeapTupleHeaderGetTypeId(td);
-						tupTypmod = HeapTupleHeaderGetTypMod(td);
-						tupdesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
-						/* Build a temporary HeapTuple control structure */
-						tmptup.t_len = HeapTupleHeaderGetDatumLength(td);
-						ItemPointerSetInvalid(&(tmptup.t_self));
-						tmptup.t_tableOid = InvalidOid;
-						tmptup.t_data = td;
-						exec_move_row(&estate, NULL, row, &tmptup, tupdesc);
+						HeapTuple	tuple;
+
+						tuple = get_tuple_from_datum(fcinfo->arg[i], &tupdesc);
+						exec_move_row(&estate, NULL, row, tuple, tupdesc);
 						ReleaseTupleDesc(tupdesc);
 					}
 					else
@@ -2449,24 +2438,30 @@ exec_stmt_return(PLpgSQL_execstate *estate, PLpgSQL_stmt_return *stmt)
 
 	if (stmt->expr != NULL)
 	{
+		estate->retval = exec_eval_expr(estate, stmt->expr,
+										&(estate->retisnull),
+										&(estate->rettype));
 		if (estate->retistuple)
 		{
-			exec_run_select(estate, stmt->expr, 1, NULL);
-			if (estate->eval_processed > 0)
+			/* Source must be of RECORD or composite type */
+			if (!type_is_rowtype(estate->rettype))
+				ereport(ERROR,
+						(errcode(ERRCODE_DATATYPE_MISMATCH),
+						errmsg("cannot return non-composite value from composite type returning function")));
+
+			if (!estate->retisnull)
 			{
-				estate->retval = PointerGetDatum(estate->eval_tuptable->vals[0]);
-				estate->rettupdesc = estate->eval_tuptable->tupdesc;
-				estate->retisnull = false;
+				HeapTuple	tuple;
+				TupleDesc	tupdesc;
+
+				tuple = get_tuple_from_datum(estate->retval, &tupdesc);
+				estate->retval = PointerGetDatum(tuple);
+				estate->rettupdesc = CreateTupleDescCopy(tupdesc);
+				ReleaseTupleDesc(tupdesc);
 			}
 		}
-		else
-		{
-			/* Normal case for scalar results */
-			estate->retval = exec_eval_expr(estate, stmt->expr,
-											&(estate->retisnull),
-											&(estate->rettype));
-		}
 
+		/* Else, the expr is of scalar type and has been evaluated. simply return. */
 		return PLPGSQL_RC_RETURN;
 	}
 
@@ -2593,26 +2588,51 @@ exec_stmt_return_next(PLpgSQL_execstate *estate,
 		bool		isNull;
 		Oid			rettype;
 
-		if (natts != 1)
-			ereport(ERROR,
-					(errcode(ERRCODE_DATATYPE_MISMATCH),
-					 errmsg("wrong result type supplied in RETURN NEXT")));
-
 		retval = exec_eval_expr(estate,
 								stmt->expr,
 								&isNull,
 								&rettype);
 
-		/* coerce type if needed */
-		retval = exec_simple_cast_value(estate,
-										retval,
-										rettype,
-										tupdesc->attrs[0]->atttypid,
-										tupdesc->attrs[0]->atttypmod,
-										isNull);
+		/* Check if expr is of RECORD or composite type */
+		if (type_is_rowtype(rettype))
+		{
+			TupleConversionMap *tupmap;
+			TupleDesc retdesc;
+
+			tuple = get_tuple_from_datum(retval, &retdesc);
+			tupmap = convert_tuples_by_position(retdesc,
+												estate->rettupdesc,
+												gettext_noop("returned record type does not match expected record type"));
+			/* it might need conversion */
+			if (tupmap)
+			{
+				tuple = do_convert_tuple(tuple, tupmap);
+				free_conversion_map(tupmap);
+				free_tuple = true;
+			}
+
+			ReleaseTupleDesc(retdesc);
+		}
+		else
+		{
+			/* Normal case for scalar results */
+
+			if (natts != 1)
+				ereport(ERROR,
+						(errcode(ERRCODE_DATATYPE_MISMATCH),
+						errmsg("wrong result type supplied in RETURN NEXT")));
+
+			/* coerce type if needed */
+			retval = exec_simple_cast_value(estate,
+											retval,
+											rettype,
+											tupdesc->attrs[0]->atttypid,
+											tupdesc->attrs[0]->atttypmod,
+											isNull);
 
-		tuplestore_putvalues(estate->tuple_store, tupdesc,
-							 &retval, &isNull);
+			tuplestore_putvalues(estate->tuple_store, tupdesc,
+								&retval, &isNull);
+		}
 	}
 	else
 	{
@@ -3901,29 +3921,16 @@ exec_assign_value(PLpgSQL_execstate *estate,
 				}
 				else
 				{
-					HeapTupleHeader td;
-					Oid			tupType;
-					int32		tupTypmod;
 					TupleDesc	tupdesc;
-					HeapTupleData tmptup;
+					HeapTuple	tuple;
 
 					/* Source must be of RECORD or composite type */
 					if (!type_is_rowtype(valtype))
 						ereport(ERROR,
 								(errcode(ERRCODE_DATATYPE_MISMATCH),
 								 errmsg("cannot assign non-composite value to a row variable")));
-					/* Source is a tuple Datum, so safe to do this: */
-					td = DatumGetHeapTupleHeader(value);
-					/* Extract rowtype info and find a tupdesc */
-					tupType = HeapTupleHeaderGetTypeId(td);
-					tupTypmod = HeapTupleHeaderGetTypMod(td);
-					tupdesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
-					/* Build a temporary HeapTuple control structure */
-					tmptup.t_len = HeapTupleHeaderGetDatumLength(td);
-					ItemPointerSetInvalid(&(tmptup.t_self));
-					tmptup.t_tableOid = InvalidOid;
-					tmptup.t_data = td;
-					exec_move_row(estate, NULL, row, &tmptup, tupdesc);
+					tuple = get_tuple_from_datum(value, &tupdesc);
+					exec_move_row(estate, NULL, row, tuple, tupdesc);
 					ReleaseTupleDesc(tupdesc);
 				}
 				break;
@@ -3943,11 +3950,8 @@ exec_assign_value(PLpgSQL_execstate *estate,
 				}
 				else
 				{
-					HeapTupleHeader td;
-					Oid			tupType;
-					int32		tupTypmod;
 					TupleDesc	tupdesc;
-					HeapTupleData tmptup;
+					HeapTuple	tuple;
 
 					/* Source must be of RECORD or composite type */
 					if (!type_is_rowtype(valtype))
@@ -3955,18 +3959,8 @@ exec_assign_value(PLpgSQL_execstate *estate,
 								(errcode(ERRCODE_DATATYPE_MISMATCH),
 								 errmsg("cannot assign non-composite value to a record variable")));
 
-					/* Source is a tuple Datum, so safe to do this: */
-					td = DatumGetHeapTupleHeader(value);
-					/* Extract rowtype info and find a tupdesc */
-					tupType = HeapTupleHeaderGetTypeId(td);
-					tupTypmod = HeapTupleHeaderGetTypMod(td);
-					tupdesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
-					/* Build a temporary HeapTuple control structure */
-					tmptup.t_len = HeapTupleHeaderGetDatumLength(td);
-					ItemPointerSetInvalid(&(tmptup.t_self));
-					tmptup.t_tableOid = InvalidOid;
-					tmptup.t_data = td;
-					exec_move_row(estate, rec, NULL, &tmptup, tupdesc);
+					tuple = get_tuple_from_datum(value, &tupdesc);
+					exec_move_row(estate, rec, NULL, tuple, tupdesc);
 					ReleaseTupleDesc(tupdesc);
 				}
 				break;
@@ -6280,3 +6274,38 @@ exec_dynquery_with_params(PLpgSQL_execstate *estate,
 
 	return portal;
 }
+
+/* ----------
+ * get_tuple_from_datum		get a tuple from the rowtype datum
+ *
+ * If rettupdesc isn't NULL, it will receive a pointer to TupleDesc
+ * of rowtype.
+ *
+ * Note: its caller's responsibility to check 'value' is rowtype datum.
+ * ----------
+ */
+static HeapTuple
+get_tuple_from_datum(Datum value, TupleDesc *rettupdesc)
+{
+	HeapTupleHeader td;
+	Oid			tupType;
+	int32		tupTypmod;
+	HeapTupleData tmptup;
+
+	td = DatumGetHeapTupleHeader(value);
+	/* Extract rowtype info and find a tupdesc */
+	if (rettupdesc)
+	{
+		tupType = HeapTupleHeaderGetTypeId(td);
+		tupTypmod = HeapTupleHeaderGetTypMod(td);
+		*rettupdesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
+	}
+
+	/* Build a temporary HeapTuple control structure */
+	tmptup.t_len = HeapTupleHeaderGetDatumLength(td);
+	ItemPointerSetInvalid(&(tmptup.t_self));
+	tmptup.t_tableOid = InvalidOid;
+	tmptup.t_data = td;
+
+	return heap_copytuple(&tmptup);
+}
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index cf164d0..c3b59e8 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -2926,7 +2926,8 @@ make_return_stmt(int location)
 	}
 	else if (plpgsql_curr_compile->fn_retistuple)
 	{
-		switch (yylex())
+		int tok = yylex();
+		switch (tok)
 		{
 			case K_NULL:
 				/* we allow this to support RETURN NULL in triggers */
@@ -2944,10 +2945,13 @@ make_return_stmt(int location)
 				break;
 
 			default:
-				ereport(ERROR,
-						(errcode(ERRCODE_DATATYPE_MISMATCH),
-						 errmsg("RETURN must specify a record or row variable in function returning row"),
-						 parser_errposition(yylloc)));
+				/*
+				 * Allow use of expression in return statement for functions returning
+				 * row types.
+				 */
+				plpgsql_push_back_token(tok);
+				new->expr = read_sql_expression(';', ";");
+				return (PLpgSQL_stmt *) new;
 				break;
 		}
 		if (yylex() != ';')
@@ -2994,7 +2998,8 @@ make_return_next_stmt(int location)
 	}
 	else if (plpgsql_curr_compile->fn_retistuple)
 	{
-		switch (yylex())
+		int tok = yylex();
+		switch (tok)
 		{
 			case T_DATUM:
 				if (yylval.wdatum.datum->dtype == PLPGSQL_DTYPE_ROW ||
@@ -3008,10 +3013,13 @@ make_return_next_stmt(int location)
 				break;
 
 			default:
-				ereport(ERROR,
-						(errcode(ERRCODE_DATATYPE_MISMATCH),
-						 errmsg("RETURN NEXT must specify a record or row variable in function returning row"),
-						 parser_errposition(yylloc)));
+				/*
+				 * Allow use of expression in return statement for functions returning
+				 * row types.
+				 */
+				plpgsql_push_back_token(tok);
+				new->expr = read_sql_expression(';', ";");
+				return (PLpgSQL_stmt *) new;
 				break;
 		}
 		if (yylex() != ';')
diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule
index be789e3..7ea2a26 100644
--- a/src/test/regress/serial_schedule
+++ b/src/test/regress/serial_schedule
@@ -134,3 +134,4 @@ test: largeobject
 test: with
 test: xml
 test: stats
+test: return_rowtype
diff --git a/src/test/regress/expected/return_rowtype.out b/src/test/regress/expected/return_rowtype.out
new file mode 100644
index 0000000..85baaed
--- /dev/null
+++ b/src/test/regress/expected/return_rowtype.out
@@ -0,0 +1,135 @@
+--create an composite type
+create type footype as (x int, y varchar);
+--test: use of variable of composite type in return statement
+create or replace function foo() returns footype as $$
+declare
+  v footype;
+begin
+  v := (1, 'hello');
+  return v;
+end;
+$$ language plpgsql;
+select foo();
+    foo    
+-----------
+ (1,hello)
+(1 row)
+
+--test: use row expr in return statement
+create or replace function foo() returns footype as $$
+begin
+  return (1, 'hello'::varchar);
+end;
+$$ language plpgsql;
+select foo();
+    foo    
+-----------
+ (1,hello)
+(1 row)
+
+DO $$
+declare
+  v footype;
+begin
+  v := foo();
+  raise info 'x = %', v.x;
+  raise info 'y = %', v.y;
+end;
+$$;
+INFO:  x = 1
+INFO:  y = hello
+drop function foo();
+--create a table
+create table footab(x int, y varchar(10));
+--test: return a row expr
+create or replace function foorec() returns footab as $$
+begin
+  return (1, 'hello'::varchar(10));
+end;
+$$ language plpgsql;
+DO $$
+declare
+  v footab%rowtype;
+begin
+  v := foorec();
+  raise info 'x = %', v.x;
+  raise info 'y = %', v.y;
+end; $$;
+INFO:  x = 1
+INFO:  y = hello
+drop function foorec();
+drop table footab;
+--test: return a row expr as record.
+create or replace function foorec() returns record as $$
+declare
+  v record;
+begin
+  v := (1, 'hello');
+  return v;
+end;
+$$ language plpgsql;
+select foorec();
+  foorec   
+-----------
+ (1,hello)
+(1 row)
+
+DO $$
+declare
+  v record;
+begin
+  v := foorec();
+  raise info 'rec = %', v;
+end; $$;
+INFO:  rec = (1,hello)
+--test: return row expr in return statement.
+create or replace function foorec() returns record as $$
+begin
+  return (1, 'hello');
+end;
+$$ language plpgsql;
+select foorec();
+  foorec   
+-----------
+ (1,hello)
+(1 row)
+
+DO $$
+declare
+  v record;
+begin
+  v := foorec();
+  raise info 'rec = %', v;
+end; $$;
+INFO:  rec = (1,hello)
+drop function foorec();
+--test: row expr in RETURN NEXT statement.
+create or replace function foo() returns setof footype as $$
+begin
+  for i in 1..10
+  loop
+    return next (1, 'hello');
+  end loop;
+  return;
+end;
+$$ language plpgsql;
+drop function foo();
+--test: use invalid expr in return statement.
+create or replace function foo() returns footype as $$
+begin
+  return 1 + 1;
+end;
+$$ language plpgsql;
+DO $$
+declare
+  v footype;
+begin
+  v := foo();
+  raise info 'x = %', v.x;
+  raise info 'y = %', v.y;
+end; $$;
+ERROR:  cannot return non-composite value from composite type returning function
+CONTEXT:  PL/pgSQL function foo() line 3 at RETURN
+PL/pgSQL function inline_code_block line 5 at assignment
+drop function foo();
+drop type footype;
diff --git a/src/test/regress/sql/return_rowtype.sql b/src/test/regress/sql/return_rowtype.sql
new file mode 100644
index 0000000..7289f30
--- /dev/null
+++ b/src/test/regress/sql/return_rowtype.sql
@@ -0,0 +1,128 @@
+--create an composite type
+create type footype as (x int, y varchar);
+
+--test: use of variable of composite type in return statement
+create or replace function foo() returns footype as $$
+declare
+  v footype;
+begin
+  v := (1, 'hello');
+  return v;
+end;
+$$ language plpgsql;
+
+select foo();
+
+--test: use row expr in return statement
+create or replace function foo() returns footype as $$
+begin
+  return (1, 'hello'::varchar);
+end;
+$$ language plpgsql;
+
+select foo();
+
+DO $$
+declare
+  v footype;
+begin
+  v := foo();
+  raise info 'x = %', v.x;
+  raise info 'y = %', v.y;
+end;
+$$;
+
+drop function foo();
+
+--create a table
+create table footab(x int, y varchar(10));
+
+--test: return a row expr
+create or replace function foorec() returns footab as $$
+begin
+  return (1, 'hello'::varchar(10));
+end;
+$$ language plpgsql;
+
+DO $$
+declare
+  v footab%rowtype;
+begin
+  v := foorec();
+  raise info 'x = %', v.x;
+  raise info 'y = %', v.y;
+end; $$;
+
+drop function foorec();
+drop table footab;
+
+--test: return a row expr as record.
+create or replace function foorec() returns record as $$
+declare
+  v record;
+begin
+  v := (1, 'hello');
+  return v;
+end;
+$$ language plpgsql;
+
+select foorec();
+
+DO $$
+declare
+  v record;
+begin
+  v := foorec();
+  raise info 'rec = %', v;
+end; $$;
+
+--test: return row expr in return statement.
+create or replace function foorec() returns record as $$
+begin
+  return (1, 'hello');
+end;
+$$ language plpgsql;
+
+select foorec();
+
+DO $$
+declare
+  v record;
+begin
+  v := foorec();
+  raise info 'rec = %', v;
+end; $$;
+
+drop function foorec();
+
+--test: row expr in RETURN NEXT statement.
+create or replace function foo() returns setof footype as $$
+begin
+  for i in 1..10
+  loop
+    return next (1, 'hello');
+  end loop;
+  return;
+end;
+$$ language plpgsql;
+
+drop function foo();
+
+--test: use invalid expr in return statement.
+create or replace function foo() returns footype as $$
+begin
+  return 1 + 1;
+end;
+$$ language plpgsql;
+
+DO $$
+declare
+  v footype;
+begin
+  v := foo();
+  raise info 'x = %', v.x;
+  raise info 'y = %', v.y;
+end; $$;
+
+drop function foo();
+drop type footype;
#13Tom Lane
tgl@sss.pgh.pa.us
In reply to: Asif Rehman (#12)

Asif Rehman <asifr.rehman@gmail.com> writes:

I have attached the stripped-down version. I will leave the type coercions
support for a separate patch.

OK, I'll take a look at this one.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#14Tom Lane
tgl@sss.pgh.pa.us
In reply to: Asif Rehman (#12)

Asif Rehman <asifr.rehman@gmail.com> writes:

I have attached the stripped-down version. I will leave the type coercions
support for a separate patch.

Applied with assorted corrections.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#15Asif Rehman
asifr.rehman@gmail.com
In reply to: Tom Lane (#14)

Thanks.

Regards,
--Asif

On Fri, Dec 7, 2012 at 9:11 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Show quoted text

Asif Rehman <asifr.rehman@gmail.com> writes:

I have attached the stripped-down version. I will leave the type

coercions

support for a separate patch.

Applied with assorted corrections.

regards, tom lane