Tweak SPI_cursor_open to allow INSERT/UPDATE/DELETE RETURNING; this was

merely a matter of fixing the error check, since the underlying Portal
infrastructure already handles it.  This in turn allows these statements
to be used in some existing plpgsql and plperl contexts, such as a
plpgsql FOR loop.  Also, do some marginal code cleanup in places that
were being sloppy about distinguishing SELECT from SELECT INTO.

13 files changed, 100 insertions, 59 deletions

diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c
index 8b9dd99d4e4..0685426a2f2 100644
--- a/src/backend/commands/portalcmds.c
+++ b/src/backend/commands/portalcmds.c
@@ -14,7 +14,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/portalcmds.c,v 1.49 2006/08/08 01:23:15 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/portalcmds.c,v 1.50 2006/08/12 20:05:54 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -87,7 +87,7 @@ PerformCursorOpen(DeclareCursorStmt *stmt, ParamListInfo params)
 
 	if (query->into)
 		ereport(ERROR,
-				(errcode(ERRCODE_SYNTAX_ERROR),
+				(errcode(ERRCODE_INVALID_CURSOR_DEFINITION),
 				 errmsg("DECLARE CURSOR may not specify INTO")));
 	if (query->rowMarks != NIL)
 		ereport(ERROR,
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 05c4a542b84..4b8a166ffae 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -26,7 +26,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.278 2006/08/12 02:52:04 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.279 2006/08/12 20:05:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -497,7 +497,7 @@ InitPlan(QueryDesc *queryDesc, int eflags)
 	/*
 	 * if there is a result relation, initialize result relation stuff
 	 */
-	if (parseTree->resultRelation != 0 && operation != CMD_SELECT)
+	if (parseTree->resultRelation)
 	{
 		List	   *resultRelations = parseTree->resultRelations;
 		int			numResultRelations;
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index 0da2abba77c..28462ba8b8b 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.104 2006/07/14 14:52:19 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/functions.c,v 1.105 2006/08/12 20:05:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -361,7 +361,9 @@ postquel_getnext(execution_state *es)
 			 * run it to completion.  (If we run to completion then
 			 * ExecutorRun is guaranteed to return NULL.)
 			 */
-			if (LAST_POSTQUEL_COMMAND(es) && es->qd->operation == CMD_SELECT)
+			if (LAST_POSTQUEL_COMMAND(es) &&
+				es->qd->operation == CMD_SELECT &&
+				es->qd->parsetree->into == NULL)
 				count = 1L;
 			else
 				count = 0L;
@@ -868,7 +870,7 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList,
 					JunkFilter **junkFilter)
 {
 	Query	   *parse;
-	int			cmd;
+	bool		isSelect;
 	List	   *tlist;
 	ListCell   *tlistitem;
 	int			tlistlen;
@@ -893,15 +895,18 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList,
 	/* find the final query */
 	parse = (Query *) lfirst(list_tail(queryTreeList));
 
-	cmd = parse->commandType;
-	tlist = parse->targetList;
+	/*
+	 * Note: eventually replace this with QueryReturnsTuples?  We'd need
+	 * a more general method of determining the output type, though.
+	 */
+	isSelect = (parse->commandType == CMD_SELECT && parse->into == NULL);
 
 	/*
 	 * The last query must be a SELECT if and only if return type isn't VOID.
 	 */
 	if (rettype == VOIDOID)
 	{
-		if (cmd == CMD_SELECT)
+		if (isSelect)
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
 			 errmsg("return type mismatch in function declared to return %s",
@@ -911,7 +916,7 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList,
 	}
 
 	/* by here, the function is declared to return some type */
-	if (cmd != CMD_SELECT)
+	if (!isSelect)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
 			 errmsg("return type mismatch in function declared to return %s",
@@ -921,6 +926,7 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList,
 	/*
 	 * Count the non-junk entries in the result targetlist.
 	 */
+	tlist = parse->targetList;
 	tlistlen = ExecCleanTargetListLength(tlist);
 
 	fn_typtype = get_typtype(rettype);
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index c1f39f7c4f7..6c4cc585460 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.154 2006/08/12 02:52:04 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.155 2006/08/12 20:05:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -837,26 +837,12 @@ SPI_cursor_open(const char *name, void *plan,
 	planTree = (Plan *) linitial(ptlist);
 
 	/* Must be a query that returns tuples */
-	switch (queryTree->commandType)
-	{
-		case CMD_SELECT:
-			if (queryTree->into != NULL)
-				ereport(ERROR,
-						(errcode(ERRCODE_INVALID_CURSOR_DEFINITION),
-						 errmsg("cannot open SELECT INTO query as cursor")));
-			break;
-		case CMD_UTILITY:
-			if (!UtilityReturnsTuples(queryTree->utilityStmt))
-				ereport(ERROR,
-						(errcode(ERRCODE_INVALID_CURSOR_DEFINITION),
-						 errmsg("cannot open non-SELECT query as cursor")));
-			break;
-		default:
-			ereport(ERROR,
-					(errcode(ERRCODE_INVALID_CURSOR_DEFINITION),
-					 errmsg("cannot open non-SELECT query as cursor")));
-			break;
-	}
+	if (!QueryReturnsTuples(queryTree))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_CURSOR_DEFINITION),
+				 /* translator: %s is name of a SQL command, eg INSERT */
+				 errmsg("cannot open %s query as cursor",
+						CreateQueryTag(queryTree))));
 
 	/* Reset SPI result (note we deliberately don't touch lastoid) */
 	SPI_processed = 0;
@@ -876,7 +862,7 @@ SPI_cursor_open(const char *name, void *plan,
 		portal = CreatePortal(name, false, false);
 	}
 
-	/* Switch to portals memory and copy the parsetree and plan to there */
+	/* Switch to portal's memory and copy the parsetree and plan to there */
 	oldcontext = MemoryContextSwitchTo(PortalGetHeapMemory(portal));
 	queryTree = copyObject(queryTree);
 	planTree = copyObject(planTree);
@@ -919,9 +905,9 @@ SPI_cursor_open(const char *name, void *plan,
 	 * Set up the portal.
 	 */
 	PortalDefineQuery(portal,
-					  NULL,
+					  NULL,		/* no statement name */
 					  spiplan->query,
-					  "SELECT", /* don't have the raw parse tree... */
+					  CreateQueryTag(queryTree),
 					  list_make1(queryTree),
 					  list_make1(planTree),
 					  PortalGetHeapMemory(portal));
@@ -954,9 +940,16 @@ SPI_cursor_open(const char *name, void *plan,
 	 */
 	PortalStart(portal, paramLI, snapshot);
 
-	Assert(portal->strategy == PORTAL_ONE_SELECT ||
-		   portal->strategy == PORTAL_ONE_RETURNING ||
-		   portal->strategy == PORTAL_UTIL_SELECT);
+	/*
+	 * If this test fails then we're out of sync with pquery.c about
+	 * which queries can return tuples...
+	 */
+	if (portal->strategy == PORTAL_MULTI_QUERY)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_CURSOR_DEFINITION),
+				 /* translator: %s is name of a SQL command, eg INSERT */
+				 errmsg("cannot open %s query as cursor",
+						CreateQueryTag(queryTree))));
 
 	/* Return the created portal */
 	return portal;
@@ -1046,12 +1039,12 @@ SPI_getargcount(void *plan)
 
 /*
  * Returns true if the plan contains exactly one command
- * and that command originates from normal SELECT (i.e.
- * *not* a SELECT ... INTO). In essence, the result indicates
- * if the command can be used with SPI_cursor_open
+ * and that command returns tuples to the caller (eg, SELECT or
+ * INSERT ... RETURNING, but not SELECT ... INTO). In essence,
+ * the result indicates if the command can be used with SPI_cursor_open
  *
  * Parameters
- *	  plan A plan previously prepared using SPI_prepare
+ *	  plan: A plan previously prepared using SPI_prepare
  */
 bool
 SPI_is_cursor_plan(void *plan)
@@ -1070,7 +1063,7 @@ SPI_is_cursor_plan(void *plan)
 	{
 		Query	   *queryTree = (Query *) linitial((List *) linitial(qtlist));
 
-		if (queryTree->commandType == CMD_SELECT && queryTree->into == NULL)
+		if (QueryReturnsTuples(queryTree))
 			return true;
 	}
 	return false;
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index ad128605dcc..ef92d9d3c9f 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -15,7 +15,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/prep/prepjointree.c,v 1.41 2006/08/12 02:52:05 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/prep/prepjointree.c,v 1.42 2006/08/12 20:05:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -623,7 +623,6 @@ is_simple_subquery(Query *subquery)
 	 */
 	if (!IsA(subquery, Query) ||
 		subquery->commandType != CMD_SELECT ||
-		subquery->resultRelation != 0 ||
 		subquery->into != NULL)
 		elog(ERROR, "subquery is bogus");
 
@@ -686,7 +685,6 @@ is_simple_union_all(Query *subquery)
 	/* Let's just make sure it's a valid subselect ... */
 	if (!IsA(subquery, Query) ||
 		subquery->commandType != CMD_SELECT ||
-		subquery->resultRelation != 0 ||
 		subquery->into != NULL)
 		elog(ERROR, "subquery is bogus");
 
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 04f346d4625..71e727a7b4c 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.218 2006/08/12 02:52:05 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.219 2006/08/12 20:05:55 tgl Exp $
  *
  * HISTORY
  *	  AUTHOR			DATE			MAJOR EVENT
@@ -2689,7 +2689,6 @@ inline_function(Oid funcid, Oid result_type, List *args,
 	 */
 	if (!IsA(querytree, Query) ||
 		querytree->commandType != CMD_SELECT ||
-		querytree->resultRelation != 0 ||
 		querytree->into ||
 		querytree->hasAggs ||
 		querytree->hasSubLinks ||
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 61242ef712e..39c7372733f 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *	$PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.345 2006/08/12 02:52:05 tgl Exp $
+ *	$PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.346 2006/08/12 20:05:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -3116,6 +3116,15 @@ transformDeclareCursorStmt(ParseState *pstate, DeclareCursorStmt *stmt)
 	/* Shouldn't get any extras, since grammar only allows SelectStmt */
 	if (extras_before || extras_after)
 		elog(ERROR, "unexpected extra stuff in cursor statement");
+	if (!IsA(stmt->query, Query) ||
+		((Query *) stmt->query)->commandType != CMD_SELECT)
+		elog(ERROR, "unexpected non-SELECT command in cursor statement");
+
+	/* But we must explicitly disallow DECLARE CURSOR ... SELECT INTO */
+	if (((Query *) stmt->query)->into)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_CURSOR_DEFINITION),
+				 errmsg("DECLARE CURSOR may not specify INTO")));
 
 	return result;
 }
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index d34529c74ee..021fb3fa553 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/parser/parse_clause.c,v 1.155 2006/07/26 19:31:51 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/parser/parse_clause.c,v 1.156 2006/08/12 20:05:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -447,7 +447,7 @@ transformRangeSubselect(ParseState *pstate, RangeSubselect *r)
 
 	if (query->commandType != CMD_SELECT)
 		elog(ERROR, "expected SELECT query from subquery in FROM");
-	if (query->resultRelation != 0 || query->into != NULL)
+	if (query->into != NULL)
 		ereport(ERROR,
 				(errcode(ERRCODE_SYNTAX_ERROR),
 				 errmsg("subquery in FROM may not have SELECT INTO")));
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 6a7117e98bf..0812c3d4418 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.196 2006/08/02 01:59:46 joe Exp $
+ *	  $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.197 2006/08/12 20:05:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1104,7 +1104,7 @@ transformSubLink(ParseState *pstate, SubLink *sublink)
 		elog(ERROR, "bad query in sub-select");
 	qtree = (Query *) linitial(qtrees);
 	if (qtree->commandType != CMD_SELECT ||
-		qtree->resultRelation != 0)
+		qtree->into != NULL)
 		elog(ERROR, "bad query in sub-select");
 	sublink->subselect = (Node *) qtree;
 
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index 3e1f65d8681..df24c6751c2 100644
--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/rewrite/rewriteDefine.c,v 1.111 2006/07/18 17:42:00 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/rewrite/rewriteDefine.c,v 1.112 2006/08/12 20:05:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -264,7 +264,8 @@ DefineQueryRewrite(RuleStmt *stmt)
 		 * ... the one action must be a SELECT, ...
 		 */
 		query = (Query *) linitial(action);
-		if (!is_instead || query->commandType != CMD_SELECT)
+		if (!is_instead ||
+			query->commandType != CMD_SELECT || query->into != NULL)
 			ereport(ERROR,
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("rules on SELECT must have action INSTEAD SELECT")));
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 807b64360c8..7d6941ddcef 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.264 2006/08/12 02:52:05 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.265 2006/08/12 20:05:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1225,6 +1225,38 @@ UtilityTupleDescriptor(Node *parsetree)
 
 
 /*
+ * QueryReturnsTuples
+ *		Return "true" if this Query will send output to the destination.
+ */
+bool
+QueryReturnsTuples(Query *parsetree)
+{
+	switch (parsetree->commandType)
+	{
+		case CMD_SELECT:
+			/* returns tuples ... unless it's SELECT INTO */
+			if (parsetree->into == NULL)
+				return true;
+			break;
+		case CMD_INSERT:
+		case CMD_UPDATE:
+		case CMD_DELETE:
+			/* the forms with RETURNING return tuples */
+			if (parsetree->returningList)
+				return true;
+			break;
+		case CMD_UTILITY:
+			return UtilityReturnsTuples(parsetree->utilityStmt);
+		case CMD_UNKNOWN:
+		case CMD_NOTHING:
+			/* probably shouldn't get here */
+			break;
+	}
+	return false;				/* default */
+}
+
+
+/*
  * CreateCommandTag
  *		utility to get a string representation of the
  *		command operation, given a raw (un-analyzed) parsetree.
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index f0f40e002e1..222f5d26cc3 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.322 2006/08/12 02:52:06 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.323 2006/08/12 20:05:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -89,7 +89,8 @@ typedef struct Query
 	Node	   *utilityStmt;	/* non-null if this is a non-optimizable
 								 * statement */
 
-	int			resultRelation; /* target relation (index into rtable) */
+	int			resultRelation; /* rtable index of target relation for
+								 * INSERT/UPDATE/DELETE; 0 for SELECT */
 
 	RangeVar   *into;			/* target relation for SELECT INTO */
 	List	   *intoOptions;		/* options from WITH clause */
diff --git a/src/include/tcop/utility.h b/src/include/tcop/utility.h
index 09f96a262c7..6674dec129e 100644
--- a/src/include/tcop/utility.h
+++ b/src/include/tcop/utility.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/tcop/utility.h,v 1.27 2006/03/05 15:59:00 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/tcop/utility.h,v 1.28 2006/08/12 20:05:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -28,6 +28,8 @@ extern const char *CreateCommandTag(Node *parsetree);
 
 extern const char *CreateQueryTag(Query *parsetree);
 
+extern bool QueryReturnsTuples(Query *parsetree);
+
 extern bool QueryIsReadOnly(Query *parsetree);
 
 extern void CheckRelationOwnership(RangeVar *rel, bool noCatalogs);