From 2ace38d226246b83e5cc4d8f4063a82a485ddc95 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon, 9 Nov 2009 02:36:59 +0000
Subject: [PATCH] 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.
---
src/backend/executor/execCurrent.c | 10 ++---
src/backend/parser/gram.y | 10 +----
src/backend/parser/parse_expr.c | 60 ++++++++++++++++-----------
src/test/regress/expected/plpgsql.out | 46 ++++++++++++++++++++
src/test/regress/sql/plpgsql.sql | 20 +++++++++
5 files changed, 107 insertions(+), 39 deletions(-)
diff --git a/src/backend/executor/execCurrent.c b/src/backend/executor/execCurrent.c
index 35dc05a52b8..b1b9289c262 100644
--- a/src/backend/executor/execCurrent.c
+++ b/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;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index f7fb4b859f6..d3c7c356d9f 100644
--- a/src/backend/parser/gram.y
+++ b/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; }
;
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 1e76d3b546f..bae5c7fafad 100644
--- a/src/backend/parser/parse_expr.c
+++ b/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;
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 534a60057dc..16b7907a2f7 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/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 $$
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index 51bfce2e0c1..c75f037cdbc 100644
--- a/src/test/regress/sql/plpgsql.sql
+++ b/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
--
GitLab