Add support for passing cursor parameters in named notation in PL/pgSQL.

Yeb Havinga, reviewed by Kevin Grittner, with small changes by me.

5 files changed, 410 insertions, 13 deletions

diff --git a/src/pl/plpgsql/src/gram.y b/src/pl/plpgsql/src/gram.y
index 8c4c2f71ca2..c04afb50fe0 100644
--- a/src/pl/plpgsql/src/gram.y
+++ b/src/pl/plpgsql/src/gram.y
@@ -67,6 +67,7 @@ static	PLpgSQL_expr	*read_sql_construct(int until,
 											const char *sqlstart,
 											bool isexpression,
 											bool valid_sql,
+											bool trim,
 											int *startloc,
 											int *endtoken);
 static	PLpgSQL_expr	*read_sql_expression(int until,
@@ -1313,6 +1314,7 @@ for_control		: for_variable K_IN
 													   "SELECT ",
 													   true,
 													   false,
+													   true,
 													   &expr1loc,
 													   &tok);
 
@@ -1692,7 +1694,7 @@ stmt_raise		: K_RAISE
 									expr = read_sql_construct(',', ';', K_USING,
 															  ", or ; or USING",
 															  "SELECT ",
-															  true, true,
+															  true, true, true,
 															  NULL, &tok);
 									new->params = lappend(new->params, expr);
 								}
@@ -1790,7 +1792,7 @@ stmt_dynexecute : K_EXECUTE
 						expr = read_sql_construct(K_INTO, K_USING, ';',
 												  "INTO or USING or ;",
 												  "SELECT ",
-												  true, true,
+												  true, true, true,
 												  NULL, &endtoken);
 
 						new = palloc(sizeof(PLpgSQL_stmt_dynexecute));
@@ -1829,7 +1831,7 @@ stmt_dynexecute : K_EXECUTE
 									expr = read_sql_construct(',', ';', K_INTO,
 															  ", or ; or INTO",
 															  "SELECT ",
-															  true, true,
+															  true, true, true,
 															  NULL, &endtoken);
 									new->params = lappend(new->params, expr);
 								} while (endtoken == ',');
@@ -2322,7 +2324,7 @@ static PLpgSQL_expr *
 read_sql_expression(int until, const char *expected)
 {
 	return read_sql_construct(until, 0, 0, expected,
-							  "SELECT ", true, true, NULL, NULL);
+							  "SELECT ", true, true, true, NULL, NULL);
 }
 
 /* Convenience routine to read an expression with two possible terminators */
@@ -2331,7 +2333,7 @@ read_sql_expression2(int until, int until2, const char *expected,
 					 int *endtoken)
 {
 	return read_sql_construct(until, until2, 0, expected,
-							  "SELECT ", true, true, NULL, endtoken);
+							  "SELECT ", true, true, true, NULL, endtoken);
 }
 
 /* Convenience routine to read a SQL statement that must end with ';' */
@@ -2339,7 +2341,7 @@ static PLpgSQL_expr *
 read_sql_stmt(const char *sqlstart)
 {
 	return read_sql_construct(';', 0, 0, ";",
-							  sqlstart, false, true, NULL, NULL);
+							  sqlstart, false, true, true, NULL, NULL);
 }
 
 /*
@@ -2352,6 +2354,7 @@ read_sql_stmt(const char *sqlstart)
  * sqlstart:	text to prefix to the accumulated SQL text
  * isexpression: whether to say we're reading an "expression" or a "statement"
  * valid_sql:   whether to check the syntax of the expr (prefixed with sqlstart)
+ * trim:		trim trailing whitespace
  * startloc:	if not NULL, location of first token is stored at *startloc
  * endtoken:	if not NULL, ending token is stored at *endtoken
  *				(this is only interesting if until2 or until3 isn't zero)
@@ -2364,6 +2367,7 @@ read_sql_construct(int until,
 				   const char *sqlstart,
 				   bool isexpression,
 				   bool valid_sql,
+				   bool trim,
 				   int *startloc,
 				   int *endtoken)
 {
@@ -2443,8 +2447,11 @@ read_sql_construct(int until,
 	plpgsql_append_source_text(&ds, startlocation, yylloc);
 
 	/* trim any trailing whitespace, for neatness */
-	while (ds.len > 0 && scanner_isspace(ds.data[ds.len - 1]))
-		ds.data[--ds.len] = '\0';
+	if (trim)
+	{
+		while (ds.len > 0 && scanner_isspace(ds.data[ds.len - 1]))
+			ds.data[--ds.len] = '\0';
+	}
 
 	expr = palloc0(sizeof(PLpgSQL_expr));
 	expr->dtype			= PLPGSQL_DTYPE_EXPR;
@@ -3375,15 +3382,23 @@ check_labels(const char *start_label, const char *end_label, int end_location)
  * Read the arguments (if any) for a cursor, followed by the until token
  *
  * If cursor has no args, just swallow the until token and return NULL.
- * If it does have args, we expect to see "( expr [, expr ...] )" followed
- * by the until token.  Consume all that and return a SELECT query that
- * evaluates the expression(s) (without the outer parens).
+ * If it does have args, we expect to see "( arg [, arg ...] )" followed
+ * by the until token, where arg may be a plain expression, or a named
+ * parameter assignment of the form argname := expr. Consume all that and
+ * return a SELECT query that evaluates the expression(s) (without the outer
+ * parens).
  */
 static PLpgSQL_expr *
 read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
 {
 	PLpgSQL_expr *expr;
+	PLpgSQL_row *row;
 	int			tok;
+	int			argc = 0;
+	char	  **argv;
+	StringInfoData ds;
+	char	   *sqlstart = "SELECT ";
+	bool		named = false;
 
 	tok = yylex();
 	if (cursor->cursor_explicit_argrow < 0)
@@ -3402,6 +3417,9 @@ read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
 		return NULL;
 	}
 
+	row = (PLpgSQL_row *) plpgsql_Datums[cursor->cursor_explicit_argrow];
+	argv = (char **) palloc0(row->nfields * sizeof(char *));
+
 	/* Else better provide arguments */
 	if (tok != '(')
 		ereport(ERROR,
@@ -3411,9 +3429,119 @@ read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
 				 parser_errposition(yylloc)));
 
 	/*
-	 * Read expressions until the matching ')'.
+	 * Read the arguments, one by one.
 	 */
-	expr = read_sql_expression(')', ")");
+	for (argc = 0; argc < row->nfields; argc++)
+	{
+		PLpgSQL_expr *item;
+		int		endtoken;
+		int		argpos;
+		int		tok1,
+				tok2;
+		int		arglocation;
+
+		/* Check if it's a named parameter: "param := value" */
+		plpgsql_peek2(&tok1, &tok2, &arglocation, NULL);
+		if (tok1 == IDENT && tok2 == COLON_EQUALS)
+		{
+			char   *argname;
+
+			/* Read the argument name, and find its position */
+			yylex();
+			argname = yylval.str;
+
+			for (argpos = 0; argpos < row->nfields; argpos++)
+			{
+				if (strcmp(row->fieldnames[argpos], argname) == 0)
+					break;
+			}
+			if (argpos == row->nfields)
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+						 errmsg("cursor \"%s\" has no argument named \"%s\"",
+								cursor->refname, argname),
+						 parser_errposition(yylloc)));
+
+			/*
+			 * Eat the ":=". We already peeked, so the error should never
+			 * happen.
+			 */
+			tok2 = yylex();
+			if (tok2 != COLON_EQUALS)
+				yyerror("syntax error");
+
+			named = true;
+		}
+		else
+			argpos = argc;
+
+		/*
+		 * Read the value expression. To provide the user with meaningful
+		 * parse error positions, we check the syntax immediately, instead of
+		 * checking the final expression that may have the arguments
+		 * reordered. Trailing whitespace must not be trimmed, because
+		 * otherwise input of the form (param -- comment\n, param) would be
+		 * translated into a form where the second parameter is commented
+		 * out.
+		 */
+		item = read_sql_construct(',', ')', 0,
+								  ",\" or \")",
+								  sqlstart,
+								  true, true,
+								  false, /* do not trim */
+								  NULL, &endtoken);
+
+		if (endtoken == ')' && !(argc == row->nfields - 1))
+			ereport(ERROR,
+					(errcode(ERRCODE_SYNTAX_ERROR),
+					 errmsg("not enough arguments for cursor \"%s\"",
+							cursor->refname),
+					 parser_errposition(yylloc)));
+
+		if (endtoken == ',' && (argc == row->nfields - 1))
+			ereport(ERROR,
+					(errcode(ERRCODE_SYNTAX_ERROR),
+					 errmsg("too many arguments for cursor \"%s\"",
+							cursor->refname),
+					 parser_errposition(yylloc)));
+
+		if (argv[argpos] != NULL)
+			ereport(ERROR,
+					(errcode(ERRCODE_SYNTAX_ERROR),
+					 errmsg("duplicate value for cursor \"%s\" parameter \"%s\"",
+							cursor->refname, row->fieldnames[argpos]),
+					 parser_errposition(arglocation)));
+
+		argv[argpos] = item->query + strlen(sqlstart);
+	}
+
+	/* Make positional argument list */
+	initStringInfo(&ds);
+	appendStringInfoString(&ds, sqlstart);
+	for (argc = 0; argc < row->nfields; argc++)
+	{
+		Assert(argv[argc] != NULL);
+
+		/*
+		 * Because named notation allows permutated argument lists, include
+		 * the parameter name for meaningful runtime errors.
+		 */
+		appendStringInfoString(&ds, argv[argc]);
+		if (named)
+			appendStringInfo(&ds, " AS %s",
+							 quote_identifier(row->fieldnames[argc]));
+		if (argc < row->nfields - 1)
+			appendStringInfoString(&ds, ", ");
+	}
+	appendStringInfoChar(&ds, ';');
+
+	expr = palloc0(sizeof(PLpgSQL_expr));
+	expr->dtype			= PLPGSQL_DTYPE_EXPR;
+	expr->query			= pstrdup(ds.data);
+	expr->plan			= NULL;
+	expr->paramnos		= NULL;
+	expr->ns            = plpgsql_ns_top();
+	pfree(ds.data);
 
 	/* Next we'd better find the until token */
 	tok = yylex();
diff --git a/src/pl/plpgsql/src/pl_scanner.c b/src/pl/plpgsql/src/pl_scanner.c
index 76e8436e50e..e6dcaae7a55 100644
--- a/src/pl/plpgsql/src/pl_scanner.c
+++ b/src/pl/plpgsql/src/pl_scanner.c
@@ -424,6 +424,36 @@ plpgsql_append_source_text(StringInfo buf,
 }
 
 /*
+ * Peek two tokens ahead in the input stream. The first token and its
+ * location the query are returned in *tok1_p and *tok1_loc, second token
+ * and its location in *tok2_p and *tok2_loc.
+ *
+ * NB: no variable or unreserved keyword lookup is performed here, they will
+ * be returned as IDENT. Reserved keywords are resolved as usual.
+ */
+void
+plpgsql_peek2(int *tok1_p, int *tok2_p, int *tok1_loc, int *tok2_loc)
+{
+	int			tok1,
+				tok2;
+	TokenAuxData aux1,
+				aux2;
+
+	tok1 = internal_yylex(&aux1);
+	tok2 = internal_yylex(&aux2);
+
+	*tok1_p = tok1;
+	if (tok1_loc)
+		*tok1_loc = aux1.lloc;
+	*tok2_p = tok2;
+	if (tok2_loc)
+		*tok2_loc = aux2.lloc;
+
+	push_back_token(tok2, &aux2);
+	push_back_token(tok1, &aux1);
+}
+
+/*
  * plpgsql_scanner_errposition
  *		Report an error cursor position, if possible.
  *
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index c638f4323f4..28460215e5c 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -962,6 +962,8 @@ extern int	plpgsql_yylex(void);
 extern void plpgsql_push_back_token(int token);
 extern void plpgsql_append_source_text(StringInfo buf,
 						   int startlocation, int endlocation);
+extern void plpgsql_peek2(int *tok1_p, int *tok2_p, int *tok1_loc,
+			  int *tok2_loc);
 extern int	plpgsql_scanner_errposition(int location);
 extern void plpgsql_yyerror(const char *message);
 extern int	plpgsql_location_to_lineno(int location);
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 238bf5f0aec..bdef259c7ed 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -2292,6 +2292,135 @@ select refcursor_test2(20000, 20000) as "Should be false",
 (1 row)
 
 --
+-- tests for cursors with named parameter arguments
+--
+create function namedparmcursor_test1(int, int) returns boolean as $$
+declare
+    c1 cursor (param1 int, param12 int) for select * from rc_test where a > param1 and b > param12;
+    nonsense record;
+begin
+    open c1(param12 := $2, param1 := $1);
+    fetch c1 into nonsense;
+    close c1;
+    if found then
+        return true;
+    else
+        return false;
+    end if;
+end
+$$ language plpgsql;
+select namedparmcursor_test1(20000, 20000) as "Should be false",
+       namedparmcursor_test1(20, 20) as "Should be true";
+ Should be false | Should be true 
+-----------------+----------------
+ f               | t
+(1 row)
+
+-- mixing named and positional argument notations
+create function namedparmcursor_test2(int, int) returns boolean as $$
+declare
+    c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+    nonsense record;
+begin
+    open c1(param1 := $1, $2);
+    fetch c1 into nonsense;
+    close c1;
+    if found then
+        return true;
+    else
+        return false;
+    end if;
+end
+$$ language plpgsql;
+select namedparmcursor_test2(20, 20);
+ namedparmcursor_test2 
+-----------------------
+ t
+(1 row)
+
+-- mixing named and positional: param2 is given twice, once in named notation
+-- and second time in positional notation. Should throw an error at parse time
+create function namedparmcursor_test3() returns void as $$
+declare
+    c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+begin
+    open c1(param2 := 20, 21);
+end
+$$ language plpgsql;
+ERROR:  duplicate value for cursor "c1" parameter "param2"
+LINE 5:     open c1(param2 := 20, 21);
+                                  ^
+-- mixing named and positional: same as previous test, but param1 is duplicated
+create function namedparmcursor_test4() returns void as $$
+declare
+    c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+begin
+    open c1(20, param1 := 21);
+end
+$$ language plpgsql;
+ERROR:  duplicate value for cursor "c1" parameter "param1"
+LINE 5:     open c1(20, param1 := 21);
+                        ^
+-- duplicate named parameter, should throw an error at parse time
+create function namedparmcursor_test5() returns void as $$
+declare
+  c1 cursor (p1 int, p2 int) for
+    select * from tenk1 where thousand = p1 and tenthous = p2;
+begin
+  open c1 (p2 := 77, p2 := 42);
+end
+$$ language plpgsql;
+ERROR:  duplicate value for cursor "c1" parameter "p2"
+LINE 6:   open c1 (p2 := 77, p2 := 42);
+                             ^
+-- not enough parameters, should throw an error at parse time
+create function namedparmcursor_test6() returns void as $$
+declare
+  c1 cursor (p1 int, p2 int) for
+    select * from tenk1 where thousand = p1 and tenthous = p2;
+begin
+  open c1 (p2 := 77);
+end
+$$ language plpgsql;
+ERROR:  not enough arguments for cursor "c1"
+LINE 6:   open c1 (p2 := 77);
+                           ^
+-- division by zero runtime error, the context given in the error message
+-- should be sensible
+create function namedparmcursor_test7() returns void as $$
+declare
+  c1 cursor (p1 int, p2 int) for
+    select * from tenk1 where thousand = p1 and tenthous = p2;
+begin
+  open c1 (p2 := 77, p1 := 42/0);
+end $$ language plpgsql;
+select namedparmcursor_test7();
+ERROR:  division by zero
+CONTEXT:  SQL statement "SELECT 42/0 AS p1, 77 AS p2;"
+PL/pgSQL function "namedparmcursor_test7" line 6 at OPEN
+-- check that line comments work correctly within the argument list (there
+-- is some special handling of this case in the code: the newline after the
+-- comment must be preserved when the argument-evaluating query is
+-- constructed, otherwise the comment effectively comments out the next
+-- argument, too)
+create function namedparmcursor_test8() returns int4 as $$
+declare
+  c1 cursor (p1 int, p2 int) for
+    select count(*) from tenk1 where thousand = p1 and tenthous = p2;
+  n int4;
+begin
+  open c1 (77 -- test
+  , 42);
+  fetch c1 into n;
+  return n;
+end $$ language plpgsql;
+select namedparmcursor_test8();
+ namedparmcursor_test8 
+-----------------------
+                     0
+(1 row)
+
+--
 -- tests for "raise" processing
 --
 create function raise_test1(int) returns int as $$
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index b47c2de312a..f577dc3cdc6 100644
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -1946,6 +1946,114 @@ select refcursor_test2(20000, 20000) as "Should be false",
        refcursor_test2(20, 20) as "Should be true";
 
 --
+-- tests for cursors with named parameter arguments
+--
+create function namedparmcursor_test1(int, int) returns boolean as $$
+declare
+    c1 cursor (param1 int, param12 int) for select * from rc_test where a > param1 and b > param12;
+    nonsense record;
+begin
+    open c1(param12 := $2, param1 := $1);
+    fetch c1 into nonsense;
+    close c1;
+    if found then
+        return true;
+    else
+        return false;
+    end if;
+end
+$$ language plpgsql;
+
+select namedparmcursor_test1(20000, 20000) as "Should be false",
+       namedparmcursor_test1(20, 20) as "Should be true";
+
+-- mixing named and positional argument notations
+create function namedparmcursor_test2(int, int) returns boolean as $$
+declare
+    c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+    nonsense record;
+begin
+    open c1(param1 := $1, $2);
+    fetch c1 into nonsense;
+    close c1;
+    if found then
+        return true;
+    else
+        return false;
+    end if;
+end
+$$ language plpgsql;
+select namedparmcursor_test2(20, 20);
+
+-- mixing named and positional: param2 is given twice, once in named notation
+-- and second time in positional notation. Should throw an error at parse time
+create function namedparmcursor_test3() returns void as $$
+declare
+    c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+begin
+    open c1(param2 := 20, 21);
+end
+$$ language plpgsql;
+
+-- mixing named and positional: same as previous test, but param1 is duplicated
+create function namedparmcursor_test4() returns void as $$
+declare
+    c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+begin
+    open c1(20, param1 := 21);
+end
+$$ language plpgsql;
+
+-- duplicate named parameter, should throw an error at parse time
+create function namedparmcursor_test5() returns void as $$
+declare
+  c1 cursor (p1 int, p2 int) for
+    select * from tenk1 where thousand = p1 and tenthous = p2;
+begin
+  open c1 (p2 := 77, p2 := 42);
+end
+$$ language plpgsql;
+
+-- not enough parameters, should throw an error at parse time
+create function namedparmcursor_test6() returns void as $$
+declare
+  c1 cursor (p1 int, p2 int) for
+    select * from tenk1 where thousand = p1 and tenthous = p2;
+begin
+  open c1 (p2 := 77);
+end
+$$ language plpgsql;
+
+-- division by zero runtime error, the context given in the error message
+-- should be sensible
+create function namedparmcursor_test7() returns void as $$
+declare
+  c1 cursor (p1 int, p2 int) for
+    select * from tenk1 where thousand = p1 and tenthous = p2;
+begin
+  open c1 (p2 := 77, p1 := 42/0);
+end $$ language plpgsql;
+select namedparmcursor_test7();
+
+-- check that line comments work correctly within the argument list (there
+-- is some special handling of this case in the code: the newline after the
+-- comment must be preserved when the argument-evaluating query is
+-- constructed, otherwise the comment effectively comments out the next
+-- argument, too)
+create function namedparmcursor_test8() returns int4 as $$
+declare
+  c1 cursor (p1 int, p2 int) for
+    select count(*) from tenk1 where thousand = p1 and tenthous = p2;
+  n int4;
+begin
+  open c1 (77 -- test
+  , 42);
+  fetch c1 into n;
+  return n;
+end $$ language plpgsql;
+select namedparmcursor_test8();
+
+--
 -- tests for "raise" processing
 --
 create function raise_test1(int) returns int as $$