Add "USING expressions" option to plpgsql's OPEN cursor FOR EXECUTE.

This is the last EXECUTE-like plpgsql statement that was missing
the capability of inserting parameter values via USING.

Pavel Stehule, reviewed by Itagaki Takahiro

doc/src/sgml/plpgsql.sgml

@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/plpgsql.sgml,v 1.149 2009/12/28 19:11:51 petere Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/plpgsql.sgml,v 1.150 2010/01/19 01:35:30 tgl Exp $ -->
 
 <chapter id="plpgsql">
   <title><application>PL/pgSQL</application> - <acronym>SQL</acronym> Procedural Language</title>
@@ -2495,7 +2495,7 @@ OPEN curs1 FOR SELECT * FROM foo WHERE key = mykey;
      <title><command>OPEN FOR EXECUTE</command></title>
 
 <synopsis>
-OPEN <replaceable>unbound_cursorvar</replaceable> <optional> <optional> NO </optional> SCROLL </optional> FOR EXECUTE <replaceable class="command">query_string</replaceable>;
+OPEN <replaceable>unbound_cursorvar</replaceable> <optional> <optional> NO </optional> SCROLL </optional> FOR EXECUTE <replaceable class="command">query_string</replaceable> <optional> USING <replaceable>expression</replaceable> <optional>, ... </optional> </optional>;
 </synopsis>
 
          <para>
@@ -2507,7 +2507,8 @@ OPEN <replaceable>unbound_cursorvar</replaceable> <optional> <optional> NO </opt
           command.  As usual, this gives flexibility so the query plan can vary
           from one run to the next (see <xref linkend="plpgsql-plan-caching">),
           and it also means that variable substitution is not done on the
-          command string.
+          command string. As with <command>EXECUTE</command>, parameter values
+          can be inserted into the dynamic command via <literal>USING</>.
           The <literal>SCROLL</> and
           <literal>NO SCROLL</> options have the same meanings as for a bound
           cursor.
@@ -2516,8 +2517,12 @@ OPEN <replaceable>unbound_cursorvar</replaceable> <optional> <optional> NO </opt
        <para>
         An example:
 <programlisting>
-OPEN curs1 FOR EXECUTE 'SELECT * FROM ' || quote_ident($1);
+OPEN curs1 FOR EXECUTE 'SELECT * FROM ' || quote_ident(tabname) ' WHERE col1 = $1' USING keyvalue;
 </programlisting>
+        In this example, the table name is inserted into the query textually,
+        so use of <function>quote_ident()</> is recommended to guard against
+        SQL injection.  The comparison value for <literal>col1</> is inserted
+        via a <literal>USING</> parameter, so it needs no quoting.
        </para>
      </sect3>
 

src/pl/plpgsql/src/gram.y

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/gram.y,v 1.139 2010/01/10 17:56:50 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/gram.y,v 1.140 2010/01/19 01:35:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1704,7 +1704,27 @@ stmt_open		: K_OPEN cursor_variable
 							tok = yylex();
 							if (tok == K_EXECUTE)
 							{
-								new->dynquery = read_sql_stmt("SELECT ");
+								int		endtoken;
+
+								new->dynquery =
+									read_sql_expression2(K_USING, ';',
+														 "USING or ;",
+														 &endtoken);
+
+								/* If we found "USING", collect argument(s) */
+								if (endtoken == K_USING)
+								{
+									PLpgSQL_expr *expr;
+									
+									do
+									{
+										expr = read_sql_expression2(',', ';',
+																	", or ;",
+																	&endtoken);
+										new->params = lappend(new->params,
+															  expr);
+									} while (endtoken == ',');
+								}
 							}
 							else
 							{

src/pl/plpgsql/src/pl_exec.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.253 2010/01/02 16:58:13 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.254 2010/01/19 01:35:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -200,7 +200,8 @@ static PreparedParamsData *exec_eval_using_params(PLpgSQL_execstate *estate,
 					   List *params);
 static void free_params_data(PreparedParamsData *ppd);
 static Portal exec_dynquery_with_params(PLpgSQL_execstate *estate,
-						  PLpgSQL_expr *query, List *params);
+						  PLpgSQL_expr *dynquery, List *params,
+						  const char *portalname, int cursorOptions);
 
 
 /* ----------
@@ -2337,7 +2338,7 @@ exec_stmt_return_query(PLpgSQL_execstate *estate,
 		/* RETURN QUERY EXECUTE */
 		Assert(stmt->dynquery != NULL);
 		portal = exec_dynquery_with_params(estate, stmt->dynquery,
-										   stmt->params);
+										   stmt->params, NULL, 0);
 	}
 
 	tupmap = convert_tuples_by_position(portal->tupDesc,
@@ -3133,7 +3134,8 @@ exec_stmt_dynfors(PLpgSQL_execstate *estate, PLpgSQL_stmt_dynfors *stmt)
 	Portal		portal;
 	int			rc;
 
-	portal = exec_dynquery_with_params(estate, stmt->query, stmt->params);
+	portal = exec_dynquery_with_params(estate, stmt->query, stmt->params,
+									   NULL, 0);
 
 	/*
 	 * Execute the loop
@@ -3161,7 +3163,6 @@ exec_stmt_open(PLpgSQL_execstate *estate, PLpgSQL_stmt_open *stmt)
 	PLpgSQL_expr *query;
 	Portal		portal;
 	ParamListInfo paramLI;
-	bool		isnull;
 
 	/* ----------
 	 * Get the cursor variable and if it has an assigned name, check
@@ -3201,43 +3202,11 @@ exec_stmt_open(PLpgSQL_execstate *estate, PLpgSQL_stmt_open *stmt)
 		 * This is an OPEN refcursor FOR EXECUTE ...
 		 * ----------
 		 */
-		Datum		queryD;
+		portal = exec_dynquery_with_params(estate,
-		Oid			restype;
+										   stmt->dynquery,
-		char	   *querystr;
+										   stmt->params,
-		SPIPlanPtr	curplan;
+										   curname,
-
+										   stmt->cursor_options);
-		/* ----------
-		 * We evaluate the string expression after the
-		 * EXECUTE keyword. It's result is the querystring we have
-		 * to execute.
-		 * ----------
-		 */
-		queryD = exec_eval_expr(estate, stmt->dynquery, &isnull, &restype);
-		if (isnull)
-			ereport(ERROR,
-					(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
-					 errmsg("query string argument of EXECUTE is null")));
-
-		/* Get the C-String representation */
-		querystr = convert_value_to_string(queryD, restype);
-
-		exec_eval_cleanup(estate);
-
-		/* ----------
-		 * Now we prepare a query plan for it and open a cursor
-		 * ----------
-		 */
-		curplan = SPI_prepare_cursor(querystr, 0, NULL, stmt->cursor_options);
-		if (curplan == NULL)
-			elog(ERROR, "SPI_prepare_cursor failed for \"%s\": %s",
-				 querystr, SPI_result_code_string(SPI_result));
-		portal = SPI_cursor_open(curname, curplan, NULL, NULL,
-								 estate->readonly_func);
-		if (portal == NULL)
-			elog(ERROR, "could not open cursor for query \"%s\": %s",
-				 querystr, SPI_result_code_string(SPI_result));
-		pfree(querystr);
-		SPI_freeplan(curplan);
 
 		/*
 		 * If cursor variable was NULL, store the generated portal name in it
@@ -5530,8 +5499,11 @@ free_params_data(PreparedParamsData *ppd)
  * Open portal for dynamic query
  */
 static Portal
-exec_dynquery_with_params(PLpgSQL_execstate *estate, PLpgSQL_expr *dynquery,
+exec_dynquery_with_params(PLpgSQL_execstate *estate,
-						  List *params)
+						  PLpgSQL_expr *dynquery,
+						  List *params,
+						  const char *portalname,
+						  int cursorOptions)
 {
 	Portal		portal;
 	Datum		query;
@@ -5564,20 +5536,22 @@ exec_dynquery_with_params(PLpgSQL_execstate *estate, PLpgSQL_expr *dynquery,
 		PreparedParamsData *ppd;
 
 		ppd = exec_eval_using_params(estate, params);
-		portal = SPI_cursor_open_with_args(NULL,
+		portal = SPI_cursor_open_with_args(portalname,
 										   querystr,
 										   ppd->nargs, ppd->types,
 										   ppd->values, ppd->nulls,
-										   estate->readonly_func, 0);
+										   estate->readonly_func, 
+										   cursorOptions);
 		free_params_data(ppd);
 	}
 	else
 	{
-		portal = SPI_cursor_open_with_args(NULL,
+		portal = SPI_cursor_open_with_args(portalname,
 										   querystr,
 										   0, NULL,
 										   NULL, NULL,
-										   estate->readonly_func, 0);
+										   estate->readonly_func, 
+										   cursorOptions);
 	}
 
 	if (portal == NULL)

src/pl/plpgsql/src/pl_funcs.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_funcs.c,v 1.87 2010/01/02 16:58:13 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_funcs.c,v 1.88 2010/01/19 01:35:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -619,9 +619,28 @@ dump_open(PLpgSQL_stmt_open *stmt)
 		printf("  execute = '");
 		dump_expr(stmt->dynquery);
 		printf("'\n");
+
+		if (stmt->params != NIL)
+		{
+			ListCell   *lc;
+			int			i;
+
+			dump_indent += 2;
+			dump_ind();
+			printf("    USING\n");
+			dump_indent += 2;
+			i = 1;
+			foreach(lc, stmt->params)
+			{
+				dump_ind();
+				printf("    parameter $%d: ", i++);
+				dump_expr((PLpgSQL_expr *) lfirst(lc));
+				printf("\n");
+			}
+			dump_indent -= 4;
+		}
 	}
 	dump_indent -= 2;
-
 }
 
 static void

src/pl/plpgsql/src/plpgsql.h

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/plpgsql.h,v 1.128 2010/01/10 17:15:18 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/plpgsql.h,v 1.129 2010/01/19 01:35:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -503,6 +503,7 @@ typedef struct
 	PLpgSQL_expr *argquery;
 	PLpgSQL_expr *query;
 	PLpgSQL_expr *dynquery;
+	List	   *params;			/* USING expressions */
 } PLpgSQL_stmt_open;
 
 

src/test/regress/expected/plpgsql.out

@@ -3189,6 +3189,35 @@ NOTICE:  6
         26
 (1 row)
 
+drop function exc_using(int, text);
+create or replace function exc_using(int) returns void as $$
+declare 
+  c refcursor;
+  i int;
+begin
+  open c for execute 'select * from generate_series(1,$1)' using $1+1;
+  loop
+    fetch c into i;
+    exit when not found;
+    raise notice '%', i;
+  end loop;
+  close c;
+  return;  
+end;
+$$ language plpgsql;
+select exc_using(5);
+NOTICE:  1
+NOTICE:  2
+NOTICE:  3
+NOTICE:  4
+NOTICE:  5
+NOTICE:  6
+ exc_using 
+-----------
+ 
+(1 row)
+
+drop function exc_using(int);
 -- test FOR-over-cursor
 create or replace function forc01() returns void as $$
 declare

src/test/regress/sql/plpgsql.sql

@@ -2629,6 +2629,28 @@ $$ language plpgsql;
 
 select exc_using(5, 'foobar');
 
+drop function exc_using(int, text);
+
+create or replace function exc_using(int) returns void as $$
+declare 
+  c refcursor;
+  i int;
+begin
+  open c for execute 'select * from generate_series(1,$1)' using $1+1;
+  loop
+    fetch c into i;
+    exit when not found;
+    raise notice '%', i;
+  end loop;
+  close c;
+  return;  
+end;
+$$ language plpgsql;
+
+select exc_using(5);
+
+drop function exc_using(int);
+
 -- test FOR-over-cursor
 
 create or replace function forc01() returns void as $$