Fix WHERE CURRENT OF to work as designed within plpgsql. The argument

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.

Author: tglsfdc

src/backend/executor/execCurrent.c

@@ -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;
 

src/backend/parser/gram.y

@@ -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; }
 		;
 

src/backend/parser/parse_expr.c

@@ -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;

src/test/regress/expected/plpgsql.out

@@ -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 $$

src/test/regress/sql/plpgsql.sql

@@ -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