From 7893462e44756a0ab110e43b4c5c41a5e96e6883 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 21 Aug 2000 20:55:31 +0000 Subject: [PATCH] Move pg_checkretval out of the planner (where it never belonged) into pg_proc.c (where it's actually used). Fix it to correctly handle tlists that contain resjunk target items, and improve error messages. This addresses bug reported by Krupnikov 6-July-00. --- src/backend/catalog/pg_proc.c | 130 ++++++++++++++++++++++++- src/backend/executor/execQual.c | 37 +++++++- src/backend/optimizer/plan/planner.c | 136 +-------------------------- src/include/executor/executor.h | 3 +- src/include/optimizer/planner.h | 7 +- 5 files changed, 164 insertions(+), 149 deletions(-) diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c index ab6ce91e46..98d0cdca0c 100644 --- a/src/backend/catalog/pg_proc.c +++ b/src/backend/catalog/pg_proc.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/catalog/pg_proc.c,v 1.47 2000/08/21 17:22:35 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/catalog/pg_proc.c,v 1.48 2000/08/21 20:55:31 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -20,8 +20,9 @@ #include "catalog/pg_language.h" #include "catalog/pg_proc.h" #include "catalog/pg_type.h" +#include "executor/executor.h" #include "miscadmin.h" -#include "optimizer/planner.h" +#include "parser/parse_expr.h" #include "parser/parse_type.h" #include "tcop/tcopprot.h" #include "utils/builtins.h" @@ -30,6 +31,9 @@ #include "utils/syscache.h" +static void checkretval(Oid rettype, List *queryTreeList); + + /* ---------------------------------------------------------------- * ProcedureCreate * ---------------------------------------------------------------- @@ -220,7 +224,7 @@ ProcedureCreate(char *procedureName, { querytree_list = pg_parse_and_rewrite(prosrc, typev, parameterCount); /* typecheck return value */ - pg_checkretval(typeObjectId, querytree_list); + checkretval(typeObjectId, querytree_list); } /* @@ -334,3 +338,123 @@ ProcedureCreate(char *procedureName, heap_freetuple(tup); return retval; } + +/* + * checkretval() -- check return value of a list of sql parse trees. + * + * The return value of a sql function is the value returned by + * the final query in the function. We do some ad-hoc define-time + * type checking here to be sure that the user is returning the + * type he claims. + */ +static void +checkretval(Oid rettype, List *queryTreeList) +{ + Query *parse; + int cmd; + List *tlist; + List *tlistitem; + int tlistlen; + Type typ; + Resdom *resnode; + Relation reln; + Oid relid; + int relnatts; + int i; + + /* find the final query */ + parse = (Query *) nth(length(queryTreeList) - 1, queryTreeList); + + cmd = parse->commandType; + tlist = parse->targetList; + + /* + * The last query must be a SELECT if and only if there is a return type. + */ + if (rettype == InvalidOid) + { + if (cmd == CMD_SELECT) + elog(ERROR, "function declared with no return type, but final query is a SELECT"); + return; + } + + /* by here, the function is declared to return some type */ + if ((typ = typeidType(rettype)) == NULL) + elog(ERROR, "can't find return type %u for function", rettype); + + if (cmd != CMD_SELECT) + elog(ERROR, "function declared to return %s, but final query is not a SELECT", typeTypeName(typ)); + + /* + * Count the non-junk entries in the result targetlist. + */ + tlistlen = ExecCleanTargetListLength(tlist); + + /* + * For base-type returns, the target list should have exactly one entry, + * and its type should agree with what the user declared. + */ + if (typeTypeRelid(typ) == InvalidOid) + { + if (tlistlen != 1) + elog(ERROR, "function declared to return %s returns multiple columns in final SELECT", typeTypeName(typ)); + + resnode = (Resdom *) ((TargetEntry *) lfirst(tlist))->resdom; + if (resnode->restype != rettype) + elog(ERROR, "return type mismatch in function: declared to return %s, returns %s", typeTypeName(typ), typeidTypeName(resnode->restype)); + + return; + } + + /* + * If the target list is of length 1, and the type of the varnode in + * the target list is the same as the declared return type, this is + * okay. This can happen, for example, where the body of the function + * is 'SELECT (x = func2())', where func2 has the same return type + * as the function that's calling it. + */ + if (tlistlen == 1) + { + resnode = (Resdom *) ((TargetEntry *) lfirst(tlist))->resdom; + if (resnode->restype == rettype) + return; + } + + /* + * By here, the procedure returns a tuple or set of tuples. This part of + * the typechecking is a hack. We look up the relation that is the + * declared return type, and be sure that attributes 1 .. n in the target + * list match the declared types. + */ + reln = heap_open(typeTypeRelid(typ), AccessShareLock); + relid = reln->rd_id; + relnatts = reln->rd_rel->relnatts; + + if (tlistlen != relnatts) + elog(ERROR, "function declared to return %s does not SELECT the right number of columns (%d)", typeTypeName(typ), relnatts); + + /* expect attributes 1 .. n in order */ + i = 0; + foreach(tlistitem, tlist) + { + TargetEntry *tle = (TargetEntry *) lfirst(tlistitem); + Oid tletype; + + if (tle->resdom->resjunk) + continue; + tletype = exprType(tle->expr); + if (tletype != reln->rd_att->attrs[i]->atttypid) + elog(ERROR, "function declared to return %s returns %s instead of %s at column %d", + typeTypeName(typ), + typeidTypeName(tletype), + typeidTypeName(reln->rd_att->attrs[i]->atttypid), + i+1); + i++; + } + + /* this shouldn't happen, but let's just check... */ + if (i != relnatts) + elog(ERROR, "function declared to return %s does not SELECT the right number of columns (%d)", typeTypeName(typ), relnatts); + + heap_close(reln, AccessShareLock); +} diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c index 6e934a1772..83117d836e 100644 --- a/src/backend/executor/execQual.c +++ b/src/backend/executor/execQual.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/executor/execQual.c,v 1.77 2000/08/08 15:41:22 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/executor/execQual.c,v 1.78 2000/08/21 20:55:30 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1444,17 +1444,18 @@ ExecQual(List *qual, ExprContext *econtext, bool resultForNull) return result; } +/* + * Number of items in a tlist (including any resjunk items!) + */ int ExecTargetListLength(List *targetlist) { - int len; + int len = 0; List *tl; - TargetEntry *curTle; - len = 0; foreach(tl, targetlist) { - curTle = lfirst(tl); + TargetEntry *curTle = (TargetEntry *) lfirst(tl); if (curTle->resdom != NULL) len++; @@ -1464,6 +1465,32 @@ ExecTargetListLength(List *targetlist) return len; } +/* + * Number of items in a tlist, not including any resjunk items + */ +int +ExecCleanTargetListLength(List *targetlist) +{ + int len = 0; + List *tl; + + foreach(tl, targetlist) + { + TargetEntry *curTle = (TargetEntry *) lfirst(tl); + + if (curTle->resdom != NULL) + { + if (! curTle->resdom->resjunk) + len++; + } + else + { + len += curTle->fjoin->fj_nNodes; + } + } + return len; +} + /* ---------------------------------------------------------------- * ExecTargetList * diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 1a9a7d36fa..4be9b05bb9 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -8,21 +8,17 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.87 2000/08/08 15:41:38 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.88 2000/08/21 20:55:29 tgl Exp $ * *------------------------------------------------------------------------- */ -#include #include "postgres.h" -#include "access/heapam.h" #include "catalog/pg_type.h" -#include "executor/executor.h" #include "nodes/makefuncs.h" #include "optimizer/clauses.h" #include "optimizer/paths.h" -#include "optimizer/plancat.h" #include "optimizer/planmain.h" #include "optimizer/planner.h" #include "optimizer/prep.h" @@ -30,7 +26,6 @@ #include "optimizer/tlist.h" #include "optimizer/var.h" #include "parser/parse_expr.h" -#include "parser/parse_type.h" #include "utils/lsyscache.h" @@ -871,132 +866,3 @@ make_sortplan(List *tlist, Plan *plannode, List *sortcls) return (Plan *) make_sort(sort_tlist, plannode, keyno); } - -/* - * pg_checkretval() -- check return value of a list of sql parse - * trees. - * - * The return value of a sql function is the value returned by - * the final query in the function. We do some ad-hoc define-time - * type checking here to be sure that the user is returning the - * type he claims. - * - * XXX Why is this function in this module? - */ -void -pg_checkretval(Oid rettype, List *queryTreeList) -{ - Query *parse; - List *tlist; - List *rt; - int cmd; - Type typ; - Resdom *resnode; - Relation reln; - Oid relid; - int relnatts; - int i; - - /* find the final query */ - parse = (Query *) nth(length(queryTreeList) - 1, queryTreeList); - - /* - * test 1: if the last query is a utility invocation, then there had - * better not be a return value declared. - */ - if (parse->commandType == CMD_UTILITY) - { - if (rettype == InvalidOid) - return; - else - elog(ERROR, "return type mismatch in function decl: final query is a catalog utility"); - } - - /* okay, it's an ordinary query */ - tlist = parse->targetList; - rt = parse->rtable; - cmd = parse->commandType; - - /* - * test 2: if the function is declared to return no value, then the - * final query had better not be a retrieve. - */ - if (rettype == InvalidOid) - { - if (cmd == CMD_SELECT) - elog(ERROR, - "function declared with no return type, but final query is a retrieve"); - else - return; - } - - /* by here, the function is declared to return some type */ - if ((typ = typeidType(rettype)) == NULL) - elog(ERROR, "can't find return type %u for function\n", rettype); - - /* - * test 3: if the function is declared to return a value, then the - * final query had better be a retrieve. - */ - if (cmd != CMD_SELECT) - elog(ERROR, "function declared to return type %s, but final query is not a retrieve", typeTypeName(typ)); - - /* - * test 4: for base type returns, the target list should have exactly - * one entry, and its type should agree with what the user declared. - */ - - if (typeTypeRelid(typ) == InvalidOid) - { - if (ExecTargetListLength(tlist) > 1) - elog(ERROR, "function declared to return %s returns multiple values in final retrieve", typeTypeName(typ)); - - resnode = (Resdom *) ((TargetEntry *) lfirst(tlist))->resdom; - if (resnode->restype != rettype) - elog(ERROR, "return type mismatch in function: declared to return %s, returns %s", typeTypeName(typ), typeidTypeName(resnode->restype)); - - /* by here, base return types match */ - return; - } - - /* - * If the target list is of length 1, and the type of the varnode in - * the target list is the same as the declared return type, this is - * okay. This can happen, for example, where the body of the function - * is 'retrieve (x = func2())', where func2 has the same return type - * as the function that's calling it. - */ - if (ExecTargetListLength(tlist) == 1) - { - resnode = (Resdom *) ((TargetEntry *) lfirst(tlist))->resdom; - if (resnode->restype == rettype) - return; - } - - /* - * By here, the procedure returns a (set of) tuples. This part of the - * typechecking is a hack. We look up the relation that is the - * declared return type, and be sure that attributes 1 .. n in the - * target list match the declared types. - */ - reln = heap_open(typeTypeRelid(typ), AccessShareLock); - relid = reln->rd_id; - relnatts = reln->rd_rel->relnatts; - - if (ExecTargetListLength(tlist) != relnatts) - elog(ERROR, "function declared to return type %s does not retrieve (%s.*)", typeTypeName(typ), typeTypeName(typ)); - - /* expect attributes 1 .. n in order */ - for (i = 1; i <= relnatts; i++) - { - TargetEntry *tle = lfirst(tlist); - Node *thenode = tle->expr; - Oid tletype = exprType(thenode); - - if (tletype != reln->rd_att->attrs[i - 1]->atttypid) - elog(ERROR, "function declared to return type %s does not retrieve (%s.all)", typeTypeName(typ), typeTypeName(typ)); - tlist = lnext(tlist); - } - - heap_close(reln, AccessShareLock); -} diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 7849e87a78..ea589e06dd 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2000, PostgreSQL, Inc * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: executor.h,v 1.47 2000/08/06 04:26:27 tgl Exp $ + * $Id: executor.h,v 1.48 2000/08/21 20:55:29 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -84,6 +84,7 @@ extern Datum ExecEvalExprSwitchContext(Node *expression, ExprContext *econtext, bool *isNull, bool *isDone); extern bool ExecQual(List *qual, ExprContext *econtext, bool resultForNull); extern int ExecTargetListLength(List *targetlist); +extern int ExecCleanTargetListLength(List *targetlist); extern TupleTableSlot *ExecProject(ProjectionInfo *projInfo, bool *isDone); /* diff --git a/src/include/optimizer/planner.h b/src/include/optimizer/planner.h index 3f4fc2a38e..da099940e6 100644 --- a/src/include/optimizer/planner.h +++ b/src/include/optimizer/planner.h @@ -7,22 +7,19 @@ * Portions Copyright (c) 1996-2000, PostgreSQL, Inc * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: planner.h,v 1.15 2000/03/21 05:11:51 tgl Exp $ + * $Id: planner.h,v 1.16 2000/08/21 20:55:28 tgl Exp $ * *------------------------------------------------------------------------- */ #ifndef PLANNER_H #define PLANNER_H -/* -*/ - #include "nodes/parsenodes.h" #include "nodes/plannodes.h" + extern Plan *planner(Query *parse); extern Plan *subquery_planner(Query *parse, double tuple_fraction); extern Plan *union_planner(Query *parse, double tuple_fraction); -extern void pg_checkretval(Oid rettype, List *querytree_list); #endif /* PLANNER_H */