From 8efbe30df51a1fb3064195dfaf5189d1a9c5eee9 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 2 Apr 2004 23:14:08 +0000 Subject: [PATCH] check_sql_fn_retval has always thought that we supported doing 'SELECT foo()' in a SQL function returning a rowtype, to simply pass back the results of another function returning the same rowtype. However, that hasn't actually worked in many years. Now it works again. --- src/backend/catalog/pg_proc.c | 43 ++++++++++++++++----- src/backend/executor/functions.c | 56 +++++++++++++++------------- src/backend/optimizer/util/clauses.c | 6 +-- src/include/catalog/pg_proc.h | 4 +- 4 files changed, 68 insertions(+), 41 deletions(-) diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c index 0640aacbe5..749ad44127 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.114 2004/04/01 21:28:44 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/catalog/pg_proc.c,v 1.115 2004/04/02 23:14:05 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -362,8 +362,13 @@ create_parameternames_array(int parameterCount, const char *parameterNames[]) * function execution startup. The rettype is then the actual resolved * output type of the function, rather than the declared type. (Therefore, * we should never see ANYARRAY or ANYELEMENT as rettype.) + * + * The return value is true if the function returns the entire tuple result + * of its final SELECT, 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. */ -void +bool check_sql_fn_retval(Oid rettype, char fn_typtype, List *queryTreeList) { Query *parse; @@ -387,7 +392,7 @@ check_sql_fn_retval(Oid rettype, char fn_typtype, List *queryTreeList) errmsg("return type mismatch in function declared to return %s", format_type_be(rettype)), errdetail("Function's final statement must be a SELECT."))); - return; + return false; } /* find the final query */ @@ -408,7 +413,7 @@ check_sql_fn_retval(Oid rettype, char fn_typtype, List *queryTreeList) errmsg("return type mismatch in function declared to return %s", format_type_be(rettype)), errdetail("Function's final statement must not be a SELECT."))); - return; + return false; } /* by here, the function is declared to return some type */ @@ -468,7 +473,7 @@ check_sql_fn_retval(Oid rettype, char fn_typtype, List *queryTreeList) { restype = ((TargetEntry *) lfirst(tlist))->resdom->restype; if (IsBinaryCoercible(restype, rettype)) - return; + return false; /* NOT returning whole tuple */ } /* @@ -536,16 +541,31 @@ check_sql_fn_retval(Oid rettype, char fn_typtype, List *queryTreeList) errdetail("Final SELECT returns too few columns."))); relation_close(reln, AccessShareLock); + + /* Report that we are returning entire tuple result */ + return true; } else if (rettype == RECORDOID) { - /* Shouldn't have a typerelid */ - Assert(typerelid == InvalidOid); + /* + * If the target list is of length 1, and the type of the varnode + * in the target list matches the declared return type, this is + * okay. This can happen, for example, where the body of the + * function is 'SELECT func2()', where func2 has the same return + * type as the function that's calling it. + */ + if (tlistlen == 1) + { + restype = ((TargetEntry *) lfirst(tlist))->resdom->restype; + if (IsBinaryCoercible(restype, rettype)) + return false; /* NOT returning whole tuple */ + } /* - * For RECORD return type, defer this check until we get the first - * tuple. + * Otherwise assume we are returning the whole tuple. Crosschecking + * against what the caller expects will happen at runtime. */ + return true; } else if (rettype == ANYARRAYOID || rettype == ANYELEMENTOID) { @@ -560,6 +580,8 @@ check_sql_fn_retval(Oid rettype, char fn_typtype, List *queryTreeList) (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), errmsg("return type %s is not supported for SQL functions", format_type_be(rettype)))); + + return false; } @@ -751,7 +773,8 @@ fmgr_sql_validator(PG_FUNCTION_ARGS) querytree_list = pg_parse_and_rewrite(prosrc, proc->proargtypes, proc->pronargs); - check_sql_fn_retval(proc->prorettype, functyptype, querytree_list); + (void) check_sql_fn_retval(proc->prorettype, functyptype, + querytree_list); } else querytree_list = pg_parse_query(prosrc); diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c index aa7652e07e..c07eac5faf 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.79 2004/04/01 21:28:44 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/functions.c,v 1.80 2004/04/02 23:14:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -73,9 +73,7 @@ typedef SQLFunctionCache *SQLFunctionCachePtr; /* non-export function prototypes */ -static execution_state *init_execution_state(char *src, - Oid *argOidVect, int nargs, - Oid rettype, bool haspolyarg); +static execution_state *init_execution_state(List *queryTree_list); static void init_sql_fcache(FmgrInfo *finfo); static void postquel_start(execution_state *es, SQLFunctionCachePtr fcache); static TupleTableSlot *postquel_getnext(execution_state *es); @@ -90,25 +88,11 @@ static void ShutdownSQLFunction(Datum arg); static execution_state * -init_execution_state(char *src, Oid *argOidVect, int nargs, - Oid rettype, bool haspolyarg) +init_execution_state(List *queryTree_list) { - execution_state *firstes; - execution_state *preves; - List *queryTree_list, - *qtl_item; - - queryTree_list = pg_parse_and_rewrite(src, argOidVect, nargs); - - /* - * If the function has any arguments declared as polymorphic types, - * then it wasn't type-checked at definition time; must do so now. - */ - if (haspolyarg) - check_sql_fn_retval(rettype, get_typtype(rettype), queryTree_list); - - firstes = NULL; - preves = NULL; + execution_state *firstes = NULL; + execution_state *preves = NULL; + List *qtl_item; foreach(qtl_item, queryTree_list) { @@ -151,6 +135,7 @@ init_sql_fcache(FmgrInfo *finfo) bool haspolyarg; char *src; int nargs; + List *queryTree_list; Datum tmp; bool isNull; @@ -191,7 +176,9 @@ init_sql_fcache(FmgrInfo *finfo) typeStruct = (Form_pg_type) GETSTRUCT(typeTuple); /* - * get the type length and by-value flag from the type tuple + * get the type length and by-value flag from the type tuple; also + * do a preliminary check for returnsTuple (this may prove inaccurate, + * see below). */ fcache->typlen = typeStruct->typlen; fcache->typbyval = typeStruct->typbyval; @@ -199,7 +186,7 @@ init_sql_fcache(FmgrInfo *finfo) rettype == RECORDOID); /* - * Parse and plan the queries. We need the argument type info to pass + * Parse and rewrite the queries. We need the argument type info to pass * to the parser. */ nargs = procedureStruct->pronargs; @@ -242,8 +229,25 @@ init_sql_fcache(FmgrInfo *finfo) elog(ERROR, "null prosrc for function %u", foid); src = DatumGetCString(DirectFunctionCall1(textout, tmp)); - fcache->func_state = init_execution_state(src, argOidVect, nargs, - rettype, haspolyarg); + queryTree_list = pg_parse_and_rewrite(src, argOidVect, nargs); + + /* + * If the function has any arguments declared as polymorphic types, + * then it wasn't type-checked at definition time; must do so now. + * + * Also, force a type-check if the declared return type is a rowtype; + * we need to find out whether we are actually returning the whole + * tuple result, or just regurgitating a rowtype expression result. + * In the latter case we clear returnsTuple because we need not act + * different from the scalar result case. + */ + if (haspolyarg || fcache->returnsTuple) + fcache->returnsTuple = check_sql_fn_retval(rettype, + get_typtype(rettype), + queryTree_list); + + /* Finally, plan the queries */ + fcache->func_state = init_execution_state(queryTree_list); pfree(src); diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 6aea0811bb..f653640ee8 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.168 2004/04/02 19:06:57 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.169 2004/04/02 23:14:08 tgl Exp $ * * HISTORY * AUTHOR DATE MAJOR EVENT @@ -1992,8 +1992,8 @@ inline_function(Oid funcid, Oid result_type, List *args, * probably not important, but let's be careful.) */ if (polymorphic) - check_sql_fn_retval(result_type, get_typtype(result_type), - querytree_list); + (void) check_sql_fn_retval(result_type, get_typtype(result_type), + querytree_list); /* * Additional validity checks on the expression. It mustn't return a diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h index 7da13b33c9..5db922084f 100644 --- a/src/include/catalog/pg_proc.h +++ b/src/include/catalog/pg_proc.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/catalog/pg_proc.h,v 1.323 2004/04/01 21:28:45 tgl Exp $ + * $PostgreSQL: pgsql/src/include/catalog/pg_proc.h,v 1.324 2004/04/02 23:14:08 tgl Exp $ * * NOTES * The script catalog/genbki.sh reads this file and generates .bki @@ -3560,7 +3560,7 @@ extern Oid ProcedureCreate(const char *procedureName, const Oid *parameterTypes, const char *parameterNames[]); -extern void check_sql_fn_retval(Oid rettype, char fn_typtype, +extern bool check_sql_fn_retval(Oid rettype, char fn_typtype, List *queryTreeList); extern bool function_parse_error_transpose(const char *prosrc);