]> granicus.if.org Git - postgresql/commitdiff
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