diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c index c7ee17a57b721da9e7f170cdd211f72281175747..c30eac28b7fc779375193137723365ff0327132b 100644 --- a/src/backend/catalog/pg_proc.c +++ b/src/backend/catalog/pg_proc.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/catalog/pg_proc.c,v 1.168 2009/10/08 02:39:18 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/catalog/pg_proc.c,v 1.169 2009/12/14 02:15:49 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -810,7 +810,7 @@ fmgr_sql_validator(PG_FUNCTION_ARGS) proc->pronargs); (void) check_sql_fn_retval(funcoid, proc->prorettype, querytree_list, - false, NULL); + NULL, NULL); } else querytree_list = pg_parse_query(prosrc); diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c index 36b72abcce29b09e8bdd4a293c45bb1fd24c82df..93b668181ff234e1d4fc3ad8caf02e4d5ac8a952 100644 --- a/src/backend/executor/execQual.c +++ b/src/backend/executor/execQual.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.255 2009/11/20 20:38:10 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.256 2009/12/14 02:15:49 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -47,6 +47,7 @@ #include "nodes/makefuncs.h" #include "nodes/nodeFuncs.h" #include "optimizer/planner.h" +#include "parser/parse_coerce.h" #include "pgstat.h" #include "utils/acl.h" #include "utils/builtins.h" @@ -1382,7 +1383,7 @@ tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc) Form_pg_attribute dattr = dst_tupdesc->attrs[i]; Form_pg_attribute sattr = src_tupdesc->attrs[i]; - if (dattr->atttypid == sattr->atttypid) + if (IsBinaryCoercible(sattr->atttypid, dattr->atttypid)) continue; /* no worries */ if (!dattr->attisdropped) ereport(ERROR, diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c index 2934e51161ea91abac6fcf8e7ba8053c7ca2c649..7a9c319c7b46b93afaa94d36c918a122a372970c 100644 --- a/src/backend/executor/functions.c +++ b/src/backend/executor/functions.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/functions.c,v 1.136 2009/11/04 22:26:05 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/functions.c,v 1.137 2009/12/14 02:15:51 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -338,7 +338,7 @@ init_sql_fcache(FmgrInfo *finfo, bool lazyEvalOK) fcache->returnsTuple = check_sql_fn_retval(foid, rettype, queryTree_list, - false, + NULL, &fcache->junkFilter); if (fcache->returnsTuple) @@ -1003,14 +1003,27 @@ ShutdownSQLFunction(Datum arg) * function definition of a polymorphic function.) * * This function returns true if the sql function returns the entire tuple - * result of its final statement, and false otherwise. Note that because we - * allow "SELECT rowtype_expression", this may be false even when the declared - * function return type is a rowtype. + * result of its final statement, or false if it returns just the first column + * result of that statement. It throws an error if the final statement doesn't + * return the right type at all. * - * If insertRelabels is true, then binary-compatible cases are dealt with - * by actually inserting RelabelType nodes into the output targetlist; - * obviously the caller must pass a parsetree that it's okay to modify in this - * case. + * Note that because we allow "SELECT rowtype_expression", the result can be + * false even when the declared function return type is a rowtype. + * + * If modifyTargetList isn't NULL, the function will modify the final + * statement's targetlist in two cases: + * (1) if the tlist returns values that are binary-coercible to the expected + * type rather than being exactly the expected type. RelabelType nodes will + * be inserted to make the result types match exactly. + * (2) if there are dropped columns in the declared result rowtype. NULL + * output columns will be inserted in the tlist to match them. + * (Obviously the caller must pass a parsetree that is okay to modify when + * using this flag.) Note that this flag does not affect whether the tlist is + * considered to be a legal match to the result type, only how we react to + * allowed not-exact-match cases. *modifyTargetList will be set true iff + * we had to make any "dangerous" changes that could modify the semantics of + * the statement. If it is set true, the caller should not use the modified + * statement, but for simplicity we apply the changes anyway. * * If junkFilter isn't NULL, then *junkFilter is set to a JunkFilter defined * to convert the function's tuple result to the correct output tuple type. @@ -1019,10 +1032,11 @@ ShutdownSQLFunction(Datum arg) */ bool check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList, - bool insertRelabels, + bool *modifyTargetList, JunkFilter **junkFilter) { Query *parse; + List **tlist_ptr; List *tlist; int tlistlen; char fn_typtype; @@ -1031,6 +1045,8 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList, AssertArg(!IsPolymorphicType(rettype)); + if (modifyTargetList) + *modifyTargetList = false; /* initialize for no change */ if (junkFilter) *junkFilter = NULL; /* initialize in case of VOID result */ @@ -1064,6 +1080,7 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList, parse->utilityStmt == NULL && parse->intoClause == NULL) { + tlist_ptr = &parse->targetList; tlist = parse->targetList; } else if (parse && @@ -1072,6 +1089,7 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList, parse->commandType == CMD_DELETE) && parse->returningList) { + tlist_ptr = &parse->returningList; tlist = parse->returningList; } else @@ -1132,11 +1150,16 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList, format_type_be(rettype)), errdetail("Actual return type is %s.", format_type_be(restype)))); - if (insertRelabels && restype != rettype) + if (modifyTargetList && restype != rettype) + { tle->expr = (Expr *) makeRelabelType(tle->expr, rettype, -1, COERCE_DONTCARE); + /* Relabel is dangerous if TLE is a sort/group or setop column */ + if (tle->ressortgroupref != 0 || parse->setOperations) + *modifyTargetList = true; + } /* Set up junk filter if needed */ if (junkFilter) @@ -1149,6 +1172,8 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList, int tupnatts; /* physical number of columns in tuple */ int tuplogcols; /* # of nondeleted columns in tuple */ int colindex; /* physical column index */ + List *newtlist; /* new non-junk tlist entries */ + List *junkattrs; /* new junk tlist entries */ /* * If the target list is of length 1, and the type of the varnode in @@ -1165,11 +1190,16 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList, restype = exprType((Node *) tle->expr); if (IsBinaryCoercible(restype, rettype)) { - if (insertRelabels && restype != rettype) + if (modifyTargetList && restype != rettype) + { tle->expr = (Expr *) makeRelabelType(tle->expr, rettype, -1, COERCE_DONTCARE); + /* Relabel is dangerous if sort/group or setop column */ + if (tle->ressortgroupref != 0 || parse->setOperations) + *modifyTargetList = true; + } /* Set up junk filter if needed */ if (junkFilter) *junkFilter = ExecInitJunkFilter(tlist, false, NULL); @@ -1193,11 +1223,14 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList, /* * Verify that the targetlist matches the return tuple type. We scan * the non-deleted attributes to ensure that they match the datatypes - * of the non-resjunk columns. + * of the non-resjunk columns. For deleted attributes, insert NULL + * result columns if the caller asked for that. */ tupnatts = tupdesc->natts; tuplogcols = 0; /* we'll count nondeleted cols as we go */ colindex = 0; + newtlist = NIL; /* these are only used if modifyTargetList */ + junkattrs = NIL; foreach(lc, tlist) { @@ -1207,7 +1240,11 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList, Oid atttype; if (tle->resjunk) + { + if (modifyTargetList) + junkattrs = lappend(junkattrs, tle); continue; + } do { @@ -1219,6 +1256,26 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList, format_type_be(rettype)), errdetail("Final statement returns too many columns."))); attr = tupdesc->attrs[colindex - 1]; + if (attr->attisdropped && modifyTargetList) + { + Expr *null_expr; + + /* The type of the null we insert isn't important */ + null_expr = (Expr *) makeConst(INT4OID, + -1, + sizeof(int32), + (Datum) 0, + true, /* isnull */ + true /* byval */ ); + newtlist = lappend(newtlist, + makeTargetEntry(null_expr, + colindex, + NULL, + false)); + /* NULL insertion is dangerous in a setop */ + if (parse->setOperations) + *modifyTargetList = true; + } } while (attr->attisdropped); tuplogcols++; @@ -1233,28 +1290,66 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList, format_type_be(tletype), format_type_be(atttype), tuplogcols))); - if (insertRelabels && tletype != atttype) - tle->expr = (Expr *) makeRelabelType(tle->expr, - atttype, - -1, - COERCE_DONTCARE); + if (modifyTargetList) + { + if (tletype != atttype) + { + tle->expr = (Expr *) makeRelabelType(tle->expr, + atttype, + -1, + COERCE_DONTCARE); + /* Relabel is dangerous if sort/group or setop column */ + if (tle->ressortgroupref != 0 || parse->setOperations) + *modifyTargetList = true; + } + tle->resno = colindex; + newtlist = lappend(newtlist, tle); + } } - for (;;) + /* remaining columns in tupdesc had better all be dropped */ + for (colindex++; colindex <= tupnatts; colindex++) { - colindex++; - if (colindex > tupnatts) - break; if (!tupdesc->attrs[colindex - 1]->attisdropped) - tuplogcols++; + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("return type mismatch in function declared to return %s", + format_type_be(rettype)), + errdetail("Final statement returns too few columns."))); + if (modifyTargetList) + { + Expr *null_expr; + + /* The type of the null we insert isn't important */ + null_expr = (Expr *) makeConst(INT4OID, + -1, + sizeof(int32), + (Datum) 0, + true, /* isnull */ + true /* byval */ ); + newtlist = lappend(newtlist, + makeTargetEntry(null_expr, + colindex, + NULL, + false)); + /* NULL insertion is dangerous in a setop */ + if (parse->setOperations) + *modifyTargetList = true; + } } - if (tlistlen != tuplogcols) - ereport(ERROR, - (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), - errmsg("return type mismatch in function declared to return %s", - format_type_be(rettype)), - errdetail("Final statement returns too few columns."))); + if (modifyTargetList) + { + /* ensure resjunk columns are numbered correctly */ + foreach(lc, junkattrs) + { + TargetEntry *tle = (TargetEntry *) lfirst(lc); + + tle->resno = colindex++; + } + /* replace the tlist with the modified one */ + *tlist_ptr = list_concat(newtlist, junkattrs); + } /* Set up junk filter if needed */ if (junkFilter) diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index dcfc731a17a087ff7fb78a55e25d69f03cc73852..a5c343298e6b61df02e9a4485434722519ef6c88 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.279 2009/10/08 02:39:21 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.280 2009/12/14 02:15:52 tgl Exp $ * * HISTORY * AUTHOR DATE MAJOR EVENT @@ -3673,6 +3673,7 @@ inline_function(Oid funcid, Oid result_type, List *args, char *src; Datum tmp; bool isNull; + bool modifyTargetList; MemoryContext oldcxt; MemoryContext mycxt; ErrorContextCallback sqlerrcontext; @@ -3792,7 +3793,7 @@ inline_function(Oid funcid, Oid result_type, List *args, * needed; that's probably not important, but let's be careful. */ if (check_sql_fn_retval(funcid, result_type, list_make1(querytree), - true, NULL)) + &modifyTargetList, NULL)) goto fail; /* reject whole-tuple-result cases */ /* Now we can grab the tlist expression */ @@ -3800,6 +3801,8 @@ inline_function(Oid funcid, Oid result_type, List *args, /* Assert that check_sql_fn_retval did the right thing */ Assert(exprType(newexpr) == result_type); + /* It couldn't have made any dangerous tlist changes, either */ + Assert(!modifyTargetList); /* * Additional validity checks on the expression. It mustn't return a set, @@ -4088,6 +4091,7 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte) char *src; Datum tmp; bool isNull; + bool modifyTargetList; MemoryContext oldcxt; MemoryContext mycxt; ErrorContextCallback sqlerrcontext; @@ -4253,8 +4257,8 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte) * Make sure the function (still) returns what it's declared to. This * will raise an error if wrong, but that's okay since the function would * fail at runtime anyway. Note that check_sql_fn_retval will also insert - * RelabelType(s) if needed to make the tlist expression(s) match the - * declared type of the function. + * RelabelType(s) and/or NULL columns if needed to make the tlist + * expression(s) match the declared type of the function. * * If the function returns a composite type, don't inline unless the check * shows it's returning a whole tuple result; otherwise what it's @@ -4262,11 +4266,19 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte) */ if (!check_sql_fn_retval(func_oid, fexpr->funcresulttype, querytree_list, - true, NULL) && + &modifyTargetList, NULL) && (get_typtype(fexpr->funcresulttype) == TYPTYPE_COMPOSITE || fexpr->funcresulttype == RECORDOID)) goto fail; /* reject not-whole-tuple-result cases */ + /* + * If we had to modify the tlist to make it match, and the statement is + * one in which changing the tlist contents could change semantics, we + * have to punt and not inline. + */ + if (modifyTargetList) + goto fail; + /* * If it returns RECORD, we have to check against the column type list * provided in the RTE; check_sql_fn_retval can't do that. (If no match, diff --git a/src/include/executor/functions.h b/src/include/executor/functions.h index 10435f8617e792bf0ee07c25de34d613d5bcd687..ab0cf8e75ad9f3aeb1d745e93effb1a7e6cc9c9a 100644 --- a/src/include/executor/functions.h +++ b/src/include/executor/functions.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/executor/functions.h,v 1.33 2009/01/01 17:23:59 momjian Exp $ + * $PostgreSQL: pgsql/src/include/executor/functions.h,v 1.34 2009/12/14 02:15:54 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -22,7 +22,7 @@ extern Datum fmgr_sql(PG_FUNCTION_ARGS); extern bool check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList, - bool insertRelabels, + bool *modifyTargetList, JunkFilter **junkFilter); extern DestReceiver *CreateSQLFunctionDestReceiver(void); diff --git a/src/test/regress/expected/rangefuncs.out b/src/test/regress/expected/rangefuncs.out index 843bc53e4e75b7ee1f82f13654fc25d4194881ca..a32cbf5f79587eda538f89d0a746a62495d2509d 100644 --- a/src/test/regress/expected/rangefuncs.out +++ b/src/test/regress/expected/rangefuncs.out @@ -836,3 +836,64 @@ ERROR: a column definition list is required for functions returning "record" LINE 1: select * from testfoo(); ^ drop function testfoo(); +-- +-- Check some cases involving dropped columns in a rowtype result +-- +create temp table users (userid text, email text, todrop bool, enabled bool); +insert into users values ('id','email',true,true); +insert into users values ('id2','email2',true,true); +alter table users drop column todrop; +create or replace function get_first_user() returns users as +$$ SELECT * FROM users ORDER BY userid LIMIT 1; $$ +language sql stable; +SELECT get_first_user(); + get_first_user +---------------- + (id,email,t) +(1 row) + +SELECT * FROM get_first_user(); + userid | email | enabled +--------+-------+--------- + id | email | t +(1 row) + +create or replace function get_users() returns setof users as +$$ SELECT * FROM users ORDER BY userid; $$ +language sql stable; +SELECT get_users(); + get_users +---------------- + (id,email,t) + (id2,email2,t) +(2 rows) + +SELECT * FROM get_users(); + userid | email | enabled +--------+--------+--------- + id | email | t + id2 | email2 | t +(2 rows) + +drop function get_first_user(); +drop function get_users(); +drop table users; +-- this won't get inlined because of type coercion, but it shouldn't fail +create or replace function foobar() returns setof text as +$$ select 'foo'::varchar union all select 'bar'::varchar ; $$ +language sql stable; +select foobar(); + foobar +-------- + foo + bar +(2 rows) + +select * from foobar(); + foobar +-------- + foo + bar +(2 rows) + +drop function foobar(); diff --git a/src/test/regress/sql/rangefuncs.sql b/src/test/regress/sql/rangefuncs.sql index 172bbc73a9e53aa8fa2a15b35569c98e85e1f1f5..db74770228d291bcaa93b0efdccc349bb05f59c8 100644 --- a/src/test/regress/sql/rangefuncs.sql +++ b/src/test/regress/sql/rangefuncs.sql @@ -391,3 +391,41 @@ select * from testfoo() as t(f1 int8,f2 int8); select * from testfoo(); -- fail drop function testfoo(); + +-- +-- Check some cases involving dropped columns in a rowtype result +-- + +create temp table users (userid text, email text, todrop bool, enabled bool); +insert into users values ('id','email',true,true); +insert into users values ('id2','email2',true,true); +alter table users drop column todrop; + +create or replace function get_first_user() returns users as +$$ SELECT * FROM users ORDER BY userid LIMIT 1; $$ +language sql stable; + +SELECT get_first_user(); +SELECT * FROM get_first_user(); + +create or replace function get_users() returns setof users as +$$ SELECT * FROM users ORDER BY userid; $$ +language sql stable; + +SELECT get_users(); +SELECT * FROM get_users(); + +drop function get_first_user(); +drop function get_users(); +drop table users; + +-- this won't get inlined because of type coercion, but it shouldn't fail + +create or replace function foobar() returns setof text as +$$ select 'foo'::varchar union all select 'bar'::varchar ; $$ +language sql stable; + +select foobar(); +select * from foobar(); + +drop function foobar();