Move pg_checkretval out of the planner (where it never belonged) into
authorTom Lane
Mon, 21 Aug 2000 20:55:31 +0000 (20:55 +0000)
committerTom Lane
Mon, 21 Aug 2000 20:55:31 +0000 (20:55 +0000)
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
src/backend/executor/execQual.c
src/backend/optimizer/plan/planner.c
src/include/executor/executor.h
src/include/optimizer/planner.h

index ab6ce91e46d8e15fb034d9ec4ba60d0683b64ca2..98d0cdca0c4153f6a922094365b2ea9aba03bf02 100644 (file)
@@ -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);
+}
index 6e934a177251004bb706c2d08419c330a5b8e8bb..83117d836ebef83d7d5f9fb3233a40fb09c32708 100644 (file)
@@ -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
  *
index 1a9a7d36fa6de7d022fdfd3933392dd4458c7d0a..4be9b05bb900ee1ab7b7428c956b7ad33efa9f5b 100644 (file)
@@ -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);
-}
index 7849e87a78a398486cefe1f32c93819b028058ed..ea589e06dd95e9f8f9555aa56a59f8e21c4de3fc 100644 (file)
@@ -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);
 
 /*
index 3f4fc2a38e087bf4f165e2dab3475cd13af9a2cb..da099940e6ed8d068af7b02e541fa1c4f6f06ae7 100644 (file)
@@ -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 */