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

diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 493e96e8662d90dd9b394d0b48ac122a86586571..af8e372fe83facc16bfd8b06e963ffdd78f23a7b 100644 (file)
--- a/doc/src/sgml/plpgsql.sgml
+++ b/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>
@@ -1016,7 +1016,7 @@ END;
      <command>EXECUTE</command> statement is provided:
 
 <synopsis>
-EXECUTE <replaceable class="command">command-string</replaceable> <optional> INTO <optional>STRICT</optional> <replaceable>target</replaceable> </optional> <optional> USING <replaceable>expression</replaceable> <optional>, ...</optional> </optional>;
+EXECUTE <replaceable class="command">command-string</replaceable> <optional> INTO <optional>STRICT</optional> <replaceable>target</replaceable> </optional> <optional> USING <replaceable>expression</replaceable> <optional>, ... </optional> </optional>;
 </synopsis>
 
      where <replaceable>command-string</replaceable> is an expression
@@ -1500,7 +1500,7 @@ RETURN <replaceable>expression</replaceable>;
 <synopsis>
 RETURN NEXT <replaceable>expression</replaceable>;
 RETURN QUERY <replaceable>query</replaceable>;
-RETURN QUERY EXECUTE <replaceable class="command">command-string</replaceable> <optional> USING <replaceable>expression</replaceable> <optional>, ...</optional> </optional>;
+RETURN QUERY EXECUTE <replaceable class="command">command-string</replaceable> <optional> USING <replaceable>expression</replaceable> <optional>, ... </optional> </optional>;
 </synopsis>
 
      <para>
@@ -2190,7 +2190,7 @@ $$ LANGUAGE plpgsql;
      rows:
 <synopsis>
 <optional> &lt;&lt;<replaceable>label</replaceable>&gt;&gt; </optional>
-FOR <replaceable>target</replaceable> IN EXECUTE <replaceable>text_expression</replaceable> <optional> USING <replaceable>expression</replaceable> <optional>, ...</optional> </optional> LOOP
+FOR <replaceable>target</replaceable> IN EXECUTE <replaceable>text_expression</replaceable> <optional> USING <replaceable>expression</replaceable> <optional>, ... </optional> </optional> LOOP
     <replaceable>statements</replaceable>
 END LOOP <optional> <replaceable>label</replaceable> </optional>;
 </synopsis>
@@ -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>
 
@@ -2893,7 +2898,7 @@ END LOOP <optional> <replaceable>label</replaceable> </optional>;
     raise errors.
 
 <synopsis>
-RAISE <optional> <replaceable class="parameter">level</replaceable> </optional> '<replaceable class="parameter">format</replaceable>' <optional>, <replaceable class="parameter">expression</replaceable> <optional>, ...</optional></optional> <optional> USING <replaceable class="parameter">option</replaceable> = <replaceable class="parameter">expression</replaceable> <optional>, ... </optional> </optional>;
+RAISE <optional> <replaceable class="parameter">level</replaceable> </optional> '<replaceable class="parameter">format</replaceable>' <optional>, <replaceable class="parameter">expression</replaceable> <optional>, ... </optional></optional> <optional> USING <replaceable class="parameter">option</replaceable> = <replaceable class="parameter">expression</replaceable> <optional>, ... </optional> </optional>;
 RAISE <optional> <replaceable class="parameter">level</replaceable> </optional> <replaceable class="parameter">condition_name</> <optional> USING <replaceable class="parameter">option</replaceable> = <replaceable class="parameter">expression</replaceable> <optional>, ... </optional> </optional>;
 RAISE <optional> <replaceable class="parameter">level</replaceable> </optional> SQLSTATE '<replaceable class="parameter">sqlstate</>' <optional> USING <replaceable class="parameter">option</replaceable> = <replaceable class="parameter">expression</replaceable> <optional>, ... </optional> </optional>;
 RAISE <optional> <replaceable class="parameter">level</replaceable> </optional> USING <replaceable class="parameter">option</replaceable> = <replaceable class="parameter">expression</replaceable> <optional>, ... </optional>;

diff --git a/src/pl/plpgsql/src/gram.y b/src/pl/plpgsql/src/gram.y
index c4e3c128346ba05ee05ee91ad4caa562fe44ec68..46500ad58ce228e2b170368993f95d9f2577a156 100644 (file)
--- a/src/pl/plpgsql/src/gram.y
+++ b/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
                            {

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 4139126b48d3cdfe144d2a4a99343f975597a5e3..e15076a5a8388a48bbe6f4fb78443d57741e8839 100644 (file)
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/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;
-       Oid         restype;
-       char       *querystr;
-       SPIPlanPtr  curplan;
-
-       /* ----------
-        * 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);
+       portal = exec_dynquery_with_params(estate,
+                                          stmt->dynquery,
+                                          stmt->params,
+                                          curname,
+                                          stmt->cursor_options);
 
        /*
         * 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,
-                         List *params)
+exec_dynquery_with_params(PLpgSQL_execstate *estate,
+                         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)

diff --git a/src/pl/plpgsql/src/pl_funcs.c b/src/pl/plpgsql/src/pl_funcs.c
index 9e7b1794b9ec451f857f0336a725a65aa6b3ebac..a8d0664b20444fc5678eb08e5f101acd9f03c1f9 100644 (file)
--- a/src/pl/plpgsql/src/pl_funcs.c
+++ b/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

diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index 4601a4f8136eb4b1a943c2a4df1ed473c10e55b3..2aba85374687d91133b003c3570d9bce0a8b9776 100644 (file)
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/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;
 
 

diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index e126f616fe17a962c3f81966dedfe50922a4e2cd..c2bf41d6fc23ac2cfebf87e4e0905995ef9d119f 100644 (file)
--- a/src/test/regress/expected/plpgsql.out
+++ b/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

diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index 79756f6a01cdb94bc985c83535e5700deb45d556..3613194fd9a00ba686802c6a3ec3e45c54030b0d 100644 (file)
--- a/src/test/regress/sql/plpgsql.sql
+++ b/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 $$