review: plpgsql return a row-expression
* patched and compiled withou warnings
* All 133 tests passed.
but
I don't like
* call invalid function from anonymous block - it is messy (regress
tests) - there is no reason why do it
+create or replace function foo() returns footype as $$
+declare
+ v record;
+ v2 record;
+begin
+ v := (1, 'hello');
+ v2 := (1, 'hello');
+ return (v || v2);
+end;
+$$ language plpgsql;
+DO $$
+declare
+ v footype;
+begin
+ v := foo();
+ raise info 'x = %', v.x;
+ raise info 'y = %', v.y;
+end; $$;
+ERROR: operator does not exist: record || record
+LINE 1: SELECT (v || v2)
+ ^
* there is some performance issue
create or replace function fx2(a int)
returns footype as $$
declare x footype;
begin
x = (10,20);
return x;
end;
$$ language plpgsql;
postgres=# select sum(fx2.x) from generate_series(1,100000) g(i),
lateral fx2(i);
sum
---------
1000000
(1 row)
Time: 497.129 ms
returns footype as $$
begin
return (10,20);
end;
$$ language plpgsql;
postgres=# select sum(fx2.x) from generate_series(1,100000) g(i),
lateral fx2(i);
sum
---------
1000000
(1 row)
Time: 941.192 ms
following code has same functionality and it is faster
if (stmt->expr != NULL)
{
if (estate->retistuple)
{
TupleDesc tupdesc;
Datum retval;
Oid rettype;
bool isnull;
int32 tupTypmod;
Oid tupType;
HeapTupleHeader td;
HeapTupleData tmptup;
retval = exec_eval_expr(estate,
stmt->expr,
&isnull,
&rettype);
/* Source must be of RECORD or composite type */
if (!type_is_rowtype(rettype))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("cannot return
non-composite value from composite type returning function")));
if (!isnull)
{
/* Source is a tuple Datum, so safe to
do this: */
td = DatumGetHeapTupleHeader(retval);
/* 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;
estate->retval =
PointerGetDatum(heap_copytuple(&tmptup));
estate->rettupdesc =
CreateTupleDescCopy(tupdesc);
ReleaseTupleDesc(tupdesc);
}
estate->retisnull = isnull;
}
* it is to restrictive (maybe) - almost all plpgsql' statements does
automatic conversions (IO conversions when is necessary)
create type footype2 as (a numeric, b varchar)
postgres=# create or replace function fx3(a int)
returns footype2 as
$$
begin
return (10000000.22234213412342143,'ewqerwqreeeee');
end;
$$ language plpgsql;
CREATE FUNCTION
postgres=# select fx3(10);
ERROR: returned record type does not match expected record type
DETAIL: Returned type unknown does not match expected type character
varying in column 2.
CONTEXT: PL/pgSQL function fx3(integer) while casting return value to
function's return type
postgres=#
* it doesn't support RETURN NEXT
postgres=# create or replace function fx4()
postgres-# returns setof footype as $$
postgres$# begin
postgres$# for i in 1..10
postgres$# loop
postgres$# return next (10,20);
postgres$# end loop;
postgres$# return;
postgres$# end;
postgres$# $$ language plpgsql;
ERROR: RETURN NEXT must specify a record or row variable in function
returning row
LINE 6: return next (10,20);
* missing any documentation
* repeated code - following code is used on more places in pl_exec.c
and maybe it is candidate for subroutine
+ /* 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_copy(tupType, tupTypmod);
+ /* Build a HeapTuple control structure */
+ htup.t_len = HeapTupleHeaderGetDatumLength(td);
+ ItemPointerSetInvalid(&(htup.t_self));
+ htup.t_tableOid = InvalidOid;
+ htup.t_data = td;
+ tuple = heap_copytuple(&htup);
Regards
Pavel Stehule
Thanks for the review Pavel. I have taken care of the points you raised.
Please see the updated patch.
Regards,
--Asif
On Wed, Nov 21, 2012 at 1:47 AM, Pavel Stehule <pavel.stehule@gmail.com>wrote:
Show quoted text
* patched and compiled withou warnings
* All 133 tests passed.
but
I don't like
* call invalid function from anonymous block - it is messy (regress
tests) - there is no reason why do it+create or replace function foo() returns footype as $$ +declare + v record; + v2 record; +begin + v := (1, 'hello'); + v2 := (1, 'hello'); + return (v || v2); +end; +$$ language plpgsql; +DO $$ +declare + v footype; +begin + v := foo(); + raise info 'x = %', v.x; + raise info 'y = %', v.y; +end; $$; +ERROR: operator does not exist: record || record +LINE 1: SELECT (v || v2) + ^* there is some performance issue
create or replace function fx2(a int)
returns footype as $$
declare x footype;
begin
x = (10,20);
return x;
end;
$$ language plpgsql;postgres=# select sum(fx2.x) from generate_series(1,100000) g(i),
lateral fx2(i);
sum
---------
1000000
(1 row)Time: 497.129 ms
returns footype as $$
begin
return (10,20);
end;
$$ language plpgsql;postgres=# select sum(fx2.x) from generate_series(1,100000) g(i),
lateral fx2(i);
sum
---------
1000000
(1 row)Time: 941.192 ms
following code has same functionality and it is faster
if (stmt->expr != NULL)
{
if (estate->retistuple)
{
TupleDesc tupdesc;
Datum retval;
Oid rettype;
bool isnull;
int32 tupTypmod;
Oid tupType;
HeapTupleHeader td;
HeapTupleData tmptup;retval = exec_eval_expr(estate,
stmt->expr,
&isnull,
&rettype);/* Source must be of RECORD or composite type */
if (!type_is_rowtype(rettype))
ereport(ERROR,(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("cannot return
non-composite value from composite type returning function")));if (!isnull)
{
/* Source is a tuple Datum, so safe to
do this: */
td = DatumGetHeapTupleHeader(retval);
/* 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;estate->retval =
PointerGetDatum(heap_copytuple(&tmptup));
estate->rettupdesc =
CreateTupleDescCopy(tupdesc);
ReleaseTupleDesc(tupdesc);
}estate->retisnull = isnull;
}* it is to restrictive (maybe) - almost all plpgsql' statements does
automatic conversions (IO conversions when is necessary)create type footype2 as (a numeric, b varchar)
postgres=# create or replace function fx3(a int)
returns footype2 as
$$
begin
return (10000000.22234213412342143,'ewqerwqreeeee');
end;
$$ language plpgsql;
CREATE FUNCTION
postgres=# select fx3(10);
ERROR: returned record type does not match expected record type
DETAIL: Returned type unknown does not match expected type character
varying in column 2.
CONTEXT: PL/pgSQL function fx3(integer) while casting return value to
function's return type
postgres=#* it doesn't support RETURN NEXT
postgres=# create or replace function fx4()
postgres-# returns setof footype as $$
postgres$# begin
postgres$# for i in 1..10
postgres$# loop
postgres$# return next (10,20);
postgres$# end loop;
postgres$# return;
postgres$# end;
postgres$# $$ language plpgsql;
ERROR: RETURN NEXT must specify a record or row variable in function
returning row
LINE 6: return next (10,20);* missing any documentation
* repeated code - following code is used on more places in pl_exec.c
and maybe it is candidate for subroutine+ /* 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_copy(tupType, tupTypmod); + /* Build a HeapTuple control structure */ + htup.t_len = HeapTupleHeaderGetDatumLength(td); + ItemPointerSetInvalid(&(htup.t_self)); + htup.t_tableOid = InvalidOid; + htup.t_data = td; + tuple = heap_copytuple(&htup);Regards
Pavel Stehule
Attachments:
return_rowtype.2.patchapplication/octet-stream; name=return_rowtype.2.patchDownload
diff --git a/src/backend/access/common/tupconvert.c b/src/backend/access/common/tupconvert.c
index f813432..2901f47 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"
/*
@@ -100,8 +102,7 @@ convert_tuples_by_position(TupleDesc indesc,
continue;
nincols++;
/* Found matching column, check type */
- if (atttypid != att->atttypid ||
- (atttypmod != att->atttypmod && atttypmod >= 0))
+ if (!can_coerce_type(1, &atttypid, &(att->atttypid), COERCE_IMPLICIT_CAST))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg_internal("%s", _(msg)),
@@ -234,7 +235,7 @@ 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 (!can_coerce_type(1, &atttypid, &(att->atttypid), COERCE_IMPLICIT_CAST))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg_internal("%s", _(msg)),
@@ -339,9 +340,40 @@ do_convert_tuple(HeapTuple tuple, TupleConversionMap *map)
for (i = 0; i < outnatts; i++)
{
int j = attrMap[i];
+ TupleDesc indesc = map->indesc;
+ TupleDesc outdesc = map->outdesc;
+ Oid reqtype = outdesc->attrs[i]->atttypid;
+ int32 reqtypmod = outdesc->attrs[i]->atttypmod;
outvalues[i] = invalues[j];
outisnull[i] = inisnull[j];
+
+ /*
+ * If types are not exactly matched, do the casting. We have already
+ * performed the can_coerce_type() check. So It should be safe.
+ */
+ if (!inisnull[j] &&
+ (reqtype != indesc->attrs[j - 1]->atttypid ||
+ reqtypmod != indesc->attrs[j - 1]->atttypmod))
+ {
+ Oid typinput;
+ Oid typoutput;
+ Oid typioparam;
+ Oid valtype = indesc->attrs[j - 1]->atttypid;
+ bool typIsVarlena;
+ FmgrInfo finfo_input;
+ char *strval;
+
+ getTypeOutputInfo(valtype, &typoutput, &typIsVarlena);
+ getTypeInputInfo(reqtype, &typinput, &typioparam);
+ fmgr_info(typinput, &finfo_input);
+
+ strval = OidOutputFunctionCall(typoutput, invalues[j]);
+ outvalues[i] = InputFunctionCall(&finfo_input,
+ strval,
+ typioparam,
+ reqtypmod);
+ }
}
/*
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;
Hi,
I forgot to add documentation changes in the earlier patch. In the attached
patch, I have added documentation as well as fixed other issues you raised.
Regards,
--Asif
On Thu, Nov 22, 2012 at 10:47 PM, Asif Rehman <asifr.rehman@gmail.com>wrote:
Show quoted text
Thanks for the review Pavel. I have taken care of the points you raised.
Please see the updated patch.Regards,
--AsifOn Wed, Nov 21, 2012 at 1:47 AM, Pavel Stehule <pavel.stehule@gmail.com>wrote:
* patched and compiled withou warnings
* All 133 tests passed.
but
I don't like
* call invalid function from anonymous block - it is messy (regress
tests) - there is no reason why do it+create or replace function foo() returns footype as $$ +declare + v record; + v2 record; +begin + v := (1, 'hello'); + v2 := (1, 'hello'); + return (v || v2); +end; +$$ language plpgsql; +DO $$ +declare + v footype; +begin + v := foo(); + raise info 'x = %', v.x; + raise info 'y = %', v.y; +end; $$; +ERROR: operator does not exist: record || record +LINE 1: SELECT (v || v2) + ^* there is some performance issue
create or replace function fx2(a int)
returns footype as $$
declare x footype;
begin
x = (10,20);
return x;
end;
$$ language plpgsql;postgres=# select sum(fx2.x) from generate_series(1,100000) g(i),
lateral fx2(i);
sum
---------
1000000
(1 row)Time: 497.129 ms
returns footype as $$
begin
return (10,20);
end;
$$ language plpgsql;postgres=# select sum(fx2.x) from generate_series(1,100000) g(i),
lateral fx2(i);
sum
---------
1000000
(1 row)Time: 941.192 ms
following code has same functionality and it is faster
if (stmt->expr != NULL)
{
if (estate->retistuple)
{
TupleDesc tupdesc;
Datum retval;
Oid rettype;
bool isnull;
int32 tupTypmod;
Oid tupType;
HeapTupleHeader td;
HeapTupleData tmptup;retval = exec_eval_expr(estate,
stmt->expr,
&isnull,
&rettype);/* Source must be of RECORD or composite type */
if (!type_is_rowtype(rettype))
ereport(ERROR,(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("cannot return
non-composite value from composite type returning function")));if (!isnull)
{
/* Source is a tuple Datum, so safe to
do this: */
td = DatumGetHeapTupleHeader(retval);
/* 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;estate->retval =
PointerGetDatum(heap_copytuple(&tmptup));
estate->rettupdesc =
CreateTupleDescCopy(tupdesc);
ReleaseTupleDesc(tupdesc);
}estate->retisnull = isnull;
}* it is to restrictive (maybe) - almost all plpgsql' statements does
automatic conversions (IO conversions when is necessary)create type footype2 as (a numeric, b varchar)
postgres=# create or replace function fx3(a int)
returns footype2 as
$$
begin
return (10000000.22234213412342143,'ewqerwqreeeee');
end;
$$ language plpgsql;
CREATE FUNCTION
postgres=# select fx3(10);
ERROR: returned record type does not match expected record type
DETAIL: Returned type unknown does not match expected type character
varying in column 2.
CONTEXT: PL/pgSQL function fx3(integer) while casting return value to
function's return type
postgres=#* it doesn't support RETURN NEXT
postgres=# create or replace function fx4()
postgres-# returns setof footype as $$
postgres$# begin
postgres$# for i in 1..10
postgres$# loop
postgres$# return next (10,20);
postgres$# end loop;
postgres$# return;
postgres$# end;
postgres$# $$ language plpgsql;
ERROR: RETURN NEXT must specify a record or row variable in function
returning row
LINE 6: return next (10,20);* missing any documentation
* repeated code - following code is used on more places in pl_exec.c
and maybe it is candidate for subroutine+ /* 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_copy(tupType, tupTypmod); + /* Build a HeapTuple control structure */ + htup.t_len = HeapTupleHeaderGetDatumLength(td); + ItemPointerSetInvalid(&(htup.t_self)); + htup.t_tableOid = InvalidOid; + htup.t_data = td; + tuple = heap_copytuple(&htup);Regards
Pavel Stehule
Attachments:
return_rowtype.upd.patchapplication/octet-stream; name=return_rowtype.upd.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..2901f47 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"
/*
@@ -100,8 +102,7 @@ convert_tuples_by_position(TupleDesc indesc,
continue;
nincols++;
/* Found matching column, check type */
- if (atttypid != att->atttypid ||
- (atttypmod != att->atttypmod && atttypmod >= 0))
+ if (!can_coerce_type(1, &atttypid, &(att->atttypid), COERCE_IMPLICIT_CAST))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg_internal("%s", _(msg)),
@@ -234,7 +235,7 @@ 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 (!can_coerce_type(1, &atttypid, &(att->atttypid), COERCE_IMPLICIT_CAST))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg_internal("%s", _(msg)),
@@ -339,9 +340,40 @@ do_convert_tuple(HeapTuple tuple, TupleConversionMap *map)
for (i = 0; i < outnatts; i++)
{
int j = attrMap[i];
+ TupleDesc indesc = map->indesc;
+ TupleDesc outdesc = map->outdesc;
+ Oid reqtype = outdesc->attrs[i]->atttypid;
+ int32 reqtypmod = outdesc->attrs[i]->atttypmod;
outvalues[i] = invalues[j];
outisnull[i] = inisnull[j];
+
+ /*
+ * If types are not exactly matched, do the casting. We have already
+ * performed the can_coerce_type() check. So It should be safe.
+ */
+ if (!inisnull[j] &&
+ (reqtype != indesc->attrs[j - 1]->atttypid ||
+ reqtypmod != indesc->attrs[j - 1]->atttypmod))
+ {
+ Oid typinput;
+ Oid typoutput;
+ Oid typioparam;
+ Oid valtype = indesc->attrs[j - 1]->atttypid;
+ bool typIsVarlena;
+ FmgrInfo finfo_input;
+ char *strval;
+
+ getTypeOutputInfo(valtype, &typoutput, &typIsVarlena);
+ getTypeInputInfo(reqtype, &typinput, &typioparam);
+ fmgr_info(typinput, &finfo_input);
+
+ strval = OidOutputFunctionCall(typoutput, invalues[j]);
+ outvalues[i] = InputFunctionCall(&finfo_input,
+ strval,
+ typioparam,
+ reqtypmod);
+ }
}
/*
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;
Hello
2012/11/23 Asif Rehman <asifr.rehman@gmail.com>:
Hi,
I forgot to add documentation changes in the earlier patch. In the attached
patch, I have added documentation as well as fixed other issues you raised.
ok
so
* applied and compiled cleanly
* All 133 tests passed.
* there are doc
* there are necessary regress tests
* automatic conversion is works like plpgsql user expects
* there are no performance issue now
* code respects our coding standards
+ code remove redundant lines
I have no any objection
this patch is ready to commit!
Regards
Pavel
Show quoted text
Regards,
--AsifOn Thu, Nov 22, 2012 at 10:47 PM, Asif Rehman <asifr.rehman@gmail.com>
wrote:Thanks for the review Pavel. I have taken care of the points you raised.
Please see the updated patch.Regards,
--AsifOn Wed, Nov 21, 2012 at 1:47 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:* patched and compiled withou warnings
* All 133 tests passed.
but
I don't like
* call invalid function from anonymous block - it is messy (regress
tests) - there is no reason why do it+create or replace function foo() returns footype as $$ +declare + v record; + v2 record; +begin + v := (1, 'hello'); + v2 := (1, 'hello'); + return (v || v2); +end; +$$ language plpgsql; +DO $$ +declare + v footype; +begin + v := foo(); + raise info 'x = %', v.x; + raise info 'y = %', v.y; +end; $$; +ERROR: operator does not exist: record || record +LINE 1: SELECT (v || v2) + ^* there is some performance issue
create or replace function fx2(a int)
returns footype as $$
declare x footype;
begin
x = (10,20);
return x;
end;
$$ language plpgsql;postgres=# select sum(fx2.x) from generate_series(1,100000) g(i),
lateral fx2(i);
sum
---------
1000000
(1 row)Time: 497.129 ms
returns footype as $$
begin
return (10,20);
end;
$$ language plpgsql;postgres=# select sum(fx2.x) from generate_series(1,100000) g(i),
lateral fx2(i);
sum
---------
1000000
(1 row)Time: 941.192 ms
following code has same functionality and it is faster
if (stmt->expr != NULL)
{
if (estate->retistuple)
{
TupleDesc tupdesc;
Datum retval;
Oid rettype;
bool isnull;
int32 tupTypmod;
Oid tupType;
HeapTupleHeader td;
HeapTupleData tmptup;retval = exec_eval_expr(estate,
stmt->expr,
&isnull,&rettype);
/* Source must be of RECORD or composite type */
if (!type_is_rowtype(rettype))
ereport(ERROR,(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("cannot return
non-composite value from composite type returning function")));if (!isnull)
{
/* Source is a tuple Datum, so safe to
do this: */
td = DatumGetHeapTupleHeader(retval);
/* 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;estate->retval =
PointerGetDatum(heap_copytuple(&tmptup));
estate->rettupdesc =
CreateTupleDescCopy(tupdesc);
ReleaseTupleDesc(tupdesc);
}estate->retisnull = isnull;
}* it is to restrictive (maybe) - almost all plpgsql' statements does
automatic conversions (IO conversions when is necessary)create type footype2 as (a numeric, b varchar)
postgres=# create or replace function fx3(a int)
returns footype2 as
$$
begin
return (10000000.22234213412342143,'ewqerwqreeeee');
end;
$$ language plpgsql;
CREATE FUNCTION
postgres=# select fx3(10);
ERROR: returned record type does not match expected record type
DETAIL: Returned type unknown does not match expected type character
varying in column 2.
CONTEXT: PL/pgSQL function fx3(integer) while casting return value to
function's return type
postgres=#* it doesn't support RETURN NEXT
postgres=# create or replace function fx4()
postgres-# returns setof footype as $$
postgres$# begin
postgres$# for i in 1..10
postgres$# loop
postgres$# return next (10,20);
postgres$# end loop;
postgres$# return;
postgres$# end;
postgres$# $$ language plpgsql;
ERROR: RETURN NEXT must specify a record or row variable in function
returning row
LINE 6: return next (10,20);* missing any documentation
* repeated code - following code is used on more places in pl_exec.c
and maybe it is candidate for subroutine+ /* 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_copy(tupType, tupTypmod); + /* Build a HeapTuple control structure */ + htup.t_len = HeapTupleHeaderGetDatumLength(td); + ItemPointerSetInvalid(&(htup.t_self)); + htup.t_tableOid = InvalidOid; + htup.t_data = td; + tuple = heap_copytuple(&htup);Regards
Pavel Stehule