Fix WHERE CURRENT OF to work as designed within plpgsql. The argument
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 9 Nov 2009 02:36:59 +0000 (02:36 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 9 Nov 2009 02:36:59 +0000 (02:36 +0000)
can be the name of a plpgsql cursor variable, which formerly was converted
to $N before the core parser saw it, but that's no longer the case.
Deal with plain name references to plpgsql variables, and add a regression
test case that exposes the failure.

src/backend/executor/execCurrent.c
src/backend/parser/gram.y
src/backend/parser/parse_expr.c
src/test/regress/expected/plpgsql.out
src/test/regress/sql/plpgsql.sql

index 35dc05a52b8a1ce51978a115fc65c2b7df333bb6..b1b9289c26240a5726eb1eceb0ea09d0e1f8e5ad 100644 (file)
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/backend/executor/execCurrent.c,v 1.13 2009/11/04 22:26:05 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/executor/execCurrent.c,v 1.14 2009/11/09 02:36:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -20,7 +20,7 @@
 #include "utils/portal.h"
 
 
-static char *fetch_param_value(ExprContext *econtext, int paramId);
+static char *fetch_cursor_param_value(ExprContext *econtext, int paramId);
 static ScanState *search_plan_tree(PlanState *node, Oid table_oid);
 
 
@@ -51,7 +51,7 @@ execCurrentOf(CurrentOfExpr *cexpr,
    if (cexpr->cursor_name)
        cursor_name = cexpr->cursor_name;
    else
-       cursor_name = fetch_param_value(econtext, cexpr->cursor_param);
+       cursor_name = fetch_cursor_param_value(econtext, cexpr->cursor_param);
 
    /* Fetch table name for possible use in error messages */
    table_name = get_rel_name(table_oid);
@@ -203,12 +203,12 @@ execCurrentOf(CurrentOfExpr *cexpr,
 }
 
 /*
- * fetch_param_value
+ * fetch_cursor_param_value
  *
  * Fetch the string value of a param, verifying it is of type REFCURSOR.
  */
 static char *
-fetch_param_value(ExprContext *econtext, int paramId)
+fetch_cursor_param_value(ExprContext *econtext, int paramId)
 {
    ParamListInfo paramInfo = econtext->ecxt_param_list_info;
 
index f7fb4b859f69a81875bfef4bade275b70a0c244b..d3c7c356d9f90c45a52940010eb4b023808e38d8 100644 (file)
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.688 2009/11/05 23:24:23 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.689 2009/11/09 02:36:56 tgl Exp $
  *
  * HISTORY
  *   AUTHOR            DATE            MAJOR EVENT
@@ -7979,14 +7979,6 @@ where_or_current_clause:
                    n->cursor_param = 0;
                    $$ = (Node *) n;
                }
-           | WHERE CURRENT_P OF PARAM
-               {
-                   CurrentOfExpr *n = makeNode(CurrentOfExpr);
-                   /* cvarno is filled in by parse analysis */
-                   n->cursor_name = NULL;
-                   n->cursor_param = $4;
-                   $$ = (Node *) n;
-               }
            | /*EMPTY*/                             { $$ = NULL; }
        ;
 
index 1e76d3b546fc74909dc19f90db5f9f7ae93f1cf9..bae5c7fafadf9e7c126ef5b57eff09464f5d0bd2 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.247 2009/10/31 01:41:31 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.248 2009/11/09 02:36:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1963,32 +1963,42 @@ transformCurrentOfExpr(ParseState *pstate, CurrentOfExpr *cexpr)
    Assert(sublevels_up == 0);
 
    /*
-    * If a parameter is used, it must be of type REFCURSOR.  To verify
-    * that the parameter hooks think so, build a dummy ParamRef and
-    * transform it.
+    * Check to see if the cursor name matches a parameter of type REFCURSOR.
+    * If so, replace the raw name reference with a parameter reference.
+    * (This is a hack for the convenience of plpgsql.)
     */
-   if (cexpr->cursor_name == NULL)
+   if (cexpr->cursor_name != NULL)         /* in case already transformed */
    {
-       ParamRef *p = makeNode(ParamRef);
-       Node   *n;
-
-       p->number = cexpr->cursor_param;
-       p->location = -1;
-       n = transformParamRef(pstate, p);
-       /* Allow the parameter type to be inferred if it's unknown */
-       if (exprType(n) == UNKNOWNOID)
-           n = coerce_type(pstate, n, UNKNOWNOID,
-                           REFCURSOROID, -1,
-                           COERCION_IMPLICIT, COERCE_IMPLICIT_CAST,
-                           -1);
-       if (exprType(n) != REFCURSOROID)
-           ereport(ERROR,
-                   (errcode(ERRCODE_AMBIGUOUS_PARAMETER),
-                    errmsg("inconsistent types deduced for parameter $%d",
-                           cexpr->cursor_param),
-                    errdetail("%s versus %s",
-                              format_type_be(exprType(n)),
-                              format_type_be(REFCURSOROID))));
+       ColumnRef  *cref = makeNode(ColumnRef);
+       Node       *node = NULL;
+
+       /* Build an unqualified ColumnRef with the given name */
+       cref->fields = list_make1(makeString(cexpr->cursor_name));
+       cref->location = -1;
+
+       /* See if there is a translation available from a parser hook */
+       if (pstate->p_pre_columnref_hook != NULL)
+           node = (*pstate->p_pre_columnref_hook) (pstate, cref);
+       if (node == NULL && pstate->p_post_columnref_hook != NULL)
+           node = (*pstate->p_post_columnref_hook) (pstate, cref, NULL);
+
+       /*
+        * XXX Should we throw an error if we get a translation that isn't
+        * a refcursor Param?  For now it seems best to silently ignore
+        * false matches.
+        */
+       if (node != NULL && IsA(node, Param))
+       {
+           Param  *p = (Param *) node;
+
+           if (p->paramkind == PARAM_EXTERN &&
+               p->paramtype == REFCURSOROID)
+           {
+               /* Matches, so convert CURRENT OF to a param reference */
+               cexpr->cursor_name = NULL;
+               cexpr->cursor_param = p->paramid;
+           }
+       }
    }
 
    return (Node *) cexpr;
index 534a60057dc1cdeb1971301c9fc5ffaf3394d418..16b7907a2f7c838352bfdc61111d46002b462d8a 100644 (file)
@@ -3292,6 +3292,52 @@ select * from forc_test;
  1000 | 20
 (10 rows)
 
+-- same, with a cursor whose portal name doesn't match variable name
+create or replace function forc01() returns void as $$
+declare
+  c refcursor := 'fooled_ya';
+  r record;
+begin
+  open c for select * from forc_test;
+  loop
+    fetch c into r;
+    exit when not found;
+    raise notice '%, %', r.i, r.j;
+    update forc_test set i = i * 100, j = r.j * 2 where current of c;
+  end loop;
+end;
+$$ language plpgsql;
+select forc01();
+NOTICE:  100, 2
+NOTICE:  200, 4
+NOTICE:  300, 6
+NOTICE:  400, 8
+NOTICE:  500, 10
+NOTICE:  600, 12
+NOTICE:  700, 14
+NOTICE:  800, 16
+NOTICE:  900, 18
+NOTICE:  1000, 20
+ forc01 
+--------
+(1 row)
+
+select * from forc_test;
+   i    | j  
+--------+----
+  10000 |  4
+  20000 |  8
+  30000 | 12
+  40000 | 16
+  50000 | 20
+  60000 | 24
+  70000 | 28
+  80000 | 32
+  90000 | 36
+ 100000 | 40
+(10 rows)
+
 drop function forc01();
 -- fail because cursor has no query bound to it
 create or replace function forc_bad() returns void as $$
index 51bfce2e0c1d3bc4042ef5126e72ffd6b7142d4d..c75f037cdbc0a93cf94fd4bb9c05e0a695c46e7f 100644 (file)
@@ -2689,6 +2689,26 @@ select forc01();
 
 select * from forc_test;
 
+-- same, with a cursor whose portal name doesn't match variable name
+create or replace function forc01() returns void as $$
+declare
+  c refcursor := 'fooled_ya';
+  r record;
+begin
+  open c for select * from forc_test;
+  loop
+    fetch c into r;
+    exit when not found;
+    raise notice '%, %', r.i, r.j;
+    update forc_test set i = i * 100, j = r.j * 2 where current of c;
+  end loop;
+end;
+$$ language plpgsql;
+
+select forc01();
+
+select * from forc_test;
+
 drop function forc01();
 
 -- fail because cursor has no query bound to it